💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2624104354)
>With this PR, these would now all need to figure out where the binaries live, and that seems to be significantly less trivial to do than detecting autotools or cmake.
Something like this should work:
```sh
[ -e build/bin/test_bitcoin ] && ln -s $(realpath build/bin/test_bitcoin) build/src/test/
```
> I would've preferred that this was done with cmake, or immediately after it, than now...
I agree with you in hindsight.
> ... where we're a few weeks before feature freeze for 29.0.
...
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2624104354)
>With this PR, these would now all need to figure out where the binaries live, and that seems to be significantly less trivial to do than detecting autotools or cmake.
Something like this should work:
```sh
[ -e build/bin/test_bitcoin ] && ln -s $(realpath build/bin/test_bitcoin) build/src/test/
```
> I would've preferred that this was done with cmake, or immediately after it, than now...
I agree with you in hindsight.
> ... where we're a few weeks before feature freeze for 29.0.
...
⚠️ stickies-v opened an issue: "bug: cmake installs empty manpages for non-configured targets"
(https://github.com/bitcoin/bitcoin/issues/31762)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
When configuring cmake with a subset of targets (e.g. kernel-only), a blanket `cmake --install` will still install the placeholder manpages files for all other targets.
For example:
```
cmake -B build -DBUILD_KERNEL_LIB=ON -DBUILD_BENCH=OFF -DBUILD_DAEMON=OFF -DBUILD_CLI=OFF -DBUILD_GUI=OFF -DBUILD_FUZZ_BINARY=OFF -DBUILD_TESTS=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DBUILD_UTIL_CHAINSTATE=
...
(https://github.com/bitcoin/bitcoin/issues/31762)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
When configuring cmake with a subset of targets (e.g. kernel-only), a blanket `cmake --install` will still install the placeholder manpages files for all other targets.
For example:
```
cmake -B build -DBUILD_KERNEL_LIB=ON -DBUILD_BENCH=OFF -DBUILD_DAEMON=OFF -DBUILD_CLI=OFF -DBUILD_GUI=OFF -DBUILD_FUZZ_BINARY=OFF -DBUILD_TESTS=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DBUILD_UTIL_CHAINSTATE=
...
👍 TheCharlatan approved a pull request: "rpc: have getblocktemplate mintime account for timewarp"
(https://github.com/bitcoin/bitcoin/pull/31600#pullrequestreview-2583553574)
ACK e1676b08f7b0b9a6c8ed76e31f24faa03a3facc9
(https://github.com/bitcoin/bitcoin/pull/31600#pullrequestreview-2583553574)
ACK e1676b08f7b0b9a6c8ed76e31f24faa03a3facc9
📝 Sjors opened a pull request: "Pass custom DEP_OPTS and CONFIG_FLAGS to guix-build"
(https://github.com/bitcoin/bitcoin/pull/31763)
This allows building deterministic binaries with multiprocess like so:
```sh
DEP_OPTS="MULTIPROCESS=1" contrib/guix/guix-build
```
Additionally custom options can be passed to `cmake -B` using `CONFIG_FLAGS`. Example (turns off external signer support, the rest is default):
```sh
CONFIG_FLAGS="-DENABLE_EXTERNAL_SIGNER=OFF -DREDUCE_EXPORTS=ON -DBUILD_BENCH=OFF -DBUILD_GUI_TESTS=OFF -DBUILD_FUZZ_BINARY=OFF" contrib/guix/guix-build
```
The `CONFIG_FLAGS` is less useful in practice,
...
(https://github.com/bitcoin/bitcoin/pull/31763)
This allows building deterministic binaries with multiprocess like so:
```sh
DEP_OPTS="MULTIPROCESS=1" contrib/guix/guix-build
```
Additionally custom options can be passed to `cmake -B` using `CONFIG_FLAGS`. Example (turns off external signer support, the rest is default):
```sh
CONFIG_FLAGS="-DENABLE_EXTERNAL_SIGNER=OFF -DREDUCE_EXPORTS=ON -DBUILD_BENCH=OFF -DBUILD_GUI_TESTS=OFF -DBUILD_FUZZ_BINARY=OFF" contrib/guix/guix-build
```
The `CONFIG_FLAGS` is less useful in practice,
...
👋 Sjors's pull request is ready for review: "Pass custom DEP_OPTS and CONFIG_FLAGS to guix-build"
(https://github.com/bitcoin/bitcoin/pull/31763)
(https://github.com/bitcoin/bitcoin/pull/31763)
💬 Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2624277683)
If I cherry-pick #31763 on top of this, I'm unable to make a Guix build with `DEP_OPTS="MULTIPROCESS=1" contrib/guix/guix-build`, at least not with the hosts I tried (`x86_64-linux-gnu`, `aarch64-linux-gnu` and `arm64-apple-darwin`).
```
Checking that we can connect to the guix-daemon...
Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.
make: Entering directory '/home/sjors/bitcoin/depends'
make[1]: Entering directory '/home/sjors/bitcoin/depen
...
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2624277683)
If I cherry-pick #31763 on top of this, I'm unable to make a Guix build with `DEP_OPTS="MULTIPROCESS=1" contrib/guix/guix-build`, at least not with the hosts I tried (`x86_64-linux-gnu`, `aarch64-linux-gnu` and `arm64-apple-darwin`).
```
Checking that we can connect to the guix-daemon...
Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.
make: Entering directory '/home/sjors/bitcoin/depends'
make[1]: Entering directory '/home/sjors/bitcoin/depen
...
📝 hebasto opened a pull request: "cmake: Generate man pages at install time"
(https://github.com/bitcoin/bitcoin/pull/31764)
This PR enables the man pages to be generated on the fly during installation.
As the GNU help2man tool is required to generate man pages, the `INSTALL_MAN` build option is disabled by default.
Addresses this comment:https://github.com/bitcoin/bitcoin/blob/809d7e763cc9bdfff3288860a1c530460c76ffff/src/CMakeLists.txt#L456
Fixes https://github.com/bitcoin/bitcoin/issues/31762.
(https://github.com/bitcoin/bitcoin/pull/31764)
This PR enables the man pages to be generated on the fly during installation.
As the GNU help2man tool is required to generate man pages, the `INSTALL_MAN` build option is disabled by default.
Addresses this comment:https://github.com/bitcoin/bitcoin/blob/809d7e763cc9bdfff3288860a1c530460c76ffff/src/CMakeLists.txt#L456
Fixes https://github.com/bitcoin/bitcoin/issues/31762.
💬 hebasto commented on pull request "cmake: Generate man pages at install time":
(https://github.com/bitcoin/bitcoin/pull/31764#issuecomment-2624395483)
cc @theuni as an author of the TODO [comment](https://github.com/bitcoin/bitcoin/blob/809d7e763cc9bdfff3288860a1c530460c76ffff/src/CMakeLists.txt#L456).
(https://github.com/bitcoin/bitcoin/pull/31764#issuecomment-2624395483)
cc @theuni as an author of the TODO [comment](https://github.com/bitcoin/bitcoin/blob/809d7e763cc9bdfff3288860a1c530460c76ffff/src/CMakeLists.txt#L456).
💬 fanquake commented on pull request "cmake: Generate man pages at install time":
(https://github.com/bitcoin/bitcoin/pull/31764#discussion_r1935544996)
How does this change the guix dep graph?
(https://github.com/bitcoin/bitcoin/pull/31764#discussion_r1935544996)
How does this change the guix dep graph?
💬 fanquake commented on pull request "cmake: Generate man pages at install time":
(https://github.com/bitcoin/bitcoin/pull/31764#discussion_r1935549376)
> --skip-missing-binaries
I don't think we should be hardcoding a flag to ignore/skip errors into a script that you're making a requirement for release builds.
(https://github.com/bitcoin/bitcoin/pull/31764#discussion_r1935549376)
> --skip-missing-binaries
I don't think we should be hardcoding a flag to ignore/skip errors into a script that you're making a requirement for release builds.
📝 hebasto converted_to_draft a pull request: "cmake: Generate man pages at install time"
(https://github.com/bitcoin/bitcoin/pull/31764)
This PR enables the man pages to be generated on the fly during installation.
As the GNU help2man tool is required to generate man pages, the `INSTALL_MAN` build option is disabled by default.
Addresses this comment:https://github.com/bitcoin/bitcoin/blob/809d7e763cc9bdfff3288860a1c530460c76ffff/src/CMakeLists.txt#L456
Fixes https://github.com/bitcoin/bitcoin/issues/31762.
(https://github.com/bitcoin/bitcoin/pull/31764)
This PR enables the man pages to be generated on the fly during installation.
As the GNU help2man tool is required to generate man pages, the `INSTALL_MAN` build option is disabled by default.
Addresses this comment:https://github.com/bitcoin/bitcoin/blob/809d7e763cc9bdfff3288860a1c530460c76ffff/src/CMakeLists.txt#L456
Fixes https://github.com/bitcoin/bitcoin/issues/31762.
💬 TheCharlatan commented on pull request "cmake: Generate man pages at install time":
(https://github.com/bitcoin/bitcoin/pull/31764#issuecomment-2624438647)
Do the release process docs need any changes from this?
(https://github.com/bitcoin/bitcoin/pull/31764#issuecomment-2624438647)
Do the release process docs need any changes from this?
✅ hebasto closed a pull request: "cmake: Generate man pages at install time"
(https://github.com/bitcoin/bitcoin/pull/31764)
(https://github.com/bitcoin/bitcoin/pull/31764)
💬 fjahr commented on pull request "rpc: have getblocktemplate mintime account for timewarp":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2624484953)
> @fjahr the new rule only applies to blocks at height % 2016 == 0, so I'm confused about the heights you found.
Did you read what I wrote below the heights?
> I did not limit this retargeting blocks because the goal of this exercise was to see if some specific miner software has a systemic issue in that area or if this is a frequent occurrence in general which might have been worrying. But neither appears to be the case.
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2624484953)
> @fjahr the new rule only applies to blocks at height % 2016 == 0, so I'm confused about the heights you found.
Did you read what I wrote below the heights?
> I did not limit this retargeting blocks because the goal of this exercise was to see if some specific miner software has a systemic issue in that area or if this is a frequent occurrence in general which might have been worrying. But neither appears to be the case.
📝 hebasto opened a pull request: "cmake: Install man pages for built targets only"
(https://github.com/bitcoin/bitcoin/pull/31765)
Fixes https://github.com/bitcoin/bitcoin/issues/31762.
(https://github.com/bitcoin/bitcoin/pull/31765)
Fixes https://github.com/bitcoin/bitcoin/issues/31762.
💬 hebasto commented on pull request "cmake: Generate man pages at install time":
(https://github.com/bitcoin/bitcoin/pull/31764#issuecomment-2624488053)
This approach is not applicable for general cross-compilation scenarios :man_facepalming:
(https://github.com/bitcoin/bitcoin/pull/31764#issuecomment-2624488053)
This approach is not applicable for general cross-compilation scenarios :man_facepalming:
💬 Sjors commented on pull request "rpc: have getblocktemplate mintime account for timewarp":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2624518361)
I don't understand what your script is looking for. Are you saying you applied the timewarp rule to every block instead, and then checked which block violated it?
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2624518361)
I don't understand what your script is looking for. Are you saying you applied the timewarp rule to every block instead, and then checked which block violated it?
💬 fjahr commented on pull request "rpc: have getblocktemplate mintime account for timewarp":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2624524721)
> I don't understand what your script is looking for. Are you saying you applied the timewarp rule to every block instead, and then checked which block violated it?
Yes, I checked for which blocks `block_time < (prev_block_time - 600)` applies in general.
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2624524721)
> I don't understand what your script is looking for. Are you saying you applied the timewarp rule to every block instead, and then checked which block violated it?
Yes, I checked for which blocks `block_time < (prev_block_time - 600)` applies in general.
💬 fanquake commented on pull request "cmake: Generate man pages at install time":
(https://github.com/bitcoin/bitcoin/pull/31764#issuecomment-2624543963)
One other side-effect of this change would be that someone cloning a tag would no-longer get the manpages. Maybe this doesn't matter, but would be good to mention.
(https://github.com/bitcoin/bitcoin/pull/31764#issuecomment-2624543963)
One other side-effect of this change would be that someone cloning a tag would no-longer get the manpages. Maybe this doesn't matter, but would be good to mention.
📝 fanquake opened a pull request: "[WIP] leveldb: pull upstream C++23 changes"
(https://github.com/bitcoin/bitcoin/pull/31766)
PR to test https://github.com/bitcoin-core/leveldb-subtree/pull/47 in our CI.
Cherry-picks two commits from upstream (https://github.com/google/leveldb/commit/302786e211d1f2e6fd260261f642d03a91e5922c, https://github.com/google/leveldb/commit/e829478c6a3a55d8e5c1227e2678dcc18d518609), which remove the usage of `std::aligned_storage/std::aligned_union`.
Note the first cherry-pick is not clean, because due to Google tooling issues, it accidently contained a revert of the prior two commits. S
...
(https://github.com/bitcoin/bitcoin/pull/31766)
PR to test https://github.com/bitcoin-core/leveldb-subtree/pull/47 in our CI.
Cherry-picks two commits from upstream (https://github.com/google/leveldb/commit/302786e211d1f2e6fd260261f642d03a91e5922c, https://github.com/google/leveldb/commit/e829478c6a3a55d8e5c1227e2678dcc18d518609), which remove the usage of `std::aligned_storage/std::aligned_union`.
Note the first cherry-pick is not clean, because due to Google tooling issues, it accidently contained a revert of the prior two commits. S
...