Merged
Conversation
The Windows installer had drifted far behind install.sh. Most seriously, both generated launchers ran `java -jar`, which ignores the classpath entirely, so every extension jar in lib/ was invisible to the JVM and the cross-index JOIN engine was silently absent on Windows -- the same silent-wrong-results failure class as #157. install.sh:1028 carries an explicit comment forbidding -jar. Launchers (.bat and .ps1) now mirror install.sh's invocation: -cp "<jar>;<base>\lib\*" app.softnetwork.elastic.client.Cli plus --add-opens=java.base/java.nio=ALL-UNNAMED and -Dio.netty.tryReflectionSetAccessible=true on Java 9+, which Arrow/DuckDB need. Ported from install.sh: the -all bundle path (bundle listing owns its own version line, exact-version match, HEAD existence probe, fallback to the plain artifact), licence-bundle extraction (licenses/ + NOTICE, failure-tolerant), the bundle vs pure-Apache licence notice, and -NoExtensions. The Java floor for ES 6/7/8 goes 8 -> 11: the 0.20+ CLI bundles logback 1.5.x (Java-11 bytecode) and does not start on 8. Also fixed, found while testing: - Resolve-LatestVersion applied [-1] to a pipeline result that is a bare String when only one release version exists, returning its last CHARACTER ("0.20.2" -> "2") and then failing the download. Pre-existing; @()-wrapped. - The .ps1 launcher passed a multi-flag JAVA_OPTS to the JVM as ONE argument. Now split on whitespace, with empty elements dropped (a leading space would otherwise emit an empty argv entry, which java reads as the main class). - Every fallback lands on the plain engine, and this script has no coursier resolution to populate lib/, so it now says outright that JOIN and materialized views are absent on that path instead of leaving the user to rediscover #179's symptom. - java's exit status is now propagated from the .ps1 launcher (install.sh gets this from `exec java`); logback's -D is quoted in the .bat (the default target lives under %USERPROFILE%, routinely containing a space); the 309 MB download suppresses $ProgressPreference (per-chunk Write-Progress makes a download this size look hung on PowerShell 5.1); licence extraction skips path-traversal entries; the licence status is repeated in the summary and written into VERSION. Verified on PowerShell 7.4.5 against a local mirror of the artifact repo: AST parse clean; bundle install resolves 0.20.2, extracts 3 licences + NOTICE; -NoExtensions installs the plain jar with no licences and the Apache-only notice; -ListVersions lists the bundle line by default and the plain line with -NoExtensions; a bundle-miss warns and falls back. The launcher's exact java invocation (macOS ':' for Windows ';') starts the CLI and a real cross-index JOIN returns the correct 5 rows against live ES 8.18.3. NOT verifiable here (no Windows host): cmd.exe execution of the generated .bat and Windows PowerShell 5.1. See the PR for the manual checklist. Closed Issue #179 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 31, 2026
Contributor
Author
|
The Windows verification checklist from this PR's description is now tracked as #181 — it is a merge blocker for this PR. Everything in it is the part macOS + PowerShell 7.4.5 structurally cannot cover: |
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 free
to 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.
Closes #179.
What was wrong
install.ps1was not a port of the current installer — it was roughly the pre-bundle one. Two classes of problem:Functional. Both generated launchers ran
java -jar, which ignores the classpath entirely, so every extension jar inlib/was invisible to the JVM and the cross-index JOIN engine was silently absent on Windows — the same silent-wrong-results failure class as #157. install.sh:1028 carries an explicit comment forbidding exactly this.Missing surface. Grep counts,
install.shvsinstall.ps1before this PR:-allbundle path 67 hits vs 0, licence extraction 8 vs 0,--add-opens1 vs 0,--no-extensions14 vs 0. Soinstall.ps1could not install the artifact the README's one-liner advertises.What this does
Launchers now mirror install.sh:
-cp "<jar>;<base>\lib\*" app.softnetwork.elastic.client.Cli, plus--add-opens=java.base/java.nio=ALL-UNNAMEDand-Dio.netty.tryReflectionSetAccessible=trueon Java 9+ (Arrow/DuckDB need them).Ported: the bundle path (bundle listing owns its own version line, exact-version match, HEAD existence probe, fallback to plain), licence-bundle extraction (
licenses/+NOTICE, failure-tolerant), the bundle vs pure-Apache licence notice,-NoExtensions. Java floor for ES 6/7/8 raised 8 → 11 (the 0.20+ CLI bundles logback 1.5.x, Java-11 bytecode — it does not start on 8).Bugs found while testing and fixing
Resolve-LatestVersionreturned"2"instead of"0.20.2". With exactly one release version the pipeline yields a bareString, and[-1]on a string returns its last character. Pre-existing; only manifests when a listing has one release.@()-wrapped.java -versionparsed three different ways (installer,.ps1launcher,Check-Prerequisites), none taking the first line only. With two matching lines the pipeline is anObject[]whoseToString()is"System.Object[]", no regex matches, and the version silently reads 0 — which would drop--add-openswith no message. Now oneGet-JavaMajorVersion,Select-Object -First 1.JAVA_OPTSreached the JVM as one argument; now split, with empty elements dropped (a leading space would emit an empty argv entry, which java reads as the main class name and refuses to start).lib/, so it now says so outright rather than leaving the user to rediscover install.ps1 has drifted behind install.sh: Windows launchers use java -jar (lib/ extensions ignored), no bundle/licence/AppCDS/--no-extensions support #179's own symptom..ps1launcher (install.sh gets it fromexec java); logback's-Dis quoted in the.bat(the default target is under%USERPROFILE%, routinely containing a space); the 309 MB download sets$ProgressPreference = 'SilentlyContinue'(per-chunkWrite-Progressmakes a download this size look hung on 5.1); licence extraction skips path-traversal entries; licence status is repeated in the summary and written intoVERSION.Review caught two HIGH-severity bugs in my first batch attempt
Worth recording, because the batch file is the least testable part. My first version did:
cmd expands every
%VAR%in a parenthesised block in one parse pass, before executing any line in it. Withoutenabledelayedexpansionthe quote-strip was dead code and the innerFORsaw the still-quoted value, soJAVA_MAJORbecame"11and the next line was a cmd syntax error — dropping the very--add-opensflags this PR exists to add. Separately,if defined JAVA_MAJOR if %JAVA_MAJOR% GEQ 9does not guard anything, because%JAVA_MAJOR%is expanded at parse time; when empty it degenerates toif GEQ 9, a parse error that aborts the whole script so the REPL never starts.Both fixed:
JAVA_MAJORis initialised to0,%%~vstrips the quotes, the secondFORmoved out of the block, and theif definedguard is gone. A per-iterationif not defined JVERalso gives first-match-wins parity with install.sh'shead -n 1.Verification (PowerShell 7.4.5, local mirror of the artifact repo)
-allbundle 0.20.2, downloads, extracts 3 licences + NOTICE-NoExtensionslicenses/, Apache-only notice, correct version-ListVersions-NoExtensions-Version 0.19.0):substituted for Windows;).batstructureJAVA_MAJOR=0present,%%~vused, secondFORoutside the block, noif defined JAVA_MAJOR if,LOGBACK_OPTSquotedNOT verified — needs a Windows reviewer before merge
I have no Windows host, so these are reasoned-and-structurally-checked, not executed:
cmd.exerunning the generatedbin\softclient4es.bat. ConfirmJAVA_MAJORis a bare integer and--add-opensactually reaches the JVM (temporaryecho %EXTRA_OPTS%); run withjavaabsent from PATH; run from a path containing a space (C:\Program Files\...and the defaultC:\Users\<name with space>\...); pass an argument containing spaces and one containing".bin\softclient4es.ps1under 5.1 with$env:JAVA_OPTSset (single flag, multi flag, leading space) and a double-quoted SQL identifier in-c(5.1's native argument quoting mangles embedded"; 7.3+ does not).irm … | iex(the README's Windows entry point) against theparam()+exitpaths — anexitinsideiexterminates the host session.Deliberately out of scope
AppCDS is not ported (install.sh:999-1018, 1050-1084) — a pure cold-start optimisation whose JDK-version gating is the fiddliest part; Windows works correctly without it. Also absent: the
ELASTIC_*empty-env-unset loop, benign on Windows since setting an env var to""deletes it there. Worth a follow-up issue for AppCDS if Windows cold start matters.Please also confirm whether Windows is a supported R1 install target — if it is, the
-jardefect this PR fixes was release-blocking for those users.