Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👍 TheCharlatan approved a pull request: "cmake: add `USE_SOURCE_PERMISSIONS` to all `configure_file()` usage"
(https://github.com/bitcoin/bitcoin/pull/30823#pullrequestreview-2287108659)
ACK 1f054eca4e779cfa5f9f6e9278071adf65e5eafe
💬 TheCharlatan commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2334764375)
> This is a confusing CI failure:

I'm guessing you saw the actual error further up?
```
In file included from /ci_container_base/src/test/ipc_test.cpp:7:
/ci_container_base/ci/scratch/build-i686-pc-linux-gnu/src/test/ipc_test.capnp.h:18:10: fatal error: '../ipc/capnp/mining.capnp.h' file not found
18 | #include "../ipc/capnp/mining.capnp.h"
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[ 5%] Building CXX object CMakeFiles/minisketch.dir/src/minisketch/src/fields/generic_4bytes.cpp.
...
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1747285877)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1732637930

> Nit (clang-format): Missing space after `template`.

Thanks, fixed!
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1747694722)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1747656292

> Did you move these out of here because you'd otherwise get the following error:

Yes, that's the reason. It doesn't seem to be possible to access `src/ipc/` files from the files in the `src/test/` directory by writing `../ipc/` when the `capnp compile` source prefix is `src/test/`, but it does work if the source prefix is `src/`.

I think a different way to work around this would be to pass `--import-path=/path/to/
...
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2334787042)
re: https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2334764375

> I'm guessing you saw the actual error further up?

No thanks. I missed that. That is also confusing but at least it's something to work with. I'm sure why it doesn't happen in normal non-CI builds or seem to happen in the followup PR #30437. Will try to reproduce and fix
💬 jonatack commented on pull request "Add NODE_TXRELAY_V2.":
(https://github.com/bitcoin/bitcoin/pull/30837#discussion_r1747701520)
(in case you're not waiting for concept acks)

```
/src/protocol.cpp:94:13: error: enumeration value 'NODE_TXRELAY_V2' not handled in switch [-Werror,-Wswitch]
94 | switch ((ServiceFlags)service_flag) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
```
💬 hebasto commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2334792022)
> it seems like a very clean one is to set the [`CMAKE_USER_MAKE_RULES_OVERRIDE`](https://cmake.org/cmake/help/v3.3/variable/CMAKE_USER_MAKE_RULES_OVERRIDE.html) hook

FWIW, something similar was [considered](https://github.com/hebasto/bitcoin/pull/240) during work on the CMake staging branch.
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#issuecomment-2334799124)
Oh, I guess this is failing because it is a parallel build and cmake does not know the generate `src/test/ipc_test.capnp.h` file depends on the generated `ipc/capnp/mining.capnp.h` file, so can fail if these do not happen to be built in the right order. Will need to annotate that somehow.
💬 TheCharlatan commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1747712685)
That does not really seems less clear to me, but maybe I'm missing something. Using the `--import-path` argument seems akin to the `-I` option I'd pass to a compiler.
💬 TheCharlatan commented on pull request "build: Use CMake's default permissions in macOS `deploy` target":
(https://github.com/bitcoin/bitcoin/pull/30838#issuecomment-2334805688)
apple builds:
```
a4840ae0607332cb728e228023de4e77fbe2303d0d85df433ee39e9f2ab4e2e7 guix-build-31f4d1ce83a5/output/arm64-apple-darwin/SHA256SUMS.part
93ba6f8d2f3a849f7f2aa380a9996fc3685cebe1c09aea18a976ce7421622c45 guix-build-31f4d1ce83a5/output/arm64-apple-darwin/bitcoin-31f4d1ce83a5-arm64-apple-darwin-unsigned.tar.gz
47319f231c1fd4a74bfd52f7a62f012353df77762b46d9cc04c2f00001365da2 guix-build-31f4d1ce83a5/output/arm64-apple-darwin/bitcoin-31f4d1ce83a5-arm64-apple-darwin-unsigned.zip
6eeb
...
💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1747723657)
Apparently, there is no need to enable the `CMP0141` policy, as we have not used the `CMAKE_MSVC_DEBUG_INFORMATION_FORMAT` variable since https://github.com/hebasto/bitcoin/pull/215.

Fixed in https://github.com/bitcoin/bitcoin/pull/30803.
💬 achow101 commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2334821947)
Only did one run of each but I have basically the opposite result: 4406 seconds for 27.1, and 2949 for 28.0rc1.
jlest01 closed an issue: "Misleading index error message"
(https://github.com/bitcoin/bitcoin/issues/30836)
💬 jlest01 commented on issue "Misleading index error message":
(https://github.com/bitcoin/bitcoin/issues/30836#issuecomment-2334845889)
Indeed. Thanks.
💬 andrewtoth commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2334887290)
@vostrnad when you say synchronizes, do you mean running with `-reindex-chainstate` or are you starting with a fresh data directory and downloading blocks from peers? If the latter, are you downloading blocks from a node in your local network?
💬 vostrnad commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2334894401)
@andrewtoth I updated the OP. I'm synchronizing from a local node with a fresh datadir each time.
💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1747778418)
Thanks. Updated with three verbosity levels (0, 1, and 2).
💬 ryanofsky commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2334900625)
> I don't think this describes the current behaviour precisely.

Yes this is definitely not describing current behavior, it's trying to describe how current behavior "could be changed" to avoid writing the cache. The first part of https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2329412411 describes ideal behavior and contrasts it with current behavior, and the second part describes a specific way I think ideal behavior could be implemented.

I think current behavior is ok, but i
...
💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2334910034)
The push to f6dee50f85b2668e96e9dffd65e06d8f3d3a4da2 adds verbosity levels and more detailed information about the orphans.
💬 ryanofsky commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2334911382)
> > it seems like a very clean one is to set the [`CMAKE_USER_MAKE_RULES_OVERRIDE`](https://cmake.org/cmake/help/v3.3/variable/CMAKE_USER_MAKE_RULES_OVERRIDE.html) hook
>
> FWIW, something similar was [considered](https://github.com/hebasto/bitcoin/pull/240) during work on the CMake staging branch.

Interesting! Although that PR was using CMAKE_USER_MAKE_RULES_OVERRIDE it was doing something pretty different with it. It was overriding cmake's low level compile command lines, which is defini
...