ci: Test build directory path with spaces #35746

hebasto commented at 1:19 PM on July 19, 2026: member

The CI scratch directory contains a space and non-ASCII symbols to test path handling (see #34614). However, the GHA workflows override BASE_BUILD_DIR to ${{ runner.temp }}/build via the configure-environment action, bypassing the $BASE_SCRATCH_DIR/build-$HOST default from 03_test_script.sh.

The first commit fixes the NSIS template, which otherwise breaks the deploy target when paths contain spaces.

Related to #35356.

DrahtBot added the label Tests on Jul 19, 2026

DrahtBot commented at 1:20 PM on July 19, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35746.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Stale ACK	l0rinc, maflcko

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#35537 (guix: split builds into Linux, Linux GUI and macOS/Windows by fanquake)
#25573 (guix: produce a -static-pie bitcoind by fanquake)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in .github/actions/configure-environment/action.yml:12 in a668d7a893 outdated

   6 | @@ -7,7 +7,9 @@ runs:
   7 |        shell: bash
   8 |        run: |
   9 |          echo "BASE_ROOT_DIR=${{ runner.temp }}" >> "$GITHUB_ENV"
  10 | -        echo "BASE_BUILD_DIR=${{ runner.temp }}/build" >> "$GITHUB_ENV"
  11 | +        # The space and non-ASCII symbols are intentional.
  12 | +        # See BASE_SCRATCH_DIR in ci/test/00_setup_env.sh.
  13 | +        echo "BASE_BUILD_DIR=${{ runner.temp }}/build_ ₿🧪_" >> "$GITHUB_ENV"

l0rinc commented at 4:36 PM on July 19, 2026:

nit: do we need both underscore and space?

        echo "BASE_BUILD_DIR=${{ runner.temp }}/build ₿ 🧪" >> "$GITHUB_ENV"

hebasto commented at 4:44 PM on July 19, 2026:

It simply matches this code: https://github.com/bitcoin/bitcoin/blob/18c05d93016b28a9afd4c716dfe00b6e0accb30b/ci/test/00_setup_env.sh#L26

l0rinc commented at 5:07 PM on July 20, 2026:

Yes, but it looks like an unintended typo - but it's just a nit anyway

in .github/actions/configure-environment/action.yml:10 in a668d7a893

   6 | @@ -7,7 +7,9 @@ runs:
   7 |        shell: bash
   8 |        run: |
   9 |          echo "BASE_ROOT_DIR=${{ runner.temp }}" >> "$GITHUB_ENV"
  10 | -        echo "BASE_BUILD_DIR=${{ runner.temp }}/build" >> "$GITHUB_ENV"
  11 | +        # The space and non-ASCII symbols are intentional.

l0rinc commented at 4:36 PM on July 19, 2026:

could we rather state what the intention was?

hebasto commented at 4:42 PM on July 19, 2026:

Could you please suggest the exact wording?

l0rinc commented at 5:32 PM on July 20, 2026:

I just have PTSD from this line https://github.com/bitcoin/bitcoin/blob/fa615bd163ac74c11a8e15ed8513a5b81ed9ef3b/src/node/blockstorage.cpp#L1139

Which doesn't actually explain the reason, just reads like: "don't touch this ever, I knew what I was doing"

Maybe something like:

        # Space and non-ASCII symbols mimic BASE_SCRATCH_DIR in ci/test/00_setup_env.sh,
        # to test word-splitting and UTF-8 path handling on CI.

hebasto commented at 7:59 PM on July 20, 2026:

Thanks! Taken.

l0rinc commented at 6:40 PM on July 21, 2026:

The updated BASE_SCRATCH_DIR comment explains the intent properly, so the shorter reference here works for me 👍 https://github.com/bitcoin/bitcoin/blob/fac6c4270dac566ebf274a5c18e30810a3da70f9/ci/test/00_setup_env.sh#L23-L24

l0rinc commented at 4:37 PM on July 19, 2026: contributor

code review ACK a668d7a8935746d5f651c8610cdc724bebb661e5

maflcko commented at 8:59 AM on July 20, 2026: member

Heh, I was trying to test this via ./ci/test/00_setup_env_win64.sh locally, but:

On Ubuntu 26.04, it fails due to #33593 (comment)
Then I tried Debian Experimental, but it fails due to -Werror=deprecated-declarations #35704
Then I tried Debian Trixie and could confirm it fails on master and passes with this pull.

master:

Error in script "/ci_container_base/ci/scratch_ ₿🧪_/build-x86_64-w64-mingw32ucrt/bitcoin-win64-setup.nsi" on line 75 -- aborting creation process

This pull (passes)

However, you forgot to quote INSTDIR?

review ACK a668d7a8935746d5f651c8610cdc724bebb661e5 🍤

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK a668d7a8935746d5f651c8610cdc724bebb661e5 🍤
QWS4zQwTFMBtR7y2j9tpWKRtJp2QytsNFjxnxJHG7TroFSRBV8aKULQ+fGVbi2kDfHfvZIHAYoDRVOGRTRzmDQ==

</details>

hebasto commented at 10:35 AM on July 20, 2026: member

However, you forgot to quote INSTDIR?

I didn't :)

INSTDIR is expanded at runtime and is treated differently. I've verified all its uses, and they follow the NSIS documentation.

maflcko added this to the milestone 32.0 on Jul 20, 2026

l0rinc approved

hebasto force-pushed on Jul 20, 2026

hebasto commented at 7:59 PM on July 20, 2026: member

Addressed feedback from @l0rinc.

l0rinc approved

l0rinc commented at 8:05 PM on July 20, 2026: contributor

code review ACK 350991fa01808b4718fe86219f5119ef1c4f1597

DrahtBot requested review from maflcko on Jul 20, 2026

maflcko commented at 5:02 AM on July 21, 2026: member

review ACK 350991fa01808b4718fe86219f5119ef1c4f1597 📙

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 350991fa01808b4718fe86219f5119ef1c4f1597 📙
CJ8JxZfH0aFPczyfLHkfGawSzgf9MV1g2eVrsNR69g7fvtEUGnOw43spPW0b8MZ8y2wKDkY5chtnSdOhjIPXBg==

</details>

hebasto commented at 11:50 AM on July 21, 2026: member

The first commit fixes the NSIS template, which otherwise breaks the deploy target when paths contain spaces.

My Guix build:

aarch64
36ff722035de407e6e9e1d8cb567c9bac58754753787a559fe95c184054e8d63  guix-build-350991fa0180/output/dist-archive/bitcoin-350991fa0180.tar.gz
bab8ccfa6dac1874ff1481d6113e96bddb59aba071452e4f5ac768e76ae6e37f  guix-build-350991fa0180/output/x86_64-w64-mingw32/SHA256SUMS.part
51df9ce9504252a6a28121dc7f85a144919886556f27957727dda6559182f48c  guix-build-350991fa0180/output/x86_64-w64-mingw32/bitcoin-350991fa0180-win64-codesigning.tar.gz
6f06943513c89270ed121c13dc082b35bf89d2cf33008d683bc85501a0c7e0db  guix-build-350991fa0180/output/x86_64-w64-mingw32/bitcoin-350991fa0180-win64-debug.zip
a7d39aedeb079b1f86ba5c11c9a0fdeb1ff32875337abc0065cdfbf162f937e0  guix-build-350991fa0180/output/x86_64-w64-mingw32/bitcoin-350991fa0180-win64-setup-unsigned.exe
21eaf4f2d4587b418c387277d7129d8a83dc10e99cb8e86e3eba2fdcb4ccdcb9  guix-build-350991fa0180/output/x86_64-w64-mingw32/bitcoin-350991fa0180-win64-unsigned.zip

DrahtBot added the label Needs rebase on Jul 21, 2026

hebasto force-pushed on Jul 21, 2026

hebasto commented at 3:00 PM on July 21, 2026: member

Rebased to resolve a conflict with the merged bitcoin/bitcoin#35537.

DrahtBot added the label CI failed on Jul 21, 2026

DrahtBot removed the label Needs rebase on Jul 21, 2026

in share/setup.nsi.in:78 in 8f7ffa70e5

  71 | @@ -72,19 +72,19 @@ ShowUninstDetails show
  72 |  Section -Main SEC0000
  73 |      SetOutPath $INSTDIR
  74 |      SetOverwrite on
  75 | -    File @BIN_DIR@/@BITCOIN_GUI_NAME@@EXEEXT@
  76 | -    File @BIN_DIR@/@BITCOIN_WRAPPER_NAME@@EXEEXT@
  77 | +    File "@BIN_DIR@/@BITCOIN_GUI_NAME@@EXEEXT@"
  78 | +    File "@BIN_DIR@/@BITCOIN_WRAPPER_NAME@@EXEEXT@"
  79 |      File /oname=COPYING.txt @abs_top_srcdir@/COPYING
  80 |      File /oname=readme.txt @abs_top_srcdir@/doc/README_windows.txt

l0rinc commented at 6:44 PM on July 21, 2026:

Range diff shows the quotes have disappeared here.

I don't have experience with these, but my agent tells me /oname still requires its input path to be a single token, so these lines break when @abs_top_srcdir@ contains a space:

It does not make the source operand immune to whitespace. In fact, NSIS’s parser requires exactly one source token after /oname; an unquoted source path containing a space produces too many tokens.

NSIS parser implementation (https://github.com/kichik/nsis/blob/master/Source/script.cpp#L3664-L3679).

They should be:

File /oname=COPYING.txt "@abs_top_srcdir@/COPYING" File /oname=readme.txt "@abs_top_srcdir@/doc/README_windows.txt"

How come most of the CI passes here though - is it because only build folders contain the weird chars and not the source folders?

hebasto commented at 7:30 PM on July 21, 2026:

Wrong rebase. Should be fixed now.

hebasto commented at 7:59 PM on July 21, 2026:

How come most of the CI passes here though - is it because only build folders contain the weird chars and not the source folders?

I think so.

l0rinc changes_requested

l0rinc commented at 6:56 PM on July 21, 2026: contributor

My understanding is that the CI failure is unrelated - though it's suspicious that it failed exactly at check_whitespace_in_headers...

build: Quote host paths in NSIS installer template

The `File` instructions embed the build and source directories unquoted,
so `makensis` fails for the `deploy` target when either path contains
spaces.

a7e980af31

ci: Put space and non-ASCII char in `BASE_BUILD_DIR`

The GHA workflows override `BASE_BUILD_DIR`, so the build tree no longer
lives under `BASE_SCRATCH_DIR` and its word-splitting and UTF-8 coverage
is bypassed on CI. Restore it by putting a space and a non-ASCII symbol
in the externally defined path as well.

f3f302150b

hebasto force-pushed on Jul 21, 2026

hebasto commented at 7:32 PM on July 21, 2026: member

Rebased to resolve a conflict with the merged #35537.

The wrong rebase has been fixed:

$ git range-diff master 350991fa01808b4718fe86219f5119ef1c4f1597 f3f302150b5fc115e0561dd639b9b389822999e9
1:  5ee5a92610 < -:  ---------- build: Quote host paths in NSIS installer template
-:  ---------- > 1:  a7e980af31 build: Quote host paths in NSIS installer template
2:  350991fa01 = 2:  f3f302150b ci: Put space and non-ASCII char in `BASE_BUILD_DIR`

hebasto commented at 8:12 PM on July 21, 2026: member

The first commit fixes the NSIS template, which otherwise breaks the deploy target when paths contain spaces.

My Guix build:

x86_64
634dedc72f76cf053030d8c5def2c81ed8f57d8be1aeed9056caa66217837421  guix-build-f3f302150b5f/output/dist-archive/bitcoin-f3f302150b5f.tar.gz
92588ee14b95da997bc508ba1c878b796dea14c691de63366961a84f76a25360  guix-build-f3f302150b5f/output/x86_64-w64-mingw32/SHA256SUMS.part
24755186d36ee0fa28e8ac919b7cbccfa07a1aaaadbeaa7613ba06dadb41fc29  guix-build-f3f302150b5f/output/x86_64-w64-mingw32/bitcoin-f3f302150b5f-win64-codesigning.tar.gz
a9b9f7979e74b01503cf117533a113b100173cc099261856cbbd6e083e3a05af  guix-build-f3f302150b5f/output/x86_64-w64-mingw32/bitcoin-f3f302150b5f-win64-debug.zip
462d019c3a68b9831d0553f319a60b186955c850fde9f32d4d80147afe95383b  guix-build-f3f302150b5f/output/x86_64-w64-mingw32/bitcoin-f3f302150b5f-win64-setup-unsigned.exe
b9766c4f92fdf01557a366c27c67b97b15837b1c7d30eba9c451c7b548eb845c  guix-build-f3f302150b5f/output/x86_64-w64-mingw32/bitcoin-f3f302150b5f-win64-unsigned.zip

DrahtBot removed the label CI failed on Jul 21, 2026