From c997b1332e69b52bea641fe291ce6e05aa9a5ad8 Mon Sep 17 00:00:00 2001 From: Sebastian Pfitzner Date: Wed, 19 Aug 2026 15:19:23 +0000 Subject: [PATCH] Run npm/npx the way each platform needs 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 `, which mis-parses paths containing spaces, this uses the form Base documents for the purpose (see Base.shell_escape_wincmd): `cmd.exe /S /C ""` 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) --- src/search/pagefind.jl | 38 ++++++++++++++++++++--- test/pagefind.jl | 69 ++++++++++++++++++++++++++++++++++++++++++ test/runtests.jl | 9 +++++- 3 files changed, 110 insertions(+), 6 deletions(-) create mode 100644 test/pagefind.jl diff --git a/src/search/pagefind.jl b/src/search/pagefind.jl index 7d86eca1..bdb36acc 100644 --- a/src/search/pagefind.jl +++ b/src/search/pagefind.jl @@ -2,6 +2,25 @@ module PageFind using NodeJS_22_jll: npx, npm, node using HypertextLiteral: @htl +""" + npm_command(shim, args...; dir) -> Cmd + +Command that runs one of NodeJS jll's `npm` / `npx` file products with `args`, in `dir`. +""" +function npm_command( + shim::AbstractString, + args::AbstractString...; + dir::AbstractString + ) + Sys.iswindows() || return Cmd(`$(shim) $(String[args...])`; dir = dir) + + wincmd_arg(arg::AbstractString) = + Base.shell_escape_wincmd(occursin(' ', arg) ? "\"$(arg)\"" : arg) + + line = join((wincmd_arg(arg) for arg in (shim * ".cmd", args...)), ' ') + return Cmd(Cmd(["cmd.exe", "/S /C \"$(line)\""]); windows_verbatim = true, dir = dir) +end + function inject_script!(custom_scripts, rootpath) pushfirst!(custom_scripts, joinpath("assets", "default", "pagefind_integration.js")) pushfirst!(custom_scripts, joinpath("pagefind", "pagefind.js")) @@ -34,9 +53,9 @@ function build_search_index(root, docs, config, rootpath) # To fix this, we wrap all uses of npx and npm inside `node() do ...` # which will automatically adjust the necessary environment variables. node() do _ - if !success(Cmd(`$(npx) pagefind -V`; dir = root)) + if !success(npm_command(npx, "pagefind", "-V"; dir = root)) @info "Installing pagefind into $root." - if !success(Cmd(`$(npm) install pagefind`; dir = root)) + if !success(npm_command(npm, "install", "pagefind"; dir = root)) error("Could not install pagefind.") end end @@ -44,10 +63,19 @@ function build_search_index(root, docs, config, rootpath) pattern = "*/{$(join(config.index_versions, ","))}/**/*.{html}" out_path = joinpath(root, "pagefind") - mktempdir() do dir + mktempdir() do sitedir # pagefind doesn't look at symlinks, so we resolve them here: - cp(root, dir; follow_symlinks = true, force = true) - run(`$(npx) pagefind --site $(dir) --output-path $(out_path) --glob $(pattern) --root-selector article`) + cp(root, sitedir; follow_symlinks = true, force = true) + run( + npm_command( + npx, "pagefind", + "--site", sitedir, + "--output-path", out_path, + "--glob", pattern, + "--root-selector", "article"; + dir = root, + ) + ) end end diff --git a/test/pagefind.jl b/test/pagefind.jl new file mode 100644 index 00000000..7c553673 --- /dev/null +++ b/test/pagefind.jl @@ -0,0 +1,69 @@ +using Test +using MultiDocumenter +using NodeJS_22_jll: npm, npx + +const PageFind = MultiDocumenter.PageFind + +@testset "npm_command" begin + @testset "posix runs the shim directly" begin + if !Sys.iswindows() + # bin/npm and bin/npx are the npm CLI's JavaScript with a `#!/usr/bin/env node` + # shebang there, so they need no interpreter of their own. + cmd = PageFind.npm_command( + "/js/bin/npx", "pagefind", "-V"; dir = "/root" + ) + @test cmd == Cmd(`/js/bin/npx pagefind -V`; dir = "/root") + @test cmd.exec == ["/js/bin/npx", "pagefind", "-V"] + @test cmd.dir == "/root" + + # a space in a path is Julia's to escape here, not ours + spaced = PageFind.npm_command( + "/js b/npx", "install", "pagefind"; dir = "/r" + ) + @test spaced.exec == ["/js b/npx", "install", "pagefind"] + end + end + + @testset "windows goes through cmd.exe /S /C" begin + if Sys.iswindows() + cmd = PageFind.npm_command( + "C:\\js\\bin\\npx", "pagefind", "-V"; dir = "C:\\root" + ) + # the .cmd sibling, wrapped in the form Base documents for cmd.exe + @test cmd == Cmd( + Cmd(["cmd.exe", "/S /C \"C:\\js\\bin\\npx.cmd pagefind -V\""]); + windows_verbatim = true, dir = "C:\\root", + ) + # windows_verbatim has to be set, or Julia would re-quote the line we assembled + @test cmd != Cmd( + Cmd(["cmd.exe", "/S /C \"C:\\js\\bin\\npx.cmd pagefind -V\""]); + dir = "C:\\root", + ) + @test cmd.dir == "C:\\root" + + # /S strips the outer quote pair, so a path with a space keeps its own quotes and + # reaches the program intact -- the case a bare `cmd /c $path` gets wrong + spaced = PageFind.npm_command( + "C:\\Users\\John Doe\\bin\\npx", "pagefind", "-V"; dir = "C:\\r" + ) + @test last(spaced.exec) == + "/S /C \"\"C:\\Users\\John Doe\\bin\\npx.cmd\" pagefind -V\"" + + # the glob we pass contains none of cmd.exe's metacharacters, so it survives as-is + glob = "*/{stable,dev}/**/*.{html}" + globbed = PageFind.npm_command( + "C:\\js\\bin\\npx", "pagefind", "--glob", glob; dir = "C:\\r" + ) + @test occursin("--glob $(glob)", last(globbed.exec)) + end + end + + @testset "the shims this platform needs exist" begin + @test isfile(npx) + @test isfile(npm) + if Sys.iswindows() + @test isfile(npx * ".cmd") + @test isfile(npm * ".cmd") + end + end +end diff --git a/test/runtests.jl b/test/runtests.jl index 1d062f7c..9fea2a90 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -5,6 +5,10 @@ using Test include("documentertools.jl") end +@testset "pagefind" begin + include("pagefind.jl") +end + clonedir = mktempdir() outpath = joinpath(@__DIR__, "out") rootpath = "/MultiDocumenter.jl/" @@ -145,7 +149,10 @@ MultiDocumenter.make( @test isfile(outpath, "inf", "stable", "index.html") end - @test read(joinpath(outpath, "inf", "index.html"), String) == """ + # Git may check out the cloned docs with CRLF line endings (e.g. on Windows, + # where core.autocrlf defaults to true), so we normalize before comparing. + index_html = normalize_newlines(read(joinpath(outpath, "inf", "index.html"), String)) + @test index_html == """ """