Skip to content

fix: quote source build make arguments - #432

Open
jihtsan wants to merge 2 commits into
ekalinin:masterfrom
jihtsan:codex/430-quote-make
Open

jihtsan wants to merge 2 commits into
ekalinin:masterfrom
jihtsan:codex/430-quote-make

Conversation

@jihtsan

@jihtsan jihtsan commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

--source passes the make executable and its --jobs/--load-average arguments through a shell. Paths containing spaces fail to execute, and shell syntax in option values is interpreted instead of reaching make as an argument.

Quote the executable and each generated option with the existing _quote helper for both build and install. The regression invokes real stub configure/make executables, records argv, and checks that shell payloads do not create files. It covers ordinary, empty, and omitted options, paths with spaces and special characters, semicolons, and command substitution. Its argument fixture specifies the Node version so it also works with the current upstream source-build version checks.

Validation on macOS / Python 3.13.12:

  • python -m pytest tests/test_source_build.py -q — all 15 focused cases passed; the original shell-quoting implementation failed 12 cases.
  • Merged the PR changes locally with upstream default 24ca78c (including fix: do not require python2 to build node from source #429), then ran python -m pytest -m 'not integration' tests -q: 412 passed, 10 skipped, 5 deselected, 2 expected failures, and 1 unrelated failure. The failure is test_mirror_credentials_stay_out_of_error_messages receiving HTTP 503 in this environment; running that test on unchanged upstream 24ca78c reproduces the same failure. All 15 quoting regressions pass in the combined tree.
  • Flake8 for tests/test_source_build.py and git diff --check — passed.

The merged-tree check validates compatibility without adding an upstream merge commit to this branch. Actual Node compilation and Windows runtime behavior were not tested locally.

Closes #430.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--source passes --make, --jobs and --load-average to the shell unquoted

1 participant