#35356 ci: Test build dir with spaces
| Reviewability | Reviews | Agreement | Size | |||||||
|---|---|---|---|---|---|---|---|---|---|---|
| #35356 ci: Test build dir with spaces | Author silent 76d | 0 | Mild | 13 | ||||||
|
|
|
|
| ||||||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | ||||||||||
Goal
This pull request configures Windows CI in `.github/ci-windows.py` to use a build directory with spaces (`build_ _`) and sets a dedicated `VCPKG_INSTALLED_DIR`. This ensures that path-handling logic and subprocess invocation on Windows are exercised against paths containing spaces.
Problem: CI previously did not test build directories with spaces on Windows, allowing path-quoting and subprocess-handling bugs (such as external signer invocation in functional tests) to slip through undetected.
Reviewability
Stale · Author silent 76d
No author activity for 76 days; a review may wait a long time for a response.
Agreement
Mild
objection that needs a change was answered, no reply from the objector (fanquake: wallet_signer fails in CI when the build directory path contains spaces)
Thread notes: hebasto Concept ACKed. fanquake reported a CI failure in wallet_signer with spaces in the build path, which maflcko noted is an expected reproduction of an existing bug; the issue remains unresolved.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| fanquake (member) | objection, correctness | wallet_signer fails in CI when the build directory path contains spaces | answered · reply on record; standing not assessed yet · v1, not yet re-checked | changed | yes | 2026-05-22: 'can't open file 'D:\\a\\bitcoin\\bitcoin\\build_': [Errno 2] No such file or directory' |
Support:
- hebasto (member) (Concept ACK, 2026-05-22): Concept ACK. [not substantive]
Participants:
- fanquake (member, objection, 1 statements 2026-05-22 to 2026-05-22): posted CI traceback showing wallet_signer failure on Windows
- hebasto (member, support, 1 statements 2026-05-22 to 2026-05-22): Concept ACK
Files
0 lines under test/bench/ci.
- .github/ci-windows.py +13/-4
Card
The short text the ranking pass compares across PRs.
This PR modifies Windows CI (.github/ci-windows.py) to build in a directory with spaces (build_ _) and sets VCPKG_INSTALLED_DIR. The goal is to detect bugs in subprocess and path handling on Windows that do not get caught on Linux due to symlink resolution. The PR immediately triggered a known bug in wallet_signer.py where spaces in the mock signer path fail execution, leaving CI red. hebasto provided a Concept ACK, but the PR remains a draft awaiting a fix for the underlying path-handling bug.
Data
thread read through 2026-08-17 (4 statements), head 474afb8509
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Tests, CI failed · draft
Categories
Build and CI
P3 · test coverage
Configuring Windows CI to build in paths with spaces (`build_ _`) exercises Windows path-handling and quoting logic to catch platform-specific regressions.
Membership: Modifies `.github/ci-windows.py` to change CI build directory paths and environment variables.
Factors: security/stability 0, bug 1, performance 0, user value 0, leverage 1
Test infrastructure
P3 · test coverage
Testing build paths with spaces expands test harness coverage on Windows, already catching issues such as wallet_signer failures when space characters are present.
Membership: Carries the Tests area label and alters test execution configuration to run tests out of paths containing spaces.
Factors: security/stability 0, bug 1, performance 0, user value 0, leverage 1