cmake: Create subdirectories in build tree in advance #32773

pull hebasto wants to merge 2 commits into bitcoin:master from hebasto:250618-mkdir changing 1 files +63 −14
  1. hebasto commented at 7:52 pm on June 18, 2025: member

    While reviewing #32697, I noticed that symlink creation fails when the target subdirectory does not exist. In such cases, file(CREATE_LINK ... COPY_ON_ERROR SYMBOLIC) falls back to copying, which implicitly creates the required path. As a result, a single file is copied instead of symlinked for subdirectories in test/functional.

    This PR ensures that necessary subdirectories are created in advance, so that subsequent symlink creation does not fail due to missing paths.

    For example:

    • on the master branch:
    0$ cmake -B build
1$ ls -l build/test/functional/mocks/
2total 8
3-rwxrwxr-x 1 hebasto hebasto 2683 Jul  3 14:11 invalid_signer.py
4lrwxrwxrwx 1 hebasto hebasto   64 Jul  3 14:11 multi_signers.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/multi_signers.py
5lrwxrwxrwx 1 hebasto hebasto   60 Jul  3 14:11 no_signer.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/no_signer.py
6lrwxrwxrwx 1 hebasto hebasto   57 Jul  3 14:11 signer.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/signer.py
    • with this PR:
    0$ cmake -B build
1$ ls -l build/test/functional/mocks/
2total 4
3lrwxrwxrwx 1 hebasto hebasto 65 Jul  3 13:51 invalid_signer.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/invalid_signer.py
4lrwxrwxrwx 1 hebasto hebasto 64 Jul  3 13:51 multi_signers.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/multi_signers.py
5lrwxrwxrwx 1 hebasto hebasto 60 Jul  3 13:51 no_signer.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/no_signer.py
6lrwxrwxrwx 1 hebasto hebasto 57 Jul  3 13:51 signer.py -> /home/hebasto/dev/bitcoin/test/functional/mocks/signer.py
  2. hebasto added the label Build system on Jun 18, 2025
  3. hebasto added the label Tests on Jun 18, 2025
  4. DrahtBot commented at 7:52 pm on June 18, 2025: 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/32773.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK janb84
    Stale ACK BrandonOdiwuor

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    No conflicts as of last run.

  5. maflcko commented at 8:01 pm on June 18, 2025: member
    would it make sense to at least replace the recursive glob with a normal glob now, iterating over the given folders, given that they are listed anyway?
  6. hebasto commented at 9:31 am on June 19, 2025: member

    would it make sense to at least replace the recursive glob with a normal glob now, iterating over the given folders, given that they are listed anyway?

    Sure! Added a commit.

  7. janb84 commented at 5:37 pm on June 23, 2025: contributor

    Any hints how to verify / test this ?

    Have tried to see any difference between this PR and master but it’s not clear to me what difference I should see. When used on #32697 there is a difference that without this PR build/test/functional/data/util/* is populated with this PR the util directory is not being created (looking at the source this is logical but unsure if there needs to be things done after 32697 is merged or not having the directory is intentional )

  8. purpleKarrot commented at 10:01 am on June 24, 2025: contributor
    Is this setting up the environment for tests to run? Ideally, things like that should be done as test fixture and not executed during build system generation.
  9. DrahtBot added the label Needs rebase on Jun 30, 2025
  10. hebasto force-pushed on Jul 3, 2025
  11. hebasto commented at 1:16 pm on July 3, 2025: member

    Rebased. @janb84

    Any hints how to verify / test this ?

    Have tried to see any difference between this PR and master but it’s not clear to me what difference I should see. When used on #32697 there is a difference that without this PR build/test/functional/data/util/* is populated with this PR the util directory is not being created (looking at the source this is logical but unsure if there needs to be things done after 32697 is merged or not having the directory is intentional )

    I’ve updated the PR description with the relevant example. @purpleKarrot

    Is this setting up the environment for tests to run? Ideally, things like that should be done as test fixture and not executed during build system generation.

    At this moment, functional tests are not managed by ctest/cmake.

  12. janb84 commented at 2:20 pm on July 3, 2025: contributor

    ACK 0b7d038972315b5537cc42038094902ebd5dd8cf

    This PR changes :

    • Recursive Glob -> ’explicit’ non recursive GLOB
    • Subdirectories now are created in advance, so that there aren’t any problems with missing paths for sym-linking.

    The PR fixes a “bug” and the code changes make the code communicate the intentions of the code better.

    Can reproduce the successful subfolder creation and therefor file sym-linking :

    0$ ls -l
1total 4
2-rwxr-xr-x 1 jan staff 2683 Jul  3 15:57 invalid_signer.py
3lrwxr-xr-x 1 jan staff   73 Jul  3 15:57 multi_signers.py -> /Users/jan/Projects/bitcoin/test/functional/mocks/multi_signers.py
4lrwxr-xr-x 1 jan staff   69 Jul  3 15:57 no_signer.py -> /Users/jan/Projects/bitcoin/test/functional/mocks/no_signer.py
5lrwxr-xr-x 1 jan staff   66 Jul  3 15:57 signer.py -> /Users/jan/Projects/bitcoin/test/functional/mocks/signer.py

    This PR (works see invalid_signer.py):

    0ls -l
1total 0
2lrwxr-xr-x 1 jan staff 74 Jul  3 16:05 invalid_signer.py -> /Users/jan/Projects/bitcoin/test/functional/mocks/invalid_signer.py
3lrwxr-xr-x 1 jan staff 73 Jul  3 16:05 multi_signers.py -> /Users/jan/Projects/bitcoin/test/functional/mocks/multi_signers.py
4lrwxr-xr-x 1 jan staff 69 Jul  3 16:05 no_signer.py -> /Users/jan/Projects/bitcoin/test/functional/mocks/no_signer.py
5lrwxr-xr-x 1 jan staff 66 Jul  3 16:05 signer.py -> /Users/jan/Projects/bitcoin/test/functional/mocks/signer.py
    • code review ✅
    • building & tests ✅ @hebasto Thank you for supplying the additional information how to test the PR.
  13. DrahtBot removed the label Needs rebase on Jul 3, 2025
  14. DrahtBot added the label Needs rebase on Jul 10, 2025
  15. BrandonOdiwuor commented at 6:06 pm on July 11, 2025: contributor

    Tested ACK 0b7d038972315b5537cc42038094902ebd5dd8cf

    Confirmed that the subdirectories are created in the build tree before the symlinks preventing accidental COPY_ON_ERRORs like invalid_signer.py

    Commit: 0b7d038972315b5537cc42038094902ebd5dd8cf

    Branch: Master

    I also agree that using explicit file(GLOB ...) for specific file types (e.g., *.py, *.json, *.csv, *.html) is more robust than the original file(GLOB_RECURSE ...)

  16. cmake: Create subdirectories in build tree in advance 
    This change ensures that subsequent symlink creation does not fail due
to a missing path.
    88d9092571
  17. cmake: Replace recursive globbing with explicit globbing in folders fa3beb6316
  18. hebasto force-pushed on Jul 17, 2025
  19. hebasto commented at 3:43 pm on July 17, 2025: member
    Rebased to resolve conflict with merged #32881.
  20. DrahtBot removed the label Needs rebase on Jul 17, 2025
  21. janb84 commented at 8:44 am on July 18, 2025: contributor

    re ACK fa3beb6316d426b7e388d71fa2f09aed4d2da3d2

    Changes sinds last ACK:

    • Solved Merge conflict.

    Retested, still worked as described. ✅ Code review ✅

  22. DrahtBot requested review from BrandonOdiwuor on Jul 18, 2025


hebasto DrahtBot maflcko janb84 purpleKarrot BrandonOdiwuor


BrandonOdiwuor

Labels
Build system Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-07-23 00:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me