fix: resolve native binaries from local jbang directories - #2660
arnabnandy7 wants to merge 1 commit into
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| The `./.jbang` directory which `jbang wrapper install` creates is just a cache which you typically would not commit to a source code repository, so you can e.g. `echo .jbang/ >>.gitignore`. | ||
|
|
||
| When `JBANG_USE_NATIVE=true`, the wrapper searches for a matching native binary beside the wrapper scripts, then in `./.jbang`, and finally in `$JBANG_DIR/bin`. If no native binary is available, it falls back to `jbang.jar`. | ||
|
|
There was a problem hiding this comment.
We need to be careful here. If the user runs ./jbang and that is version 0.123 then it would be bad if it goes and run ~/.Jbang/bin/jbang.bin which is a different version.
Reverse true too - if you run Jbang from path i wouldn't expect Jbang to pick up native binary in my local dir.
There was a problem hiding this comment.
If the user runs ./jbang
But that's what we do now. It will pick the one that's located next to the script, but if that one doesn't exist it will fall back to any installed version of JBang.
Reverse true too
Indeed, that should not happen. Your install would be broken (it would mean that you have a ~/.jbang/bin/jbang but no ~/.jbang\bin\jbang.jar which should not happen)
Btw, unrelated to this PR, but I've never read the line above the changes:
which you typically would not commit to a source code repository
THis is not true, you would actually commit this! Just like you'd do with gradlew and mvnw wrappers
There was a problem hiding this comment.
@maxandersen @quintesse with the current lookup order, a local wrapper that has its own .jbang/jbang.jar could still pick up a native binary from the global installation, potentially running a different JBang version.
I think the safer order is:
- Look for a native binary beside the invoked launcher.
- Look in the launcher’s
.jbangdirectory. - Fall back to the launcher-local JAR.
- Only when none of those exist, delegate to
$JBANG_DIR/bin/jbang, which can resolve its own matching native binary or JAR.
This also means that invoking the globally installed launcher won’t inspect the project’s local .jbang directory, since lookup remains relative to the invoked script rather than the current working directory.
I’ll update the implementation and documentation to follow this order.
Signed-off-by: Arnab Nandy <arnab_nandy7@yahoo.com>
ce19a55 to
2e1b36a
Compare
Fixes #2624.
Native command handling now uses the same location precedence as JAR handling:
.jbangdirectory$JBANG_DIR/binWithin each directory, platform-specific native binaries are preferred over generic binaries. When no native binary is available, startup continues to fall back to
jbang.jar.This change:
App.copyJBangFiles()to copy native binaries from the source directory or its.jbangdirectory.installation.adoc.Verification:
./gradlew spotlessApply./gradlew test --tests "dev.jbang.cli.TestApp" --tests "dev.jbang.cli.TestScriptNativeLookup"