💬 hebasto commented on issue "build: cmake --install fails after --target deploy":
(https://github.com/bitcoin/bitcoin/issues/32007#issuecomment-2705751725)
I think this behaviour is expected since `test_bitcoin-qt.exe` is not part of the Windows installer and is not required for the `deploy` target.
Additionally, in [WINDOWS BUILD NOTES](https://github.com/bitcoin/bitcoin/blob/a9a2b669f3e07266ae739574e9c1cef5af711db7/doc/build-windows.md), `cmake --install build` follows `cmake --build build`, and this sequence always works.
(https://github.com/bitcoin/bitcoin/issues/32007#issuecomment-2705751725)
I think this behaviour is expected since `test_bitcoin-qt.exe` is not part of the Windows installer and is not required for the `deploy` target.
Additionally, in [WINDOWS BUILD NOTES](https://github.com/bitcoin/bitcoin/blob/a9a2b669f3e07266ae739574e9c1cef5af711db7/doc/build-windows.md), `cmake --install build` follows `cmake --build build`, and this sequence always works.
⚠️ saikiran57 opened an issue: "Check if the wallet already contains the descriptor GetDescriptorScriptPubKeyMan"
(https://github.com/bitcoin/bitcoin/issues/32013)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
In importdescriptors rpc call when we try import descriptor address we are internally calling ProcessDescriptorImport() method which checking
` // Check if the wallet already contains the descriptor
auto existing_spk_manager = wallet.GetDescriptorScriptPubKeyMan(w_desc);
if (existing_spk_manager) {
if (!existing_spk_manager->CanUpdateToWa
...
(https://github.com/bitcoin/bitcoin/issues/32013)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
In importdescriptors rpc call when we try import descriptor address we are internally calling ProcessDescriptorImport() method which checking
` // Check if the wallet already contains the descriptor
auto existing_spk_manager = wallet.GetDescriptorScriptPubKeyMan(w_desc);
if (existing_spk_manager) {
if (!existing_spk_manager->CanUpdateToWa
...
🤔 hebasto reviewed a pull request: "doc: add note to Windows build about stripping bins"
(https://github.com/bitcoin/bitcoin/pull/32002#pullrequestreview-2666503323)
Approach ACK 9d0311d5bda104afaa8a89aa6505d683610609de.
(https://github.com/bitcoin/bitcoin/pull/32002#pullrequestreview-2666503323)
Approach ACK 9d0311d5bda104afaa8a89aa6505d683610609de.
💬 hebasto commented on pull request "doc: add note to Windows build about stripping bins":
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1984606195)
```suggestion
After building using the Windows subsystem, it can be useful to copy the compiled
```
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1984606195)
```suggestion
After building using the Windows subsystem, it can be useful to copy the compiled
```
💬 hebasto commented on pull request "doc: add note to Windows build about stripping bins":
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1984606657)
```suggestion
an install prefix using `CMAKE_INSTALL_PREFIX`. For example, to install to `c:\workspace\bitcoin`,
```
(https://github.com/bitcoin/bitcoin/pull/32002#discussion_r1984606657)
```suggestion
an install prefix using `CMAKE_INSTALL_PREFIX`. For example, to install to `c:\workspace\bitcoin`,
```
📝 hebasto opened a pull request: "ci: Do not try to install for fuzz builds"
(https://github.com/bitcoin/bitcoin/pull/32014)
This PR is a follow-up to https://github.com/bitcoin/bitcoin/pull/31844 and extends the changes from fb0546b1c5ebb858605bef4c9fa001782e0ab213 to all fuzz builds in the CI.
Fixes https://github.com/bitcoin/bitcoin/issues/32001.
(https://github.com/bitcoin/bitcoin/pull/32014)
This PR is a follow-up to https://github.com/bitcoin/bitcoin/pull/31844 and extends the changes from fb0546b1c5ebb858605bef4c9fa001782e0ab213 to all fuzz builds in the CI.
Fixes https://github.com/bitcoin/bitcoin/issues/32001.
💬 hebasto commented on issue "build: document `BITCOIN_GENBUILD_NO_GIT` environment variable?":
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2705830610)
FWIW, `BITCOIN_GENBUILD_NO_GIT` was introduced in https://github.com/bitcoin/bitcoin/pull/7522.
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2705830610)
FWIW, `BITCOIN_GENBUILD_NO_GIT` was introduced in https://github.com/bitcoin/bitcoin/pull/7522.
💬 hebasto commented on issue "build: document `BITCOIN_GENBUILD_NO_GIT` environment variable?":
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2705837980)
> > in some build systems
>
> Which build systems? This should be documented with a specific example, especially since it seems like an uncommon usecase/workflow.
Gentoo, I assume: https://bugs.gentoo.org/476294.
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2705837980)
> > in some build systems
>
> Which build systems? This should be documented with a specific example, especially since it seems like an uncommon usecase/workflow.
Gentoo, I assume: https://bugs.gentoo.org/476294.
💬 hebasto commented on issue "cmake: (release) version handling is broken":
(https://github.com/bitcoin/bitcoin/issues/31898#issuecomment-2705851491)
Should this issue still be tagged for v29.0? If so, what immediate actions are expected?
(https://github.com/bitcoin/bitcoin/issues/31898#issuecomment-2705851491)
Should this issue still be tagged for v29.0? If so, what immediate actions are expected?
💬 hebasto commented on issue "build: `-static-pie` builds no-longer working with CMake":
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2705863417)
> You can make things work using `-DAPPEND_LDFLAGS`, but using this (non-standard workaround) shouldn't be necessary.
This can be achieved by introducing a dedicated build option, such as `ENABLE_STATIC_PIE`, if that's acceptable.
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2705863417)
> You can make things work using `-DAPPEND_LDFLAGS`, but using this (non-standard workaround) shouldn't be necessary.
This can be achieved by introducing a dedicated build option, such as `ENABLE_STATIC_PIE`, if that's acceptable.
💬 Juma-creator commented on issue "build: `-static-pie` builds no-longer working with CMake":
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2705905947)
. **Replace Project Name and Version:**
- Change `project(MyProject VERSION 1.0)` to your project's name and version.
- Example:
```cmake
project(YourProjectName VERSION 2.0)
```
2. **Specify C++ Standard:**
- Adjust the C++ standard if necessary.
- Example:
```cmake
set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
```
3. **Organize Project Structure:**
- Adjust the directory structure to match your project. Ensure your source fil
...
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2705905947)
. **Replace Project Name and Version:**
- Change `project(MyProject VERSION 1.0)` to your project's name and version.
- Example:
```cmake
project(YourProjectName VERSION 2.0)
```
2. **Specify C++ Standard:**
- Adjust the C++ standard if necessary.
- Example:
```cmake
set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
```
3. **Organize Project Structure:**
- Adjust the directory structure to match your project. Ensure your source fil
...
💬 fanquake commented on issue "build: document `BITCOIN_GENBUILD_NO_GIT` environment variable?":
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2705911816)
> Gentoo, I assume: https://bugs.gentoo.org/476294.
Reading that thread, it's not really clear. The last comment (7 years ago) suggests that there was no issue with 0.13.0, even though the fix from https://github.com/bitcoin/bitcoin/pull/7522 only went into 0.15.0? It'd be good if someone could demonstrate an issue today, with the current source code.
(https://github.com/bitcoin/bitcoin/issues/31999#issuecomment-2705911816)
> Gentoo, I assume: https://bugs.gentoo.org/476294.
Reading that thread, it's not really clear. The last comment (7 years ago) suggests that there was no issue with 0.13.0, even though the fix from https://github.com/bitcoin/bitcoin/pull/7522 only went into 0.15.0? It'd be good if someone could demonstrate an issue today, with the current source code.
💬 fanquake commented on issue "build: `-static-pie` builds no-longer working with CMake":
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2705919305)
> introducing a dedicated build option, such as ENABLE_STATIC_PIE, if that's acceptable.
I don't think so. We shouldn't have to introduce our own options, to map to compiler options, to workaround CMake issues. This seems like something that needs to be solved in CMake itself.
(https://github.com/bitcoin/bitcoin/issues/31843#issuecomment-2705919305)
> introducing a dedicated build option, such as ENABLE_STATIC_PIE, if that's acceptable.
I don't think so. We shouldn't have to introduce our own options, to map to compiler options, to workaround CMake issues. This seems like something that needs to be solved in CMake itself.
💬 Sjors commented on pull request "Remove Taproot activation height":
(https://github.com/bitcoin/bitcoin/pull/26201#discussion_r1984719254)
Done
(https://github.com/bitcoin/bitcoin/pull/26201#discussion_r1984719254)
Done
💬 dergoegge commented on pull request "ci: Do not try to install for fuzz builds":
(https://github.com/bitcoin/bitcoin/pull/32014#discussion_r1984723062)
Why does this need to change? The regular msan job is not broken afaik.
(https://github.com/bitcoin/bitcoin/pull/32014#discussion_r1984723062)
Why does this need to change? The regular msan job is not broken afaik.
💬 fanquake commented on pull request "Implement OP_CHECKSIGFROMSTACK(VERIFY)":
(https://github.com/bitcoin/bitcoin/pull/29270#issuecomment-2705951177)
@reardencode It's no-longer possible to re-open:

You'll need to open a new PR.
(https://github.com/bitcoin/bitcoin/pull/29270#issuecomment-2705951177)
@reardencode It's no-longer possible to re-open:

You'll need to open a new PR.
💬 Sjors commented on issue "CI: Cmake warnings should be errors":
(https://github.com/bitcoin/bitcoin/issues/31476#issuecomment-2705965427)
This doesn't seem release blocking to me.
The only open PR that points to this issue is #31665 which isn't marked for v29.
(https://github.com/bitcoin/bitcoin/issues/31476#issuecomment-2705965427)
This doesn't seem release blocking to me.
The only open PR that points to this issue is #31665 which isn't marked for v29.
💬 fanquake commented on issue "build: cmake --install fails after --target deploy":
(https://github.com/bitcoin/bitcoin/issues/32007#issuecomment-2705974429)
Note that this isn't Windows specific. The same happens for macOS build.
(https://github.com/bitcoin/bitcoin/issues/32007#issuecomment-2705974429)
Note that this isn't Windows specific. The same happens for macOS build.
💬 hodlinator commented on pull request "miniscript: fixes #29098 by only use first k valid signatures":
(https://github.com/bitcoin/bitcoin/pull/31719#discussion_r1984747128)
Thanks! Although you are still inserting the loop in the middle of the old comment. How about this? (Or putting the `nsat_return`-block with comment last, not sure what makes most sense).
```C++
// for nsat_return, it expects node.keys.size() number of ZEROs, thus adding this for loop
InputStack nsat_return;
for (size_t i = 0; i < node.keys.size(); ++i) {
nsat_return = std::move(nsat_return) + ZERO;
}
auto& nsat{nsat_return};
// The dissatisfaction consists of as many empty vectors a
...
(https://github.com/bitcoin/bitcoin/pull/31719#discussion_r1984747128)
Thanks! Although you are still inserting the loop in the middle of the old comment. How about this? (Or putting the `nsat_return`-block with comment last, not sure what makes most sense).
```C++
// for nsat_return, it expects node.keys.size() number of ZEROs, thus adding this for loop
InputStack nsat_return;
for (size_t i = 0; i < node.keys.size(); ++i) {
nsat_return = std::move(nsat_return) + ZERO;
}
auto& nsat{nsat_return};
// The dissatisfaction consists of as many empty vectors a
...
💬 Sjors commented on issue "cmake inconsistently overriding `-O3` (sometimes)":
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2705984429)
Should we just add a release note about this issue? IIUC it's a regression in our move to cmake, but I suspect that anyone who actually wants to use `-O3` can use master (once it's fixed) or will be happy to review a backport.
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2705984429)
Should we just add a release note about this issue? IIUC it's a regression in our move to cmake, but I suspect that anyone who actually wants to use `-O3` can use master (once it's fixed) or will be happy to review a backport.