feat: support native-aware wrapper install/update flows#2467
feat: support native-aware wrapper install/update flows#2467maxandersen wants to merge 15 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds platform-native bundle packaging and CI publishing, introduces JBANG_DOWNLOAD_BASEURL and Settings.getDownloadBaseUrl(), updates VersionChecker to use the configurable base URL, refactors App.java to prefer native bundles with fallback and atomic .new promotion, and enhances startup scripts (bash, cmd, ps1) to resolve, download, and install native or generic bundles. ChangesNative Bundle Download & Update Support
sequenceDiagram
participant Installer as Installer (App / Scripts)
participant Settings as Settings
participant Remote as Release Host
participant CI as CI/Artifacts
Installer->>Settings: read JBANG_DOWNLOAD_BASEURL / version
Installer->>Remote: request native bundle URL (native or fallback)
CI->>Remote: upload per-OS artifacts (zip/tar)
Remote-->>Installer: serve selected archive
Installer->>Installer: extract, promote *.new, chmod/promote exe
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@quintesse pushing this suggestion on adding native image to the scripts for early feedback + to run test on CI to see if the packaging of native works across the OS's. |
There was a problem hiding this comment.
In general it looks okay, although somewhat verbose and with code duplication (eg download code gets repeated twice).
The only thing I think is actually wrong is that when you set JBANG_USE_NATIVE to true it requires a native binary to exist, if not it will try to download the binary release and install it... each time.
I think we should do that at most once.
We could either come up with some marker system that tells JBang to not try again, but that might get overly complex (you have to think about how you would ever clear those marks, to signal that you want to try again).
Or we simply say that we only try to install binaries for architectures that we know have binary releases.
This would mean that this code:
native_bundle_arch() {
case "$arch" in
x86_64|amd64)
echo "x64"
;;
aarch64|arm64)
echo "aarch64"
;;
*)
echo "$arch"
;;
esac
}Should not print $arch in the default case but an empty string so the code knows not to try to download binaries for that architecture.
If in the future we start supporting that architecture then the moment the user updates to the latest jbang version the binaries will automatically be installed when they first run jbang after the update.
|
@quintesse adjusted based on your suggestions and issues found locally - see commits for rationales for the fixes. |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/scripts/jbang.cmd (2)
47-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPromote
jbang.bin.exe.newbefore the early native jump.Line 47 jumps straight to
:run_with_cliwheneverbinaryPathis already set, so Lines 56-59 never run in the normal native path. That means staged native updates are never promoted and the old binary keeps running.Suggested fix
+if "%JBANG_USE_NATIVE%"=="true" if exist "%~dp0jbang.bin.exe.new" ( + copy /y "%~dp0jbang.bin.exe.new" "%~dp0jbang.bin.exe" > nul 2>&1 + del /f /q "%~dp0jbang.bin.exe.new" +) + if not "!binaryPath!"=="" goto :run_with_cli if not exist "%jarPath%" goto :run_with_cli if exist "%jarPath%.new" ( rem a new jbang version was found, we replace the old one with it copy /y "%jarPath%.new" "%jarPath%" > nul 2>&1 del /f /q "%jarPath%.new" ) - -if "%JBANG_USE_NATIVE%"=="true" if exist "%~dp0jbang.bin.exe.new" ( - copy /y "%~dp0jbang.bin.exe.new" "%~dp0jbang.bin.exe" > nul 2>&1 - del /f /q "%~dp0jbang.bin.exe.new" -)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/scripts/jbang.cmd` around lines 47 - 59, The early jump to :run_with_cli when %binaryPath% is set prevents promoting staged native updates; move the promotion block that checks "%JBANG_USE_NATIVE%"=="true" and the existence of "%~dp0jbang.bin.exe.new" to run before the conditional goto :run_with_cli so the staged native binary (jbang.bin.exe.new) is copied into place and deleted prior to any early native-path exit; update the script so the promotion logic referencing %JBANG_USE_NATIVE% and "%~dp0jbang.bin.exe.new" executes before the check that jumps to :run_with_cli.
35-44:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTreat the generic fallback as a valid installed state.
If
JBANG_USE_NATIVE=trueand the native ZIP is missing, the delegated PowerShell install can still succeed with the generic bundle and leavejbang.bin.exeabsent. Line 38 then makesneedsInstalltrue on every subsequent launch, so this path keeps re-running the bootstrap instead of using the installed JAR fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/scripts/jbang.cmd` around lines 35 - 44, The current bootstrap marks needsInstall=true whenever JBANG_USE_NATIVE=="true" and jbang.bin.exe is missing, which forces reinstall loops even if the generic jbang.jar is present; change the logic so the presence of "%JBDIR%\bin\jbang.jar" alone counts as a valid installed state and only require installation when the jar is missing (i.e., check for jbang.jar first and set needsInstall=true if it does not exist; only consider the native jbang.bin.exe requirement if the jar is absent or you explicitly need to prefer native, otherwise do not set needsInstall just because jbang.bin.exe is missing). Ensure you update the checks around the needsInstall variable and the JBANG_USE_NATIVE gate (variables/binaries referenced: needsInstall, "%JBDIR%\bin\jbang.jar", "%JBDIR%\bin\jbang.bin.exe", JBANG_USE_NATIVE, jbang.ps1, jbang.cmd).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/tag-and-release.yml:
- Around line 81-83: The workflow uses the deprecated ::set-output command to
export RELEASE_VERSION; update the step that sets RELEASE_VERSION (the
RELEASE_VERSION variable assignment and the echo "::set-output
name=RELEASE_VERSION::$RELEASE_VERSION" line) to write the output to
$GITHUB_OUTPUT instead by emitting a line in the format RELEASE_VERSION=<value>
into $GITHUB_OUTPUT (append using shell redirection) so downstream steps can
read the output.
- Around line 72-77: The download step "Download native bundles" tries to fetch
artifacts matching "*-jbang-native-bundles-*" but no such upload step exists;
add a fail-fast guard so the step only runs when the producer upload step
completed successfully (referencing the existing upload step named
"jreleaser-release"). Update the "Download native bundles" step to include an
if: conditional (e.g., if: ${{ steps.jreleaser-release.outcome == 'success' }})
so the download only runs when the producer step has run and succeeded, or
alternatively change the download pattern to match the actual uploaded artifact
name ("jreleaser-release") if that was intended.
In `@build.gradle`:
- Around line 332-343: The archive tasks nativeDistZip and nativeDistTar
currently rely on commonSpec which uses a configuration-time check for
jbang.bin* existence, so in clean builds the nativeImage output can be missed;
change commonSpec (or the archive task definitions) to include the nativeImage
outputs via task providers instead of a file-exists check — e.g., remove the
config-time File.exists guard and use provider-based inclusion like from({
tasks.named('nativeImage').flatMap { it.outputs.files } }) or
from(tasks.named('nativeImage').map { it.outputs.files }) and keep
dependsOn('nativeImage') so the jbang.bin* produced by nativeImage is always
captured into nativeDistZip/nativeDistTar.
In `@docs/modules/ROOT/pages/installation.adoc`:
- Around line 52-54: Update the documentation entry for the environment variable
JBANG_DOWNLOAD_BASEURL in the "Startup Script Environment Variables" table on
installation.adoc so the documented default matches runtime behavior: change the
shown default from https://github.com/jbangdev/jbang/releases to
https://www.jbang.dev/releases and ensure the descriptive text still explains
this is the base URL used for downloading JBang releases (for testing/offline
mirrors).
In `@src/main/java/dev/jbang/cli/App.java`:
- Around line 256-262: The resolveDownloadUrl method builds URLs with a
hardcoded "v" prefix which causes double "v" when JBANG_DOWNLOAD_VERSION already
includes a leading "v"; update resolveDownloadUrl to normalize the version
string (trim and strip any leading 'v' or 'V') before concatenating so that
resolveDownloadUrl(String version, String bundleName) returns baseUrl +
"/download/v" + normalizedVersion + "/" + bundleName and preserves the existing
latest branch when version is null/empty; reference the resolveDownloadUrl
method to locate and change the normalization logic.
In `@src/main/java/dev/jbang/Settings.java`:
- Around line 47-53: In getDownloadBaseUrl(), the environment override stored in
baseUrl is only checked with trim() for emptiness but the untrimmed value is
returned and normalized, which can produce invalid URLs like " https://... ";
fix by trimming the env value once it's read (assign baseUrl = baseUrl.trim())
before the emptiness check/normalization so replaceAll("/+$", "") runs against
the trimmed string; references: getDownloadBaseUrl(), ENV_DOWNLOAD_BASEURL,
DEFAULT_RELEASES_URL, variable baseUrl.
In `@src/main/scripts/jbang`:
- Around line 385-390: The current check sets needs_install="true" whenever
JBANG_USE_NATIVE=true and the native binary ($native_name) is missing, which
ignores a successful generic bundle install; modify the logic around the
needs_install determination in the same block (symbols: JBANG_USE_NATIVE,
native_name, needs_install, install_jbang_bundle) so that installation is
triggered only if neither the native binary nor the generic jbang launcher/JAR
(e.g., jbang.bin or whatever generic artifact your script uses) exists;
alternatively, after calling install_jbang_bundle("$JBANG_DOWNLOAD_VERSION",
"$JBANG_USE_NATIVE") verify which artifact was actually installed and only treat
missing launchers as needing install on subsequent runs (i.e., check for either
the native binary OR the generic jbang launcher before leaving
needs_install=true).
In `@src/main/scripts/jbang.ps1`:
- Around line 223-228: The script re-triggers installation because when
Install-JBangBundle falls back to the generic jbang.zip it doesn't create the
native executable, so the $needsInstall check (using Get-NativeExecName) stays
true; fix by updating Install-JBangBundle to create the expected native shim at
"$JBDIR\bin\$(Get-NativeExecName)" (or a small launcher/executable-stub)
whenever it falls back to the generic JAR so subsequent runs find the native
exec and $needsInstall becomes false, or alternatively adjust the $needsInstall
logic to consider a successful generic fallback (presence of jbang.jar and
jbang.ps1) as sufficient even when JBANG_USE_NATIVE is "true".
- Around line 153-170: The script uses PowerShell 7 ternary syntax in the
interpolations $($ReleaseVersion ? $ReleaseVersion : 'latest'), which breaks
under the declared PowerShell 5 requirement; replace both occurrences with a
PowerShell 5–compatible conditional that yields $ReleaseVersion when it is
set/non-empty and 'latest' otherwise (either by computing a local variable like
$displayVersion before the WriteLine calls or by using an -if/-else expression),
and keep the surrounding logic in the native/generic download blocks
(references: $ReleaseVersion, Get-NativeBundleUrl, Get-FallbackBundleUrl, the
WriteLine calls).
---
Outside diff comments:
In `@src/main/scripts/jbang.cmd`:
- Around line 47-59: The early jump to :run_with_cli when %binaryPath% is set
prevents promoting staged native updates; move the promotion block that checks
"%JBANG_USE_NATIVE%"=="true" and the existence of "%~dp0jbang.bin.exe.new" to
run before the conditional goto :run_with_cli so the staged native binary
(jbang.bin.exe.new) is copied into place and deleted prior to any early
native-path exit; update the script so the promotion logic referencing
%JBANG_USE_NATIVE% and "%~dp0jbang.bin.exe.new" executes before the check that
jumps to :run_with_cli.
- Around line 35-44: The current bootstrap marks needsInstall=true whenever
JBANG_USE_NATIVE=="true" and jbang.bin.exe is missing, which forces reinstall
loops even if the generic jbang.jar is present; change the logic so the presence
of "%JBDIR%\bin\jbang.jar" alone counts as a valid installed state and only
require installation when the jar is missing (i.e., check for jbang.jar first
and set needsInstall=true if it does not exist; only consider the native
jbang.bin.exe requirement if the jar is absent or you explicitly need to prefer
native, otherwise do not set needsInstall just because jbang.bin.exe is
missing). Ensure you update the checks around the needsInstall variable and the
JBANG_USE_NATIVE gate (variables/binaries referenced: needsInstall,
"%JBDIR%\bin\jbang.jar", "%JBDIR%\bin\jbang.bin.exe", JBANG_USE_NATIVE,
jbang.ps1, jbang.cmd).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e78e114b-d701-472a-95fa-1820167d62b5
📒 Files selected for processing (11)
.github/workflows/step-ci-build.yml.github/workflows/tag-and-release.ymlbuild.gradledocs/modules/ROOT/pages/installation.adocjreleaser.ymlsrc/main/java/dev/jbang/Settings.javasrc/main/java/dev/jbang/cli/App.javasrc/main/java/dev/jbang/util/VersionChecker.javasrc/main/scripts/jbangsrc/main/scripts/jbang.cmdsrc/main/scripts/jbang.ps1
| |`JBANG_DOWNLOAD_BASEURL` | ||
| |`https://github.com/jbangdev/jbang/releases` | ||
| |Base URL for downloading JBang releases (used for testing and offline mirrors) |
There was a problem hiding this comment.
Fix the documented default for JBANG_DOWNLOAD_BASEURL to match runtime behavior.
Line 53 documents GitHub releases as default, but the Java default is https://www.jbang.dev/releases. This inconsistency can cause confusing troubleshooting outcomes.
Proposed fix
|`JBANG_DOWNLOAD_BASEURL`
-|`https://github.com/jbangdev/jbang/releases`
+|`https://www.jbang.dev/releases`
|Base URL for downloading JBang releases (used for testing and offline mirrors)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/modules/ROOT/pages/installation.adoc` around lines 52 - 54, Update the
documentation entry for the environment variable JBANG_DOWNLOAD_BASEURL in the
"Startup Script Environment Variables" table on installation.adoc so the
documented default matches runtime behavior: change the shown default from
https://github.com/jbangdev/jbang/releases to https://www.jbang.dev/releases and
ensure the descriptive text still explains this is the base URL used for
downloading JBang releases (for testing/offline mirrors).
| private static String resolveDownloadUrl(String version, String bundleName) { | ||
| String baseUrl = getDownloadBaseUrl(); | ||
| if (version == null || version.trim().isEmpty()) { | ||
| return baseUrl + "/latest/download/" + bundleName; | ||
| } else { | ||
| return baseUrl + "/download/v" + version + "/" + bundleName; | ||
| } |
There was a problem hiding this comment.
Normalize JBANG_DOWNLOAD_VERSION before adding the v prefix.
With the current logic, setting JBANG_DOWNLOAD_VERSION=v0.138.0 generates /download/vv0.138.0/..., which breaks downloads.
Proposed fix
private static String resolveDownloadUrl(String version, String bundleName) {
String baseUrl = getDownloadBaseUrl();
if (version == null || version.trim().isEmpty()) {
return baseUrl + "/latest/download/" + bundleName;
} else {
- return baseUrl + "/download/v" + version + "/" + bundleName;
+ String normalizedVersion = version.trim();
+ if (normalizedVersion.startsWith("v")) {
+ normalizedVersion = normalizedVersion.substring(1);
+ }
+ return baseUrl + "/download/v" + normalizedVersion + "/" + bundleName;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private static String resolveDownloadUrl(String version, String bundleName) { | |
| String baseUrl = getDownloadBaseUrl(); | |
| if (version == null || version.trim().isEmpty()) { | |
| return baseUrl + "/latest/download/" + bundleName; | |
| } else { | |
| return baseUrl + "/download/v" + version + "/" + bundleName; | |
| } | |
| private static String resolveDownloadUrl(String version, String bundleName) { | |
| String baseUrl = getDownloadBaseUrl(); | |
| if (version == null || version.trim().isEmpty()) { | |
| return baseUrl + "/latest/download/" + bundleName; | |
| } else { | |
| String normalizedVersion = version.trim(); | |
| if (normalizedVersion.startsWith("v")) { | |
| normalizedVersion = normalizedVersion.substring(1); | |
| } | |
| return baseUrl + "/download/v" + normalizedVersion + "/" + bundleName; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/dev/jbang/cli/App.java` around lines 256 - 262, The
resolveDownloadUrl method builds URLs with a hardcoded "v" prefix which causes
double "v" when JBANG_DOWNLOAD_VERSION already includes a leading "v"; update
resolveDownloadUrl to normalize the version string (trim and strip any leading
'v' or 'V') before concatenating so that resolveDownloadUrl(String version,
String bundleName) returns baseUrl + "/download/v" + normalizedVersion + "/" +
bundleName and preserves the existing latest branch when version is null/empty;
reference the resolveDownloadUrl method to locate and change the normalization
logic.
| if ($NativeRequested -eq "true") { | ||
| $jburl = Get-NativeBundleUrl $ReleaseVersion | ||
| [Console]::Error.WriteLine("Downloading JBang $($ReleaseVersion ? $ReleaseVersion : 'latest') native bundle...") | ||
| try { Invoke-WebRequest "$jburl" -OutFile $bundlePath; $ok=$? } catch { | ||
| $ok=$false | ||
| $err=$_ | ||
| } | ||
| if (-not $ok) { | ||
| [Console]::Error.WriteLine("WARNING: Native JBang bundle not available from $jburl, falling back to generic bundle") | ||
| $jburl = Get-FallbackBundleUrl $ReleaseVersion | ||
| try { Invoke-WebRequest "$jburl" -OutFile $bundlePath; $ok=$? } catch { | ||
| $ok=$false | ||
| $err=$_ | ||
| } | ||
| } | ||
| } else { | ||
| $jburl="https://github.com/jbangdev/jbang/releases/download/v$env:JBANG_DOWNLOAD_VERSION/jbang.zip"; | ||
| } | ||
| [Console]::Error.WriteLine("Downloading JBang $env:JBANG_DOWNLOAD_VERSION...") | ||
| try { Invoke-WebRequest "$jburl" -OutFile "$TDIR\urls\jbang.zip"; $ok=$? } catch { | ||
| $ok=$false | ||
| $err=$_ | ||
| $jburl = Get-FallbackBundleUrl $ReleaseVersion | ||
| [Console]::Error.WriteLine("Downloading JBang $($ReleaseVersion ? $ReleaseVersion : 'latest')...") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Windows PowerShell 5.1 support the ternary operator (condition ? expr1 : expr2), or was it introduced in PowerShell 7?
💡 Result:
Windows PowerShell 5.1 does not support the ternary operator [1][2]. The ternary operator (condition ? expr1 : expr2) was introduced in PowerShell 7.0 [1][3]. While users of Windows PowerShell 5.1 often use if-else statements as the standard alternative for conditional logic [2], they can also utilize expression-based if or switch statements, provided the results are wrapped in subexpressions $() and properly handled to avoid issues with null or empty collections [3]. Official Microsoft documentation confirms that the ternary operator syntax is a feature of PowerShell 7 and later versions [1][4].
Citations:
- 1: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_if?view=powershell-7.4
- 2: https://www.reddit.com/r/PowerShell/comments/1in5r6f/if_statement_vs_ternary_operator/
- 3: https://stackoverflow.com/questions/31341998/ternary-operator-in-powershell
- 4: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_operators?view=powershell-7.6
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '130,190p' src/main/scripts/jbang.ps1 | nl -ba | sed -n '1,120p'Repository: jbangdev/jbang
Length of output: 101
🏁 Script executed:
python3 - <<'PY'
path="src/main/scripts/jbang.ps1"
start=130
end=190
with open(path,'r',encoding='utf-8') as f:
for i,line in enumerate(f, start=1):
if start <= i <= end:
print(f"{i:4d}:{line.rstrip()}")
PYRepository: jbangdev/jbang
Length of output: 2560
🏁 Script executed:
python3 - <<'PY'
path="src/main/scripts/jbang.ps1"
with open(path,'r',encoding='utf-8') as f:
for i in range(1,60):
line=f.readline()
if not line: break
print(f"{i:3d}:{line.rstrip()}")
PYRepository: jbangdev/jbang
Length of output: 2728
Remove PowerShell 7 ternary syntax from this PowerShell 5 script
src/main/scripts/jbang.ps1 uses $($ReleaseVersion ? $ReleaseVersion : 'latest') at lines 155 and 170, but the script declares #Requires -Version 5, and Windows PowerShell 5.1 cannot parse the condition ? expr1 : expr2 ternary operator (it’s a PowerShell 7+ feature). Replace with a PowerShell 5-compatible conditional.
Suggested fix
function Install-JBangBundle {
param([string]$ReleaseVersion, [string]$NativeRequested)
$bundlePath = "$TDIR\urls\jbang.zip"
+ $displayVersion = if ($ReleaseVersion) { $ReleaseVersion } else { "latest" }
New-Item -ItemType Directory -Force -Path "$TDIR\urls" >$null 2>&1
if ($NativeRequested -eq "true") {
$jburl = Get-NativeBundleUrl $ReleaseVersion
- [Console]::Error.WriteLine("Downloading JBang $($ReleaseVersion ? $ReleaseVersion : 'latest') native bundle...")
+ [Console]::Error.WriteLine("Downloading JBang $displayVersion native bundle...")
try { Invoke-WebRequest "$jburl" -OutFile $bundlePath; $ok=$? } catch {
$ok=$false
$err=$_
}
@@
} else {
$jburl = Get-FallbackBundleUrl $ReleaseVersion
- [Console]::Error.WriteLine("Downloading JBang $($ReleaseVersion ? $ReleaseVersion : 'latest')...")
+ [Console]::Error.WriteLine("Downloading JBang $displayVersion...")
try { Invoke-WebRequest "$jburl" -OutFile $bundlePath; $ok=$? } catch {
$ok=$false
$err=$_
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/scripts/jbang.ps1` around lines 153 - 170, The script uses
PowerShell 7 ternary syntax in the interpolations $($ReleaseVersion ?
$ReleaseVersion : 'latest'), which breaks under the declared PowerShell 5
requirement; replace both occurrences with a PowerShell 5–compatible conditional
that yields $ReleaseVersion when it is set/non-empty and 'latest' otherwise
(either by computing a local variable like $displayVersion before the WriteLine
calls or by using an -if/-else expression), and keep the surrounding logic in
the native/generic download blocks (references: $ReleaseVersion,
Get-NativeBundleUrl, Get-FallbackBundleUrl, the WriteLine calls).
Adds native binary support to JBang wrapper scripts and install/update mechanisms.
- Wrapper scripts (bash, PowerShell, CMD) now detect and use native binaries when JBANG_USE_NATIVE=true
- Falls back to JAR execution if native binary unavailable
- Download native platform-specific bundles (jbang-{os}-{arch}.tar/zip)
- Automatic fallback to generic bundle if native bundle missing
- Safe update mechanism using .new files promoted on next wrapper run
- Both native and generic bundles respect JBANG_DOWNLOAD_VERSION
- Supports versioned downloads (e.g., /download/v0.138.0/jbang-linux-x64.tar)
- Defaults to /latest/download/ when version not specified
- JBANG_USE_NATIVE: prefer native binary over JAR
- JBANG_DOWNLOAD_BASEURL: override release base URL (for testing/mirrors)
- JBANG_DOWNLOAD_VERSION: specific version to download during bootstrap
- Added nativeDistZip and nativeDistTar Gradle tasks
- CI builds and uploads native bundles for Linux, macOS, Windows
- JReleaser configured to include native bundle artifacts
- Fixed unreachable code in VersionChecker.retrieveLatestVersion()
- Improved exception logging in native bundle fallback
- Replaced Stream.forEach with for-loop for side effects
- Used Arrays.asList for Java 8 compatibility
- Added cross-reference comments to keep Java/bash/PS1 in sync
- Null-safe string comparisons throughout
- Added environment variable table to installation.adoc
- Documents JBANG_USE_NATIVE, JBANG_DOWNLOAD_BASEURL, JBANG_DOWNLOAD_VERSION
Closes #2458
The retval variable was set by download() but never updated after the tar extraction. If tar failed (corrupted archive, disk full), the error was silently ignored because retval still held the download exit code (0). Now we capture $? after tar.
The variable was declared but no longer used after the download logic was refactored to reuse jburl for the fallback path.
Align getNativeBundleArch() in build.gradle with the same function in App.java, jbang (bash), and jbang.ps1 — all of which now return empty/null for unrecognized architectures to prevent attempting native bundle downloads on unsupported platforms.
The copyFiles refactor inadvertently changed jbang.jar to use .new promotion on all platforms. The original code only used .new for jbang.jar on Windows (to work around locked-file issues). Native binaries (jbang.bin/jbang.bin.exe) correctly use .new on all platforms since a running binary cannot be replaced in place.
Both App.java and VersionChecker.java duplicated the same DEFAULT_JBANG_RELEASES_URL constant and env-var-reading logic for JBANG_DOWNLOAD_BASEURL. Centralise in Settings to avoid divergence.
This reverts commit 7a74bd3.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.gradle`:
- Around line 332-343: The native bundle tasks nativeDistZip and nativeDistTar
rely on commonSpec which references build/tmp/version.txt but only declare
dependsOn('nativeImage'), so running these tasks on a clean workspace can
produce bundles missing version.txt; update the task registrations for
nativeDistZip and nativeDistTar to add an explicit dependency on the task or
file that generates version.txt (e.g., dependsOn('versionTxt') or
dependsOn(tasks.named('versionTxt'))), ensuring versionTxt is executed before
packaging while keeping existing nativeImage dependency.
In `@src/main/scripts/jbang`:
- Around line 70-73: The case statement that maps native bundle architectures
(the case on variable "arch") only matches "x86_64|amd64" and therefore misses
when "arch" was already normalized to "x64"; update the pattern in that case arm
to also match "x64" (e.g., change the pattern to "x86_64|amd64|x64") so the
branch echoes "x64" for normalized inputs and the native bundle download logic
uses the correct architecture string.
In `@src/main/scripts/jbang.cmd`:
- Around line 37-39: The current check sets needsInstall when JBANG_USE_NATIVE
is true and jbang.bin.exe is missing, which forces reinstall even if a generic
fallback (jbang.jar + jbang.cmd) is already present; change the condition that
sets needsInstall for native mode to require both that jbang.bin.exe is missing
AND that the generic fallback (jbang.cmd or jbang.jar) is also missing — i.e.,
update the logic around JBANG_USE_NATIVE, jbang.bin.exe, jbang.cmd and jbang.jar
so the script does not mark needsInstall=true for native when only the native
binary is absent but a working fallback exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d346eb7-88d5-4f10-9b6d-5ed64859b2f0
📒 Files selected for processing (11)
.github/workflows/step-ci-build.yml.github/workflows/tag-and-release.ymlbuild.gradledocs/modules/ROOT/pages/installation.adocjreleaser.ymlsrc/main/java/dev/jbang/Settings.javasrc/main/java/dev/jbang/cli/App.javasrc/main/java/dev/jbang/util/VersionChecker.javasrc/main/scripts/jbangsrc/main/scripts/jbang.cmdsrc/main/scripts/jbang.ps1
✅ Files skipped from review due to trivial changes (1)
- docs/modules/ROOT/pages/installation.adoc
| tasks.register('nativeDistZip', Zip) { | ||
| dependsOn('nativeImage') | ||
| with commonSpec | ||
| archiveFileName = "${getNativeBundleBaseName()}.zip" | ||
| } | ||
|
|
||
| tasks.register('nativeDistTar', Tar) { | ||
| dependsOn('nativeImage') | ||
| with commonSpec | ||
| compression = Compression.NONE | ||
| archiveFileName = "${getNativeBundleBaseName()}.tar" | ||
| } |
There was a problem hiding this comment.
Add versionTxt as an explicit dependency for native bundle tasks.
commonSpec includes build/tmp/version.txt, but these tasks only depend on nativeImage. Running them directly on a clean workspace can package bundles without version.txt.
Proposed fix
tasks.register('nativeDistZip', Zip) {
- dependsOn('nativeImage')
+ dependsOn('nativeImage', 'versionTxt')
with commonSpec
archiveFileName = "${getNativeBundleBaseName()}.zip"
}
tasks.register('nativeDistTar', Tar) {
- dependsOn('nativeImage')
+ dependsOn('nativeImage', 'versionTxt')
with commonSpec
compression = Compression.NONE
archiveFileName = "${getNativeBundleBaseName()}.tar"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tasks.register('nativeDistZip', Zip) { | |
| dependsOn('nativeImage') | |
| with commonSpec | |
| archiveFileName = "${getNativeBundleBaseName()}.zip" | |
| } | |
| tasks.register('nativeDistTar', Tar) { | |
| dependsOn('nativeImage') | |
| with commonSpec | |
| compression = Compression.NONE | |
| archiveFileName = "${getNativeBundleBaseName()}.tar" | |
| } | |
| tasks.register('nativeDistZip', Zip) { | |
| dependsOn('nativeImage', 'versionTxt') | |
| with commonSpec | |
| archiveFileName = "${getNativeBundleBaseName()}.zip" | |
| } | |
| tasks.register('nativeDistTar', Tar) { | |
| dependsOn('nativeImage', 'versionTxt') | |
| with commonSpec | |
| compression = Compression.NONE | |
| archiveFileName = "${getNativeBundleBaseName()}.tar" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@build.gradle` around lines 332 - 343, The native bundle tasks nativeDistZip
and nativeDistTar rely on commonSpec which references build/tmp/version.txt but
only declare dependsOn('nativeImage'), so running these tasks on a clean
workspace can produce bundles missing version.txt; update the task registrations
for nativeDistZip and nativeDistTar to add an explicit dependency on the task or
file that generates version.txt (e.g., dependsOn('versionTxt') or
dependsOn(tasks.named('versionTxt'))), ensuring versionTxt is executed before
packaging while keeping existing nativeImage dependency.
| case "$arch" in | ||
| x86_64|amd64) | ||
| echo "x64" | ||
| ;; |
There was a problem hiding this comment.
Fix x64 mapping in native bundle architecture resolution.
Line 70 only matches x86_64|amd64, but arch is normalized to x64 earlier (Line 334). On x64 hosts this returns empty arch and skips native bundle download.
Suggested patch
native_bundle_arch() {
case "$arch" in
- x86_64|amd64)
+ x86_64|amd64|x64)
echo "x64"
;;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case "$arch" in | |
| x86_64|amd64) | |
| echo "x64" | |
| ;; | |
| case "$arch" in | |
| x86_64|amd64|x64) | |
| echo "x64" | |
| ;; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/scripts/jbang` around lines 70 - 73, The case statement that maps
native bundle architectures (the case on variable "arch") only matches
"x86_64|amd64" and therefore misses when "arch" was already normalized to "x64";
update the pattern in that case arm to also match "x64" (e.g., change the
pattern to "x86_64|amd64|x64") so the branch echoes "x64" for normalized inputs
and the native bundle download logic uses the correct architecture string.
| if not exist "%JBDIR%\bin\jbang.jar" set needsInstall=true | ||
| if "%JBANG_USE_NATIVE%"=="true" if not exist "%JBDIR%\bin\jbang.bin.exe" set needsInstall=true | ||
| if "!needsInstall!"=="true" ( |
There was a problem hiding this comment.
Avoid install loop after successful generic fallback in native mode.
Line 38 makes install mandatory whenever jbang.bin.exe is missing. If fallback installed only jbang.jar + jbang.cmd, this keeps reinstalling every run.
Suggested patch
set needsInstall=false
if not exist "%JBDIR%\bin\jbang.jar" set needsInstall=true
- if "%JBANG_USE_NATIVE%"=="true" if not exist "%JBDIR%\bin\jbang.bin.exe" set needsInstall=true
+ if not exist "%JBDIR%\bin\jbang.cmd" set needsInstall=true
+ if "%JBANG_USE_NATIVE%"=="true" if not exist "%JBDIR%\bin\jbang.bin.exe" if not exist "%JBDIR%\bin\jbang.cmd" set needsInstall=true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/scripts/jbang.cmd` around lines 37 - 39, The current check sets
needsInstall when JBANG_USE_NATIVE is true and jbang.bin.exe is missing, which
forces reinstall even if a generic fallback (jbang.jar + jbang.cmd) is already
present; change the condition that sets needsInstall for native mode to require
both that jbang.bin.exe is missing AND that the generic fallback (jbang.cmd or
jbang.jar) is also missing — i.e., update the logic around JBANG_USE_NATIVE,
jbang.bin.exe, jbang.cmd and jbang.jar so the script does not mark
needsInstall=true for native when only the native binary is absent but a working
fallback exists.
|
@quintesse I rebased with main so this has retry logic - what I then noticed that when url is unreachable jbang just looks stuck so I added printing of url on download + a retry line printed on retry. With the latest env options its now easier to test and validate before we go live, you can test it using: ensure no global jbang.jar available: run with specific url: or change just version: so output is now like this: Meaning retries are visible but also that url is shown on each download to user which in the past we kept hidden. @quintesse wdyt? would like second set of eyes before pushing this :) |
curl's --retry and wget's --tries flags operate silently when combined with -s/-q, giving no indication that retries are happening. This makes it impossible to diagnose download failures in the field. Replace the built-in retry delegation with a manual retry loop (matching the approach already used in jbang.ps1's Invoke-Download), providing: - Retry attempt feedback: 'Download attempt 1/6 failed, retrying in 1 second(s)...' - Exponential backoff by default (1s, 2s, 4s, 8s...), or fixed delay via JBANG_DOWNLOAD_RETRY_DELAY - URL shown on the initial download line for both bash and PS1 scripts Also adds JBANG_DOWNLOAD_URL support to resolve_download_url() (bash) and Resolve-DownloadUrl (PS1) as a full URL override, integrated with the native-aware install refactoring.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Max Rydahl Andersen <max.andersen@gmail.com>
|
fixed the failure in ps1 due to bad/different exit behaviour than tests expected. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/scripts/jbang.ps1 (1)
130-137:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe new ARM64 mapping is unreachable with the current architecture initialization.
Line 46 still hard-codes
$arch = 'x64', so this function never seesarm64on Windows ARM64 hosts. The wrapper will keep resolving x64 native bundles and x64 JDK downloads instead of the aarch64 variants described in the PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/scripts/jbang.ps1` around lines 130 - 137, Get-NativeBundleArch never sees "arm64" because $arch is currently hard-coded to 'x64'; fix by replacing the hard-coded $arch initialization with runtime detection (e.g., check $env:PROCESSOR_ARCHITECTURE or use WMI/Get-CimInstance to detect the OS/processor arch) so $arch can be set to the actual value ("ARM64"/"arm64"/"aarch64" or "AMD64"/"x86_64") before calling Get-NativeBundleArch; ensure the normalization in Get-NativeBundleArch still maps "arm64" to "aarch64" so Windows ARM64 hosts resolve aarch64 bundles and JDKs.
♻️ Duplicate comments (4)
src/main/scripts/jbang.ps1 (2)
188-197:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAvoid PowerShell 7 ternary syntax in a PowerShell 5 script.
#Requires -Version 5means these interpolations are parsed by Windows PowerShell 5.1, where? :is unsupported. As written, the script fails before it can run.Suggested fix
function Install-JBangBundle { param([string]$ReleaseVersion, [string]$NativeRequested) $bundlePath = "$TDIR\urls\jbang.zip" + $displayVersion = if ($ReleaseVersion) { $ReleaseVersion } else { "latest" } New-Item -ItemType Directory -Force -Path "$TDIR\urls" >$null 2>&1 if ($NativeRequested -eq "true") { $jburl = Get-NativeBundleUrl $ReleaseVersion - [Console]::Error.WriteLine("Downloading JBang $($ReleaseVersion ? $ReleaseVersion : 'latest') native bundle from $jburl...") + [Console]::Error.WriteLine("Downloading JBang $displayVersion native bundle from $jburl...") $ok = Invoke-Download "$jburl" $bundlePath if (-not $ok) { [Console]::Error.WriteLine("WARNING: Native JBang bundle not available from $jburl, falling back to generic bundle") $jburl = Get-FallbackBundleUrl $ReleaseVersion $ok = Invoke-Download "$jburl" $bundlePath } } else { $jburl = Get-FallbackBundleUrl $ReleaseVersion - [Console]::Error.WriteLine("Downloading JBang $($ReleaseVersion ? $ReleaseVersion : 'latest') from $jburl...") + [Console]::Error.WriteLine("Downloading JBang $displayVersion from $jburl...") $ok = Invoke-Download "$jburl" $bundlePath }Does Windows PowerShell 5.1 support the ternary operator (`condition ? expr1 : expr2`), or was it introduced in PowerShell 7?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/scripts/jbang.ps1` around lines 188 - 197, The string interpolation uses PowerShell 7 ternary syntax ($($ReleaseVersion ? $ReleaseVersion : 'latest')) which breaks on Windows PowerShell 5. Replace the inline ternary by computing a local variable (e.g., $ver) before the download calls: set $ver = if ($ReleaseVersion) { $ReleaseVersion } else { 'latest' } and then use $ver in the messages and when calling Get-FallbackBundleUrl/Invoke-Download (references: $ReleaseVersion, Invoke-Download, Get-FallbackBundleUrl).
246-249:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't redownload after a successful generic fallback.
With
JBANG_USE_NATIVE=true, a fallback tojbang.zipleavesjbang.bin.exeabsent by design. This check makes$needsInstalltrue again on every launch, so callers that rely on the bootstrap script keep reinstalling instead of reusing the JAR fallback.Suggested fix
- if ($env:JBANG_USE_NATIVE -eq "true" -and -not (Test-Path "$JBDIR\bin\$(Get-NativeExecName)")) { + if ($env:JBANG_USE_NATIVE -eq "true" ` + -and -not (Test-Path "$JBDIR\bin\$(Get-NativeExecName)") ` + -and (-not (Test-Path "$JBDIR\bin\jbang.jar") -or -not (Test-Path "$JBDIR\bin\jbang.ps1"))) { $needsInstall = $true }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/scripts/jbang.ps1` around lines 246 - 249, The launch loop currently forces reinstallation when JBANG_USE_NATIVE="true" if the native binary from Get-NativeExecName is missing even though a fallback JAR (jbang.jar) was successfully installed; update the native-check branch to only mark $needsInstall = $true when the native binary is missing AND the fallback JAR is also missing (i.e., add a Test-Path "$JBDIR\bin\jbang.jar" negative check to the condition). This change touches the $needsInstall variable and the if that checks $env:JBANG_USE_NATIVE and Get-NativeExecName so the script will reuse the JAR fallback instead of redownloading.src/main/scripts/jbang (2)
70-73:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRecognize normalized
x64here.Line 334 already normalizes
x86_64|amd64tox64, so this helper returns an empty arch on the common path and native mode always falls back to the generic bundle.Suggested fix
native_bundle_arch() { case "$arch" in - x86_64|amd64) + x86_64|amd64|x64) echo "x64" ;;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/scripts/jbang` around lines 70 - 73, The helper that computes a platform-specific arch is missing recognition for the already-normalized "x64" value, so when "x86_64|amd64" is normalized to "x64" earlier the helper returns empty and native mode falls back to generic; update the helper's case block (the one using pattern case "$arch" in and matching x86_64|amd64) to also accept "x64" (e.g., add x64 to the pattern alongside x86_64|amd64) so the normalized value returns the expected arch string instead of empty.
412-414:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't force a reinstall after a successful generic fallback.
If
JBANG_USE_NATIVE=truebut the native bundle was unavailable, a valid generic install still leaves$native_nameabsent. This block flipsneeds_installback totrueon every launch, so the bootstrap script keeps downloading instead of reusing the JAR fallback.Suggested fix
- if [[ "$JBANG_USE_NATIVE" == "true" && ! -f "$JBDIR/bin/$native_name" ]]; then + if [[ "$JBANG_USE_NATIVE" == "true" && ! -f "$JBDIR/bin/$native_name" \ + && ( ! -f "$JBDIR/bin/jbang.jar" || ! -f "$JBDIR/bin/jbang" ) ]]; then needs_install="true" fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/scripts/jbang` around lines 412 - 414, The current check sets needs_install="true" whenever JBANG_USE_NATIVE=="true" and the native binary "$JBDIR/bin/$native_name" is missing, which forces reinstall even when a working generic fallback is present; change the condition in that if (the one referencing JBANG_USE_NATIVE, JBDIR and native_name) to only set needs_install="true" if the native binary is missing AND there is no usable generic fallback (e.g. test that the fallback executable/jar such as "$JBDIR/jbang" or "$JBDIR/jbang.jar" is also missing); in short, add a second negative check for the generic fallback before flipping needs_install so an existing JAR fallback isn’t overridden.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/main/scripts/jbang.ps1`:
- Around line 130-137: Get-NativeBundleArch never sees "arm64" because $arch is
currently hard-coded to 'x64'; fix by replacing the hard-coded $arch
initialization with runtime detection (e.g., check $env:PROCESSOR_ARCHITECTURE
or use WMI/Get-CimInstance to detect the OS/processor arch) so $arch can be set
to the actual value ("ARM64"/"arm64"/"aarch64" or "AMD64"/"x86_64") before
calling Get-NativeBundleArch; ensure the normalization in Get-NativeBundleArch
still maps "arm64" to "aarch64" so Windows ARM64 hosts resolve aarch64 bundles
and JDKs.
---
Duplicate comments:
In `@src/main/scripts/jbang`:
- Around line 70-73: The helper that computes a platform-specific arch is
missing recognition for the already-normalized "x64" value, so when
"x86_64|amd64" is normalized to "x64" earlier the helper returns empty and
native mode falls back to generic; update the helper's case block (the one using
pattern case "$arch" in and matching x86_64|amd64) to also accept "x64" (e.g.,
add x64 to the pattern alongside x86_64|amd64) so the normalized value returns
the expected arch string instead of empty.
- Around line 412-414: The current check sets needs_install="true" whenever
JBANG_USE_NATIVE=="true" and the native binary "$JBDIR/bin/$native_name" is
missing, which forces reinstall even when a working generic fallback is present;
change the condition in that if (the one referencing JBANG_USE_NATIVE, JBDIR and
native_name) to only set needs_install="true" if the native binary is missing
AND there is no usable generic fallback (e.g. test that the fallback
executable/jar such as "$JBDIR/jbang" or "$JBDIR/jbang.jar" is also missing); in
short, add a second negative check for the generic fallback before flipping
needs_install so an existing JAR fallback isn’t overridden.
In `@src/main/scripts/jbang.ps1`:
- Around line 188-197: The string interpolation uses PowerShell 7 ternary syntax
($($ReleaseVersion ? $ReleaseVersion : 'latest')) which breaks on Windows
PowerShell 5. Replace the inline ternary by computing a local variable (e.g.,
$ver) before the download calls: set $ver = if ($ReleaseVersion) {
$ReleaseVersion } else { 'latest' } and then use $ver in the messages and when
calling Get-FallbackBundleUrl/Invoke-Download (references: $ReleaseVersion,
Invoke-Download, Get-FallbackBundleUrl).
- Around line 246-249: The launch loop currently forces reinstallation when
JBANG_USE_NATIVE="true" if the native binary from Get-NativeExecName is missing
even though a fallback JAR (jbang.jar) was successfully installed; update the
native-check branch to only mark $needsInstall = $true when the native binary is
missing AND the fallback JAR is also missing (i.e., add a Test-Path
"$JBDIR\bin\jbang.jar" negative check to the condition). This change touches the
$needsInstall variable and the if that checks $env:JBANG_USE_NATIVE and
Get-NativeExecName so the script will reuse the JAR fallback instead of
redownloading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d1d9981-449e-4ee2-a1ff-826467f894c7
📒 Files selected for processing (2)
src/main/scripts/jbangsrc/main/scripts/jbang.ps1
|
One thing I see is a difference in output between WIndows and Linux/Mac: "Error downloading JBang from notthere.jar to C:\Users\tako.jbang\cache\urls\jbang.zip" vs "Error downloading JBang from notthere.jar" |
|
The other thing is that when both JBANG_DOWNLOAD_VERSION and JBANG_DOWNLOAD_URL are defined we might want to prioritize the URL, right now it takes the VERSION. |
|
Also, we might want to check that the values aren't empty strings. On Powershell it was much easier for me to set the env vars to empty than to delete them and it tried to download from "". |
|
Didn't have time to figure out why exactly, but this doesn't work: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/step-ci-build.yml:
- Around line 298-307: The jreleaser-dry-run job should explicitly restrict
GITHUB_TOKEN permissions and disable credential persistence on checkout: add a
job-level permissions block to the jreleaser-dry-run job (e.g., permissions:
contents: read, actions: read, packages: write — adjust to the actual minimal
scopes you need for artifact download/upload) and set persist-credentials: false
on the actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd step to prevent
the token from being left in the workspace.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ed62319-b2da-4be9-bf3a-a37dd1338d2f
📒 Files selected for processing (1)
.github/workflows/step-ci-build.yml
| jreleaser-dry-run: | ||
| needs: [build-shared, build-test-native-image] | ||
| if: always() && needs.build-shared.result == 'success' | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| JRELEASER_VERSION: 1.19.0 | ||
| steps: | ||
| - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5 | ||
| with: | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/step-ci-build.yml"
echo "== File exists? =="
ls -l "$FILE" || exit 1
echo "== Show context around lines 280-360 =="
nl -ba "$FILE" | sed -n '280,360p'
echo "== Find all 'permissions:' occurrences in workflow =="
rg -n "^\s*permissions\s*:" "$FILE" || true
echo "== Find all actions/checkout uses in workflow =="
rg -n "uses:\s*actions/checkout@" "$FILE" -n || true
echo "== Show checkout step options near jreleaser-dry-run =="
# Extract the jreleaser-dry-run block roughly
python3 - <<'PY'
import re, itertools, pathlib
p=pathlib.Path(".github/workflows/step-ci-build.yml")
txt=p.read_text().splitlines()
start=None
for i,line in enumerate(txt):
if re.match(r'^\s*jreleaser-dry-run:\s*$', line):
start=i
break
if start is None:
print("jreleaser-dry-run not found")
raise SystemExit(0)
# print next 80 lines
for j in range(start, min(start+120, len(txt))):
print(f"{j+1:4d}: {txt[j]}")
PYRepository: jbangdev/jbang
Length of output: 264
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/step-ci-build.yml"
echo "== Show context around lines 280-360 =="
nl -ba "$FILE" | sed -n '280,360p'
echo "== Find all 'permissions:' occurrences in workflow =="
rg -n "^\s*permissions\s*:" "$FILE" || true
echo "== Find all actions/checkout uses in workflow =="
rg -n "uses:\s*actions/checkout@" "$FILE" || true
echo "== Extract jreleaser-dry-run block =="
python3 - <<'PY'
import re, pathlib
p=pathlib.Path(".github/workflows/step-ci-build.yml")
lines=p.read_text().splitlines()
start=None
for idx,l in enumerate(lines):
if re.match(r'^\s*jreleaser-dry-run:\s*$', l):
start=idx
break
if start is None:
print("jreleaser-dry-run not found")
raise SystemExit(0)
end=min(start+140,len(lines))
for i in range(start,end):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: jbangdev/jbang
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/step-ci-build.yml"
echo "== Show context around lines 280-360 =="
nl -ba "$FILE" | sed -n '280,360p'
echo "== Find all 'permissions:' occurrences in workflow =="
rg -n "^\s*permissions\s*:" "$FILE" || true
echo "== Find all actions/checkout uses in workflow =="
rg -n "uses:\s*actions/checkout@" "$FILE" || true
echo "== Extract jreleaser-dry-run block =="
python3 - <<'PY'
import re, pathlib
p=pathlib.Path(".github/workflows/step-ci-build.yml")
lines=p.read_text().splitlines()
start=None
for idx,l in enumerate(lines):
if re.match(r'^\s*jreleaser-dry-run:\s*$', l):
start=idx
break
if start is None:
print("jreleaser-dry-run not found")
raise SystemExit(0)
end=min(start+160,len(lines))
for i in range(start,end):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: jbangdev/jbang
Length of output: 159
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/step-ci-build.yml"
nl -ba "$FILE" | sed -n '280,360p'
rg -n "^\s*permissions\s*:" "$FILE" || true
rg -n "uses:\s*actions/checkout@" "$FILE" || true
python3 - <<'PY'
import re, pathlib
p=pathlib.Path(".github/workflows/step-ci-build.yml")
lines=p.read_text().splitlines()
start=None
for idx,l in enumerate(lines):
if re.match(r'^\s*jreleaser-dry-run:\s*$', l):
start=idx
break
if start is None:
raise SystemExit("jreleaser-dry-run not found")
end=min(start+160,len(lines))
for i in range(start,end):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: jbangdev/jbang
Length of output: 101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/step-ci-build.yml"
echo "== Context 280-360 =="
nl -ba "$FILE" | sed -n '280,360p'
echo "== Permissions occurrences =="
rg -n "^\s*permissions\s*:" "$FILE" || true
echo "== Checkout uses =="
rg -n "uses:\s*actions/checkout@" "$FILE" || true
echo "== jreleaser-dry-run block (full-ish) =="
python3 - <<'PY'
import re, pathlib
p=pathlib.Path(".github/workflows/step-ci-build.yml")
lines=p.read_text().splitlines()
start=None
for idx,l in enumerate(lines):
if re.match(r'^\s*jreleaser-dry-run:\s*$', l):
start=idx
break
if start is None:
print("not found")
raise SystemExit(0)
end=min(start+200,len(lines))
for i in range(start,end):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: jbangdev/jbang
Length of output: 141
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/step-ci-build.yml"
nl -ba "$FILE" | sed -n '280,360p'
rg -n "^\s*permissions\s*:" "$FILE" || true
rg -n "uses:\s*actions/checkout@" "$FILE" || true
python3 - <<'PY'
import re, pathlib
p=pathlib.Path(".github/workflows/step-ci-build.yml")
lines=p.read_text().splitlines()
start=None
for idx,l in enumerate(lines):
if re.match(r'^\s*jreleaser-dry-run:\s*$', l):
start=idx
break
if start is None:
print("jreleaser-dry-run not found")
raise SystemExit(0)
end=min(start+200,len(lines))
for i in range(start,end):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: jbangdev/jbang
Length of output: 101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE=".github/workflows/step-ci-build.yml"
echo "== permissions occurrences =="
rg -n "^\s*permissions\s*:" "$FILE" || true
echo "== actions/checkout occurrences =="
rg -n "uses:\s*actions/checkout@" "$FILE" || true
echo "== jreleaser-dry-run block with line numbers (approx) =="
python3 - <<'PY'
import pathlib, re
p=pathlib.Path(".github/workflows/step-ci-build.yml")
lines=p.read_text().splitlines()
start=None
for i,l in enumerate(lines):
if re.match(r'^\s*jreleaser-dry-run:\s*$', l):
start=i
break
if start is None:
raise SystemExit("jreleaser-dry-run not found")
end=min(len(lines), start+180)
for idx in range(start,end):
print(f"{idx+1:4d}: {lines[idx]}")
PYRepository: jbangdev/jbang
Length of output: 8433
Harden token exposure in the jreleaser-dry-run job (.github/workflows/step-ci-build.yml:298-307)
- This job has no explicit job-level
permissions:(currently none in the workflow), so scope theGITHUB_TOKENto least privilege—artifact download/upload in this job typically requires more than justcontents: read. - Disable credential persistence on the
actions/checkout@93cb6efe...step by settingpersist-credentials: false.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 305-307: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[warning] 298-342: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block
(excessive-permissions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/step-ci-build.yml around lines 298 - 307, The
jreleaser-dry-run job should explicitly restrict GITHUB_TOKEN permissions and
disable credential persistence on checkout: add a job-level permissions block to
the jreleaser-dry-run job (e.g., permissions: contents: read, actions: read,
packages: write — adjust to the actual minimal scopes you need for artifact
download/upload) and set persist-credentials: false on the
actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd step to prevent the
token from being left in the workspace.
On Windows (Git Bash/MSYS2/Cygwin), the downloaded bundle is a .zip file, but install_jbang_bundle() unconditionally used tar to extract it, causing 'This does not look like a tar archive' errors. Now uses unzip on Windows, consistent with the unpack() function used for JDK extraction.
37dbf30 to
28325ba
Compare
The jreleaser dry-run step in CI was failing because signing is configured as active: ALWAYS and the GPG/Maven Central env vars are not available in PR builds. Provide placeholder values since signing is not actually performed during dry-run.
The nativeDistZip/nativeDistTar tasks were eagerly evaluating
project.version via getNativeBundleBaseName() during task registration,
before the nisse afterEvaluate block had set the version. This caused
native bundles to be named jbang-unspecified-<os>-<arch> instead of
jbang-<version>-<os>-<arch>.
Using provider {} defers the evaluation until task execution, when the
version has been resolved.
Also require native image builds to succeed before running jreleaser
dry-run, and provide dummy signing env vars for dry-run.
CI (step-ci-build.yml): - Build native bundles (nativeDistZip/nativeDistTar) per platform - Upload native bundles as artifacts - Add jreleaser dry-run job to validate release process on PRs Release (tag-and-release.yml): - Reuse step-ci-build.yml as a called workflow for build/test/native bundles - Release job downloads artifacts from CI and runs JReleaser - Update jreleaser from 1.19.0 to 1.24.0 - Fix deprecated set-output to use GITHUB_OUTPUT Build (build.gradle): - Add nativeDistZip and nativeDistTar tasks - Add getNativeBundleOs/Arch/BaseName helpers for consistent naming - Fix commonSpec native image inclusion to be lazy (not config-time) JReleaser (jreleaser.yml): - Add native bundle artifacts (mac-aarch64, linux-x64, windows-x64) as optional release files (active: RELEASE, skipArtifact on missing) Split from #2467.
CI (step-ci-build.yml): - Build native bundles (nativeDistZip/nativeDistTar) per platform - Upload native bundles as artifacts - Add jreleaser dry-run job to validate release process on PRs Release (tag-and-release.yml): - Reuse step-ci-build.yml as a called workflow for build/test/native bundles - Release job downloads artifacts from CI and runs JReleaser - Update jreleaser from 1.19.0 to 1.24.0 - Fix deprecated set-output to use GITHUB_OUTPUT Build (build.gradle): - Add nativeDistZip and nativeDistTar tasks - Add getNativeBundleOs/Arch/BaseName helpers for consistent naming - Fix commonSpec native image inclusion to be lazy (not config-time) JReleaser (jreleaser.yml): - Add native bundle artifacts (mac-aarch64, linux-x64, windows-x64) as optional release files (active: RELEASE, skipArtifact on missing) Split from #2467.
CI (step-ci-build.yml): - Build native bundles (nativeDistZip/nativeDistTar) per platform - Upload native bundles as artifacts - Add jreleaser dry-run job to validate release process on PRs Release (tag-and-release.yml): - Reuse step-ci-build.yml as a called workflow for build/test/native bundles - Release job downloads artifacts from CI and runs JReleaser - Update jreleaser from 1.19.0 to 1.24.0 - Fix deprecated set-output to use GITHUB_OUTPUT Build (build.gradle): - Add nativeDistZip and nativeDistTar tasks - Add getNativeBundleOs/Arch/BaseName helpers for consistent naming - Fix commonSpec native image inclusion to be lazy (not config-time) JReleaser (jreleaser.yml): - Add native bundle artifacts (mac-aarch64, linux-x64, windows-x64) as optional release files (active: RELEASE, skipArtifact on missing) Split from #2467.
CI (step-ci-build.yml): - Build native bundles (nativeDistZip/nativeDistTar) per platform - Upload native bundles as artifacts - Add jreleaser dry-run job to validate release process on PRs Release (tag-and-release.yml): - Reuse step-ci-build.yml as a called workflow for build/test/native bundles - Release job downloads artifacts from CI and runs JReleaser - Update jreleaser from 1.19.0 to 1.24.0 - Fix deprecated set-output to use GITHUB_OUTPUT Build (build.gradle): - Add nativeDistZip and nativeDistTar tasks - Add getNativeBundleOs/Arch/BaseName helpers for consistent naming - Fix commonSpec native image inclusion to be lazy (not config-time) JReleaser (jreleaser.yml): - Add native bundle artifacts (mac-aarch64, linux-x64, windows-x64) as optional release files (active: RELEASE, skipArtifact on missing) Split from #2467.
CI (step-ci-build.yml): - Build native bundles (nativeDistZip/nativeDistTar) per platform - Upload native bundles as artifacts - Add jreleaser dry-run job to validate release process on PRs Release (tag-and-release.yml): - Reuse step-ci-build.yml as a called workflow for build/test/native bundles - Release job downloads artifacts from CI and runs JReleaser - Update jreleaser from 1.19.0 to 1.24.0 - Fix deprecated set-output to use GITHUB_OUTPUT Build (build.gradle): - Add nativeDistZip and nativeDistTar tasks - Add getNativeBundleOs/Arch/BaseName helpers for consistent naming - Fix commonSpec native image inclusion to be lazy (not config-time) JReleaser (jreleaser.yml): - Add native bundle artifacts (mac-aarch64, linux-x64, windows-x64) as optional release files (active: RELEASE, skipArtifact on missing) Split from #2467.
CI (step-ci-build.yml): - Build native bundles (nativeDistZip/nativeDistTar) per platform - Upload native bundles as artifacts - Add jreleaser dry-run job to validate release process on PRs Release (tag-and-release.yml): - Reuse step-ci-build.yml as a called workflow for build/test/native bundles - Release job downloads artifacts from CI and runs JReleaser - Update jreleaser from 1.19.0 to 1.24.0 - Fix deprecated set-output to use GITHUB_OUTPUT Build (build.gradle): - Add nativeDistZip and nativeDistTar tasks - Add getNativeBundleOs/Arch/BaseName helpers for consistent naming - Fix commonSpec native image inclusion to be lazy (not config-time) JReleaser (jreleaser.yml): - Add native bundle artifacts (mac-aarch64, linux-x64, windows-x64) as optional release files (active: RELEASE, skipArtifact on missing) Split from #2467.
CI (step-ci-build.yml): - Build native bundles (nativeDistZip/nativeDistTar) per platform - Upload native bundles as artifacts - Add jreleaser dry-run job to validate release process on PRs Release (tag-and-release.yml): - Reuse step-ci-build.yml as a called workflow for build/test/native bundles - Release job downloads artifacts from CI and runs JReleaser - Update jreleaser from 1.19.0 to 1.24.0 - Fix deprecated set-output to use GITHUB_OUTPUT Build (build.gradle): - Add nativeDistZip and nativeDistTar tasks - Add getNativeBundleOs/Arch/BaseName helpers for consistent naming - Fix commonSpec native image inclusion to be lazy (not config-time) JReleaser (jreleaser.yml): - Add native bundle artifacts (mac-aarch64, linux-x64, windows-x64) as optional release files (active: RELEASE, skipArtifact on missing) Split from #2467.
CI (step-ci-build.yml): - Build native bundles (nativeDistZip/nativeDistTar) per platform - Upload native bundles as artifacts - Add jreleaser dry-run job to validate release process on PRs Release (tag-and-release.yml): - Reuse step-ci-build.yml as a called workflow for build/test/native bundles - Release job downloads artifacts from CI and runs JReleaser - Update jreleaser from 1.19.0 to 1.24.0 - Fix deprecated set-output to use GITHUB_OUTPUT Build (build.gradle): - Add nativeDistZip and nativeDistTar tasks - Add getNativeBundleOs/Arch/BaseName helpers for consistent naming - Fix commonSpec native image inclusion to be lazy (not config-time) JReleaser (jreleaser.yml): - Add native bundle artifacts (mac-aarch64, linux-x64, windows-x64) as optional release files (active: RELEASE, skipArtifact on missing) Split from #2467.
CI (step-ci-build.yml): - Build native bundles (nativeDistZip/nativeDistTar) per platform - Upload native bundles as artifacts - Add jreleaser dry-run job to validate release process on PRs Release (tag-and-release.yml): - Reuse step-ci-build.yml as a called workflow for build/test/native bundles - Release job downloads artifacts from CI and runs JReleaser - Update jreleaser from 1.19.0 to 1.24.0 - Fix deprecated set-output to use GITHUB_OUTPUT Build (build.gradle): - Add nativeDistZip and nativeDistTar tasks - Add getNativeBundleOs/Arch/BaseName helpers for consistent naming - Fix commonSpec native image inclusion to be lazy (not config-time) JReleaser (jreleaser.yml): - Add native bundle artifacts (mac-aarch64, linux-x64, windows-x64) as optional release files (active: RELEASE, skipArtifact on missing) Split from #2467.
…2473) * build: update CI and release for native bundles and latest jreleaser CI (step-ci-build.yml): - Build native bundles (nativeDistZip/nativeDistTar) per platform - Upload native bundles as artifacts - Add jreleaser dry-run job to validate release process on PRs Release (tag-and-release.yml): - Reuse step-ci-build.yml as a called workflow for build/test/native bundles - Release job downloads artifacts from CI and runs JReleaser - Update jreleaser from 1.19.0 to 1.24.0 - Fix deprecated set-output to use GITHUB_OUTPUT Build (build.gradle): - Add nativeDistZip and nativeDistTar tasks - Add getNativeBundleOs/Arch/BaseName helpers for consistent naming - Fix commonSpec native image inclusion to be lazy (not config-time) JReleaser (jreleaser.yml): - Add native bundle artifacts (mac-aarch64, linux-x64, windows-x64) as optional release files (active: RELEASE, skipArtifact on missing) Split from #2467. * ci: retrigger CI * fix: use cleanInstallDist instead of rm -rf for native image install * fix: support platform-named native binaries in IntegrationManager, Wrapper and App IntegrationManager now resolves jbang.jar by directory instead of suffix replacement, so jbang-linux-x64.bin finds jbang.jar correctly. Wrapper and App checks now accept any .bin/.bin.exe suffix instead of hardcoded jbang.bin. * fix: update VersionIT regex to accept -SNAPSHOT suffix * fix: map arm64 to aarch64 in jbang script to match native binary naming macOS uname -m returns 'arm64' but Gradle names the binary with 'aarch64'. Merge the two cases so the script finds jbang-mac-aarch64.bin. * fix: include expected binary names in native binary not-found warnings * fix: disable signing in jreleaser dry-run to avoid keyring init failure * fix: correct jreleaser signing property prefix * build: use jreleaser early-access to fix signing issue * fix: use --yolo for jreleaser dry-run instead of manual signing overrides
|
Just tried 0.138.0.19 and many commands worked but a simple version with verbose turned on gave me: |
Overview
Adds native binary support to JBang wrapper scripts and install/update mechanisms, enabling faster startup and better performance when GraalVM native images are available.
Closes #2458
Core Features
JBANG_USE_NATIVE=truejbang-linux-x64.tar).newfile promotion mechanism for atomic updatesVersion Handling
JBANG_DOWNLOAD_VERSION/download/v0.138.0/jbang-linux-x64.tar)/latest/download/when version not specifiedConfiguration
New environment variables control wrapper behavior:
JBANG_USE_NATIVEfalsetrue, prefer native binary over JARJBANG_DOWNLOAD_BASEURLhttps://github.com/jbangdev/jbang/releasesJBANG_DOWNLOAD_VERSIONScript Changes
jbang(bash): Native bundle download/install functions,JBANG_USE_NATIVEsupport,.newfile promotion for native binariesjbang.ps1: Same native-aware logic ported to PowerShelljbang.cmd: Native binary detection and.newpromotion for Windows CMDJava Changes
Settings.java: SharedgetDownloadBaseUrl()respectingJBANG_DOWNLOAD_BASEURLApp.java: Native-awareinstallJBang()with platform detection and fallbackVersionChecker.java: Uses shared download base URL, fixed unreachable codeCode Quality Improvements
VersionChecker.retrieveLatestVersion()Stream.forEachwith for-loop (Java best practice for side effects)Arrays.asListfor Java 8 compatibilityDocumentation
installation.adocRelated PRs
fix:bash retry loop with user feedbackbuild:CI native bundles + jreleaser 1.24.0feat:native-aware wrapper (this PR)