fix: properly use nodejs_jll on windows - #113
Merged
Merged
Conversation
NodeJS_22_jll exports `npm`/`npx` as FileProducts pointing at bin/npm and
bin/npx, but those are not the same kind of file on every platform:
- POSIX: the npm CLI's JavaScript, carrying a `#!/usr/bin/env node` shebang, so
the kernel runs them directly. The command is unchanged from before, so there
is nothing to regress there.
- Windows: bash scripts whose own header says they are "used by the Node.js
installer, which expects the cygwin/mingw shell". CreateProcess cannot spawn
those, so every pagefind build fails there with
IOError: could not spawn `...\bin\npx pagefind -V`: unknown error (UNKNOWN)
Windows artifacts ship bin/npx.cmd alongside for exactly this, and a batch file
has to go through cmd.exe. Rather than a bare `cmd /c <path>`, which mis-parses
paths containing spaces, this uses the form Base documents for the purpose (see
Base.shell_escape_wincmd): `cmd.exe /S /C "<line>"` with windows_verbatim, where
/S makes cmd.exe strip the outer quote pair and take the rest verbatim. So
cmd.exe /S /C ""C:\Users\John Doe\bin\npx.cmd" pagefind -V"
reaches the program with its quoting intact. windows_verbatim is what stops Julia
from re-quoting the line we assembled ourselves. Note that cmd.exe expands %VAR%
before any escaping is considered, so a % in an argument can still corrupt the
command line; none of the paths involved normally contain one.
Arguably NodeJS_22_jll should point at the .cmd on Windows itself -- the artifact
ships it -- but this does not wait on that being fixed upstream.
The pagefind run now also uses dir = root, like the version probe and the install
already did, so npx resolves pagefind from the node_modules that
`npm install pagefind` populated rather than from the ambient working directory.
Verified on Linux: a full PageFind build indexes and writes its output, and
`npm install pagefind` followed by the npx probe both succeed. The command each
platform builds is unit tested, gated on the platform that can run it, so the
Windows assertions need Windows CI to execute.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pfitzseb
force-pushed
the
sp/pagefind-node-cli
branch
from
August 19, 2026 15:48
c41c429 to
c997b13
Compare
Member
Author
|
Should probably add the |
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
2 participants
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.
No description provided.