Intended to make bitcoin command line features more discoverable and allow installing new multiprocess binaries in libexec/ instead of bin/ so they don’t cause confusion.
Idea and implementation of this were discussed in #30983
This PR is part of the process separation project.
Intended to make bitcoin command line features more discoverable and allow
installing new multiprocess binaries in libexec/ instead of bin/ so they don't
cause confusion.
Idea and implementation of this were discussed in
https://github.com/bitcoin/bitcoin/issues/30983
It's possible to debug this and see what programs it is trying to call by running:
strace -e trace=execve -s 10000 build/src/bitcoin ...
Support new BITCOIN_CMD environment variable in functional test to be able to
test the new bitcoin wrapper executable and run other commands through it
instead of calling them directly.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31375.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | Sjors |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
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.
🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/33557375133
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
pr/wrap.3 -> pr/wrap.4, compare) with fixes for windows and fuzz CI errors, and lint and tidy fixes
Updated 63df9f3deb35be79496f7c240e3303e1d96c6832 -> da108a6e5be220654a65b6613ee7eb2c4ddc8677 (pr/wrap.4 -> pr/wrap.5, compare) fixing windows include error, fs lint error, and previous releases test bug
183+ // Otherwise if exe is installed in a bin/ directory, first look for target
184+ // executable in libexec/
185+ (exe_dir.filename() == "bin" && try_exec(fs::path{exe_dir.parent_path()} / "libexec" / fs::PathFromString(args[0]).filename())) ||
186+ // Otherwise look for target executable next to current exe
187+ try_exec(exe_dir / fs::PathFromString(args[0]).filename()) ||
188+ // Otherwise just look on the system path.
build/src/bitcoin gui to fail. Right now it would launch any bitcoin-qt in my $PATH.
Concept ACK
I think it would be more clear to move build/src/bitcoin-{node,gui} to build/src/libexec, rather than use a different file organization for CMake builds than for installs.
The “Win64 native, VS 2022” job still seems unhappy.
The “Win64 native, VS 2022” job still seems unhappy.
To turn off deprecation warnings for these functions, define the preprocessor macro
_CRT_NONSTDC_NO_WARNINGS. You can define this macro at the command line by including the option/D_CRT_NONSTDC_NO_WARNINGS.
We have already used this macro:https://github.com/bitcoin/bitcoin/blob/7590e93bc73b3bbac641f05d490fd5c984156b33/cmake/leveldb.cmake#L87-L89