📝 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
...
💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2624601401)
`266ac32673...7866c736c8`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2624601401)
`266ac32673...7866c736c8`: rebase due to conflicts
💬 TheCharlatan commented on pull request "build: Enhance Ccache performance across worktrees and build trees":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1935678768)
Why is this now overriding the variable instead of appending?
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1935678768)
Why is this now overriding the variable instead of appending?
💬 ryanofsky commented on issue "RFC: multiprocess binaries in 29.0":
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2624659370)
Thanks again for clarifying your concerns @darosior. Hopefully Sjors post added background and answered the specific questions you asked. I just wanted to respond to two things:
> First of all why is the default position that we should ship a new binary on short notice right before feature freeze
I think you can be assured that this is not the case. As a rule controversial PRs do not get merged. I'm just trying to get to the bottom of why #31756 + eb41f92c0889708045ce0a30c6d0f8d43cecd6d5 was c
...
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2624659370)
Thanks again for clarifying your concerns @darosior. Hopefully Sjors post added background and answered the specific questions you asked. I just wanted to respond to two things:
> First of all why is the default position that we should ship a new binary on short notice right before feature freeze
I think you can be assured that this is not the case. As a rule controversial PRs do not get merged. I'm just trying to get to the bottom of why #31756 + eb41f92c0889708045ce0a30c6d0f8d43cecd6d5 was c
...
💬 instagibbs commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#issuecomment-2624666467)
reACK 7426afbe62414fa575f91b4f8d3ea63bcc653e8b
I am -0 on "[p2p] assign just 1 random announcer in AddChildrenToWorkSet" though. That and similar changes discussed in https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934078679 seem to complicate analysis for unclear performance gains?
(https://github.com/bitcoin/bitcoin/pull/31666#issuecomment-2624666467)
reACK 7426afbe62414fa575f91b4f8d3ea63bcc653e8b
I am -0 on "[p2p] assign just 1 random announcer in AddChildrenToWorkSet" though. That and similar changes discussed in https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934078679 seem to complicate analysis for unclear performance gains?
💬 instagibbs commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#issuecomment-2624666944)
reACK 7426afbe62414fa575f91b4f8d3ea63bcc653e8b
I am -0 on "[p2p] assign just 1 random announcer in AddChildrenToWorkSet" though. That and similar changes discussed in https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934078679 seem to complicate analysis for unclear performance gains?
(https://github.com/bitcoin/bitcoin/pull/31666#issuecomment-2624666944)
reACK 7426afbe62414fa575f91b4f8d3ea63bcc653e8b
I am -0 on "[p2p] assign just 1 random announcer in AddChildrenToWorkSet" though. That and similar changes discussed in https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934078679 seem to complicate analysis for unclear performance gains?
🤔 Sjors reviewed a pull request: "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`"
(https://github.com/bitcoin/bitcoin/pull/31243#pullrequestreview-2583927266)
Concept ACK
Mostly happy with 1bfabb4f4ec07c8fb0dfc2515d413b5196e01348, but I'm confused about the key origin handling.
(https://github.com/bitcoin/bitcoin/pull/31243#pullrequestreview-2583927266)
Concept ACK
Mostly happy with 1bfabb4f4ec07c8fb0dfc2515d413b5196e01348, but I'm confused about the key origin handling.
💬 Sjors commented on pull request "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`":
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r1935702280)
e743d11aae2e459058b1af70b6d19ab90293d1f4: I'm confused by this comment. I guess by "valid" you mean that a `KeyOriginInfo` field is present, but it can be a dummy?
It seems more intuitive if we insert an origin entry ourselves here ... if possible.
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r1935702280)
e743d11aae2e459058b1af70b6d19ab90293d1f4: I'm confused by this comment. I guess by "valid" you mean that a `KeyOriginInfo` field is present, but it can be a dummy?
It seems more intuitive if we insert an origin entry ourselves here ... if possible.
💬 Sjors commented on pull request "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`":
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r1935623167)
1bfabb4f4ec07c8fb0dfc2515d413b5196e01348: maybe rename to `FillPrivKey`?
```cpp
/** Try to derive the private key at pos, if private data is available in arg, and fill out with it. */
virtual void FillPrivKey(int pos, const SigningProvider& arg, FlatSigningProvider& out) const = 0;
```
That also distinguishes it from `ConstPubkeyProvider::GetPrivKey`.
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r1935623167)
1bfabb4f4ec07c8fb0dfc2515d413b5196e01348: maybe rename to `FillPrivKey`?
```cpp
/** Try to derive the private key at pos, if private data is available in arg, and fill out with it. */
virtual void FillPrivKey(int pos, const SigningProvider& arg, FlatSigningProvider& out) const = 0;
```
That also distinguishes it from `ConstPubkeyProvider::GetPrivKey`.
💬 Sjors commented on pull request "descriptor: Move filling of keys from `DescriptorImpl::MakeScripts` to `PubkeyProvider::GetPubKey`":
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r1935646730)
e743d11aae2e459058b1af70b6d19ab90293d1f4: maybe rename this to `FillPubKey`. The filling seems more important, and the return value makes it clear enough it also gets it.
(https://github.com/bitcoin/bitcoin/pull/31243#discussion_r1935646730)
e743d11aae2e459058b1af70b6d19ab90293d1f4: maybe rename this to `FillPubKey`. The filling seems more important, and the return value makes it clear enough it also gets it.
💬 Julian128 commented on pull request "RFC: optimize `CheckBlock` input checks (duplicate detection & nulls)":
(https://github.com/bitcoin/bitcoin/pull/31682#issuecomment-2624700760)
I experimented a bit more and it looks like I got another 17% speed improvement (on block413567 on my linux machine) over your changes @l0rinc , by using a faster comparison operator and doing direct comparisons for relatively small input sizes. Can I create a new pull request for that?
(https://github.com/bitcoin/bitcoin/pull/31682#issuecomment-2624700760)
I experimented a bit more and it looks like I got another 17% speed improvement (on block413567 on my linux machine) over your changes @l0rinc , by using a faster comparison operator and doing direct comparisons for relatively small input sizes. Can I create a new pull request for that?