Uh oh!
There was an error while loading. Please reload this page.
fix(installer): repair five Windows installer defects (#233-#237) - #242
Merged
Conversation
All five were filed reviewing PR #232 and are Windows-only: install.sh bootstraps no JDK, so it shares none of these paths. #233 Resolve-Java never probed <install>\jdk, so every re-run downloaded the ~180 MB Temurin zip again and unpacked it over a perfectly good JDK. It is now step 1 — the same order both generated launchers implement — and a bundled JDK below the floor is REPLACED rather than worked around with JAVA_HOME, because the launcher prefers <install>\jdk unconditionally. Reuse and bootstrap set identical state through one Use-EmbeddedJdk helper. #234 Install-EmbeddedJdk removed the existing JDK before its replacement existed, so a failed unpack (running out of disk lands squarely in that window) left a previously working install with no JVM at all. Now: stage, assert bin\java.exe in the staged tree, and only then delete and move. #235 the generated .bat computed JAVA_MAJOR and spent it only on the --add-opens decision, so a stale JAVA_HOME - which outranks the PATH here, deliberately, because it is the JVM the installer probed - silently won and the REPL died with UnsupportedClassVersionError. It now refuses a JVM below %REQUIRED_JAVA%, guarded by `not "0"` for parity with the .ps1 launcher: refuse only a version positively read as too low, never one it failed to parse. Both launchers now name JAVA_HOME as the likely cause. #236 a bad -Version was only discovered by Download-Jar, after ~300 MB of JDK had landed in $Target, and nothing in the failure output mentioned it. Test-RequestedVersion now settles the inputs before Resolve-Java; listings are memoised (successes only) so it costs no extra HTTP, and $script:LastListingError preserves the cause -Quiet swallows. For the residual paths the main flow is wrapped in try/finally - PowerShell runs finally on `exit`, preserving the exit code - which names the JDK left behind. The JDK is KEPT on purpose: with #233 the next attempt reuses it. #237 install.cmd fetched install.ps1 from refs/heads/main, unverified, into a fixed %TEMP% path it never cleaned up. It now fetches a pinned release tag (PS1_REF), verifies it against a pinned SHA-256 with certutil before powershell.exe ever sees the file, and downloads into a fresh %RANDOM% directory removed at a single exit point. Overridable with SOFTCLIENT4ES_INSTALL_REF / _SHA256. Verified without a Windows host: 37 checks driving the real function bodies out of install.ps1's AST (JDK reuse/replace/fallthrough/bootstrap ordering, stage-then-swap under a throwing and a malformed unpack, the generated .bat and .ps1 text, and the top-level try/finally executed from its own extent), 15 static checks on install.cmd including refetching the pinned tag to confirm the pinned SHA-256, and real runs against JFrog (-Help, -ListVersions, a bogus -Version, a missing Scala variant, and an end-to-end install with a JDK pre-placed at <Target>\jdk). Not covered: cmd.exe executing install.cmd or the generated .bat, certutil output parsing on real Windows, and Expand-Archive on a real Temurin zip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fupelaqu
marked this pull request as ready for review
August 20, 2026 18:20
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the five defects filed against PR #232. All are Windows-only —
install.shbootstraps no JDK, so it shares none of these paths.Closes#233
Closes#234
Closes#235
Closes#236
Closes#237
What changed
install.ps1<install>\jdkis now step 1 ofResolve-Java. It never probed the JDK a previous run had bundled, so every re-run (upgrade,-NoExtensions, retry after a failed download) re-downloaded the ~180 MB Temurin zip over a perfectly good JDK. The order now matches the one both generated launchers document. A bundled JDK below the floor is replaced, not worked around withJAVA_HOME— the launcher prefers<install>\jdkunconditionally, so leaving a stale one there would put "the installer worked" and "the REPL starts" back into disagreement. Reuse and bootstrap set identical state through one newUse-EmbeddedJdkhelper.Install-EmbeddedJdkdeleted the existing JDK before unpacking its replacement; a failed unpack (running out of disk is squarely in that window — 180 MB zip + ~300 MB expanded + ~309 MB jar in one tree) left a previously working install with no JVM at all. It now unpacks to staging, assertsbin\java.exein the staged tree, and only then removes the old tree and moves the new one in.-Version, a missing artifact for the chosen-EsVersion/-ScalaVersion, or an unreachable repository was only discovered byDownload-Jar, after ~300 MB of JDK had landed in$Target. NewTest-RequestedVersionpre-flight runs beforeResolve-Java;Get-AvailableVersionsis memoised (successes only) so it costs no extra HTTP, and$script:LastListingErrorpreserves the cause-Quietswallows. For the residual paths the main flow is wrapped intry/finally— PowerShell runsfinallyonexitand preserves the exit code — which names the JDK left in$Target. The JDK is kept on purpose: with install.ps1: Resolve-Java never probes the JDK it bootstrapped, so every re-run re-downloads ~180 MB #233 the next attempt reuses it, and deleting could destroy a JDK a previous install depends on.Generated launchers
.batnow enforces the Java floor. It computedJAVA_MAJORand spent it only on the--add-opensdecision, so a stale%JAVA_HOME%(which outranks the PATH here, deliberately, because it is the JVM the installer probed) silently won and the REPL died withUnsupportedClassVersionError— an error naming a class-file version, which reads as a broken install. Guarded byif not "%JAVA_MAJOR%"=="0"for parity with the.ps1launcher: refuse only a version positively read as too low, never one it failed to parse. Both launchers now nameJAVA_HOMEas the likely cause.install.cmdinstall.ps1fromrefs/heads/main, unverified, into a fixed%TEMP%path it never removed, then ran it with-ExecutionPolicy Bypass. It now fetches a pinned release tag, verifies it against a pinned SHA-256 withcertutilbeforepowershell.exesees the file, and downloads into a fresh%RANDOM%directory removed at a single exit point. Overridable withSOFTCLIENT4ES_INSTALL_REF(+SOFTCLIENT4ES_INSTALL_SHA256); setting the ref alone drops the check and says so. Still a wrapper — no installer logic was added.New release ritual.
install.cmdcarries two constants that must be bumped together at every release:A mismatch is a hard failure, so a bumped tag with a stale hash breaks the fallback loudly for every
install.cmd-only user. That is the intended failure mode (loud, never "runs something unverified"), but it is a new obligation for the release checklist.Accepted trade-off:
main'sinstall.cmdnow fetches the last release's installer. That is the point of pinning — a released.cmdmust never silently pullmain— but it means these very fixes reach the.cmd-only path at the next release, not at merge. Escape hatches:set SOFTCLIENT4ES_INSTALL_REF=main, or putinstall.ps1next to the.cmd(a local copy still always wins). Say the word if you would rather default tomainand have the packaging step substitute the tag instead.Verification
No Windows host here, so, precisely:
install.ps1's AST (Parser::ParseFile→FindAll(FunctionDefinitionAst)→Invoke-Expression), so the shipped text is what runs, not a transcription: JDK reuse / stale-replace / fall-through / bootstrap ordering with fakejava.exeversion banners;Install-EmbeddedJdkwith stubbedInvoke-WebRequest/Expand-Archiveunder a throwing unpack, a malformed unpack and a good one (asserting the pre-existing JDK survives the first two); the generated.bat/.ps1text; and the top-leveltry/finallyexecuted from its own AST extent with the step functions stubbed.install.cmd: label/gotoclosure, subroutines after the finalexit /b, single cleanup point, RC defaults to failure, the one-parse-pass rule (no%VAR%read after asetin the same parenthesised block), CRLF + ASCII, and refetching the pinned tag to confirm the pinned SHA-256 still matches byte for byte.-Help;-ListVersions; a bogus-Version(fails before Java resolution and writes nothing); a missing Scala variant (now reports the underlying 404); and an end-to-end install with a JDK pre-placed at<Target>\jdk→Java 17 found via bundled JDK … — nothing to download.cmd.exeexecutinginstall.cmdor the generated.bat,certutiloutput parsing on real Windows, andExpand-Archiveon a real Temurin zip.🤖 Generated with Claude Code