bitcoin wrapper: respect CMAKE_INSTALL

ryanofsky commented at 6:28 PM on August 27, 2026: contributor

Problem: On Arch Linux the installed bitcoin command doesn't work: bitcoin -m node, bitcoin chainstate, and similar commands all fail with execvp failed to execute ... No such file or directory (#35785). Arch installs the programs the wrapper runs into a different directory than the wrapper looks in.

Solution: Make the wrapper look for those programs in the directory they were actually installed to, rather than a hardcoded one. Small change to the wrapper (src/bitcoin.cpp), plus build-system changes to pass it the configured install directories, and a functional test for the installed layout. Details are in the commit messages.

Fixes #35785. Built from earlier PRs #36037, #35789, and #36085.

test: characterize installed bitcoin wrapper libexec lookup

Add a functional test documenting how an installed `bitcoin` wrapper
resolves internal executables. When the wrapper lives in a `bin/`
directory it looks for binaries like `bitcoind` in a sibling directory
under the same install prefix; the normal build-tree tests never exercise
this because every binary sits next to the wrapper.

The test builds a fake install prefix, copies the wrapper into `bin/` and
`bitcoind` into an internal directory (but not next to the wrapper), and
invokes the wrapper by absolute path with an empty PATH so a lookup miss
cannot be satisfied by an unrelated system binary.

This characterizes current behavior: the wrapper resolves binaries from a
hardcoded `libexec/` directory and does not look in `lib/`. A following
commit changes the wrapper to honor CMAKE_INSTALL_LIBEXECDIR and updates
this test to read the configured value.

This test is adapted from the test added in
https://github.com/bitcoin/bitcoin/pull/36037.

Co-Authored-By: Avecci-Claussen <cto@greybound.tech>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

2463b7d84c

build: respect CMAKE_INSTALL_BINDIR/LIBEXECDIR in bitcoin wrapper

The `bitcoin` wrapper resolves internal executables (bitcoind,
bitcoin-node, bitcoin-gui, ...) relative to its own install location, but
hardcoded the string "libexec" for the internal-executable directory. The
CMake install rules install those binaries into CMAKE_INSTALL_LIBEXECDIR,
which defaults to "libexec" but can be set to something else: Arch Linux,
following its packaging guidelines that avoid /usr/libexec, configures
-DCMAKE_INSTALL_LIBEXECDIR=lib. On such installs `bitcoin -m node`,
`bitcoin chainstate`, etc. fail with "execvp failed to execute" because
the wrapper looks in the wrong directory.

Thread CMAKE_INSTALL_BINDIR and CMAKE_INSTALL_LIBEXECDIR into
bitcoin-build-config.h and use them in the installed-layout lookup instead
of the "bin"/"libexec" string literals, so the wrapper matches wherever
the binaries were actually installed. GNUInstallDirs is included at the
top level so these values are populated before the generated files that
substitute them, and static_asserts guard against an empty value.

The functional test is updated to read the configured LIBEXECDIR rather
than assuming "libexec".

Fixes #35785.

The fix and test build on
https://github.com/bitcoin/bitcoin/pull/36037,
https://github.com/bitcoin/bitcoin/pull/35789, and
https://github.com/bitcoin/bitcoin/pull/36085.

Co-Authored-By: Avecci-Claussen <cto@greybound.tech>
Co-Authored-By: Andrew Barnes <bortstheboat@gmail.com>
Co-Authored-By: riba2534 <riba2534@qq.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

dfc786c75e

DrahtBot commented at 6:28 PM on August 27, 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/36106.

Reviews

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

Type	Reviewers
Approach ACK	hebasto

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:

#36105 (bitcoin wrapper: Fix Windows exec so wrapper waits for child process by ryanofsky)
#36022 (test: add coverage for bitcoin wrapper argument handling by cyb3ralbert)
#32387 (ipc: add windows support by ryanofsky)

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.

ryanofsky force-pushed on Aug 27, 2026

DrahtBot added the label CI failed on Aug 27, 2026

ryanofsky commented at 7:54 PM on August 27, 2026: contributor

Updated 94c64d40c8e5b51122c39af9d781e360752aec11 -> dfc786c75ec3cdb18b824547bf586642c8bda6b5 (pr/wrapexec.1 -> pr/wrapexec.2, compare) to fix tool_bitcoin.py installed-layout check failing in CI: pass a writable -datadir so 'bitcoin node -version' doesn't depend on the default datadir ($HOME/.bitcoin) being writable https://github.com/bitcoin/bitcoin/actions/runs/33103647138/job/98627629393

DrahtBot removed the label CI failed on Aug 27, 2026

purpleKarrot commented at 3:18 AM on August 29, 2026: contributor

Looks good. But it also has AI co-authors.

hebasto commented at 11:52 AM on September 1, 2026: member

Concept ACK.

... it also has AI co-authors.

From the project's AI Policy:

Do not include agents as ... co-authors of your commits...

in src/bitcoin.cpp:25 in dfc786c75e

  17 | @@ -18,6 +18,12 @@
  18 |  #include <tinyformat.h>
  19 |  #include <vector>
  20 |  
  21 | +// These are used to locate installed executables relative to the wrapper and
  22 | +// must not be empty, so a missing GNUInstallDirs value fails at compile time
  23 | +// rather than silently breaking the installed-layout lookup in ExecCommand().
  24 | +static_assert(BITCOIN_BINDIR[0] != '\0', "BITCOIN_BINDIR must not be empty");
  25 | +static_assert(BITCOIN_INTERNAL_LIBEXECDIR[0] != '\0', "BITCOIN_INTERNAL_LIBEXECDIR must not be empty");

hebasto commented at 2:03 PM on September 1, 2026:

This limits the user's choice of values for CMAKE_INSTALL_BINDIR and CMAKE_INSTALL_LIBEXECDIR, whereas the CMake docs do not impose any such restriction. If this is strictly necessary, it should be probably documented.
Shouldn't the check for unset or empty CMake variables be performed earlier, at the configuration stage?

in CMakeLists.txt:643 in dfc786c75e

 636 | @@ -637,6 +637,11 @@ if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.29)
 637 |    set(CMAKE_SKIP_TEST_ALL_DEPENDENCY FALSE)
 638 |  endif()
 639 |  
 640 | +# Populate CMAKE_INSTALL_BINDIR, CMAKE_INSTALL_LIBEXECDIR, etc. before the
 641 | +# subdirectories below, which substitute these values into generated files
 642 | +# (test/config.ini and src/bitcoin-build-config.h).
 643 | +include(GNUInstallDirs)

hebasto commented at 2:06 PM on September 1, 2026:

While re-inclusions of GNUInstallDirs are harmless, maybe consider removing the now-redundant ones to keep the code clean.

in test/functional/tool_bitcoin.py:113 in dfc786c75e

 108 | +        # prefix constructed here, so skip the check in that case.
 109 | +        libexecdir = self.config["environment"]["LIBEXECDIR"]
 110 | +        if os.path.isabs(libexecdir):
 111 | +            self.log.info("Skipping installed-layout check; CMAKE_INSTALL_LIBEXECDIR is absolute")
 112 | +            return
 113 | +        prefix = self.nodes[0].datadir_path / "fake-prefix"

hebasto commented at 2:09 PM on September 1, 2026:

Using the node's datadir for the installation prefix seems counter-intuitive to me. Maybe place it directly under self.options.tmpdir?

hebasto commented at 2:10 PM on September 1, 2026: member

Approach ACK dfc786c75ec3cdb18b824547bf586642c8bda6b5.

Tested also with the cmake --install command's --prefix option.

The new test fails when the user specifies CMAKE_INSTALL_BINDIR:

$ cmake -B build -DCMAKE_INSTALL_BINDIR="nib"
$ cmake --build build
$ ./build/test/functional/tool_bitcoin.py
2026-09-01T13:53:13.898723Z TestFramework (INFO): PRNG seed is: 6212645667151046958
2026-09-01T13:53:13.899141Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_0y4u82jq
2026-09-01T13:53:13.899475Z TestFramework (INFO): Ensure bitcoin node command invokes bitcoind by default
2026-09-01T13:53:13.903417Z TestFramework (INFO): Ensure bitcoin -M invokes bitcoind
2026-09-01T13:53:13.907124Z TestFramework (INFO): Ensure bitcoin -M does not accept -ipcbind
2026-09-01T13:53:13.910846Z TestFramework (INFO): Ensure bitcoin -m invokes bitcoin-node
2026-09-01T13:53:13.914668Z TestFramework (INFO): Ensure bitcoin -m does accept -ipcbind
2026-09-01T13:53:13.918495Z TestFramework (INFO): Ensure bitcoin accepts -ipcbind by default
2026-09-01T13:53:13.922104Z TestFramework (INFO): Ensure bitcoin recognizes -ipcbind in config file
2026-09-01T13:53:13.926035Z TestFramework (INFO): Ensure installed wrapper finds internal binaries in configured libexec/
2026-09-01T13:53:14.046585Z TestFramework (ERROR): Unexpected exception:
Traceback (most recent call last):
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/test_framework.py", line 145, in main
    self.run_test()
    ~~~~~~~~~~~~~^^
  File "/home/hebasto/dev/bitcoin/./build/test/functional/tool_bitcoin.py", line 92, in run_test
    self.test_installed_layout()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/home/hebasto/dev/bitcoin/./build/test/functional/tool_bitcoin.py", line 143, in test_installed_layout
    assert_equal(result.returncode, 0)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/home/hebasto/dev/bitcoin/test/functional/test_framework/util.py", line 94, in assert_equal
    raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not(1 == 0)
2026-09-01T13:53:14.098127Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.
2026-09-01T13:53:14.098499Z TestFramework (WARNING): Not cleaning up dir /tmp/bitcoin_func_test_0y4u82jq
2026-09-01T13:53:14.098707Z TestFramework (ERROR): Test failed. Test logging available at /tmp/bitcoin_func_test_0y4u82jq/test_framework.log
2026-09-01T13:53:14.099086Z TestFramework (ERROR): 
2026-09-01T13:53:14.099412Z TestFramework (ERROR): Hint: Call /home/hebasto/dev/bitcoin/test/functional/combine_logs.py '/tmp/bitcoin_func_test_0y4u82jq' to consolidate all logs
2026-09-01T13:53:14.099593Z TestFramework (ERROR): 
2026-09-01T13:53:14.099752Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2026-09-01T13:53:14.099975Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2026-09-01T13:53:14.100138Z TestFramework (ERROR):

cyb3ralbert commented at 9:30 AM on September 2, 2026: contributor

The fix works — I built the branch and ran the #35785 scenario to verify it. I did find two gaps in the test coverage, though: one that seems worth fixing here, and one that could be a follow-up.

test_installed_layout derives other_dir = "lib" if libexecdir != "lib" else "libexec", which at the default CMAKE_INSTALL_LIBEXECDIR=libexec is exactly the pair the old wrapper hardcodes:

`CMAKE_INSTALL_LIBEXECDIR`	wrapper	result
`libexec` (default)	this branch	pass
`libexec` (default)	before this change	pass
`lib`	this branch	pass
`lib`	before this change	fail — `AssertionError: not(1 == 0)`

CMAKE_INSTALL_LIBEXECDIR does not appear anywhere under ci/ or .github/, so every job builds with the GNUInstallDirs default (libexec) and none of them can tell the two wrappers apart — a regression to a hardcoded directory would stay green. The lib value is what Arch actually uses (-DCMAKE_INSTALL_LIBEXECDIR=lib in its PKGBUILD), so adding it to BITCOIN_CONFIG in one of the existing ci/test/00_setup_env_*.sh jobs would make the test you already wrote cover the change. That part could go in a follow-up.

The smaller one: run_wrapper calls subprocess.run([wrapper, ...]) directly, bypassing binaries.valgrind_cmd, so the new checks are skipped under --valgrind — 25 ms and 63 ms against ~2 s for each neighbouring check in the same run. The same point was raised on #36022 in this file and fixed there by prefixing self.nodes[0].binaries.valgrind_cmd; it matters here because that command carries --trace-children=yes, which is "Needed for 'bitcoin' wrapper".

bitcoin wrapper: respect CMAKE_INSTALL_BINDIR/LIBEXECDIR #36106

Code Coverage & Benchmarks

Reviews

Conflicts