Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 maflcko commented on pull request "ci: Use macos-14 GHA image":
(https://github.com/bitcoin/bitcoin/pull/30913#issuecomment-2354837423)
### Master

* https://github.com/bitcoin/bitcoin/actions/runs/10895856163/job/30234757205#step:3:12

```
Apple clang version 15.0.0 (clang-1500.0.40.1)
Target: x86_64-apple-darwin22.6.0
Thread model: posix
InstalledDir: /Applications/Xcode_15.0.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
```

* https://github.com/bitcoin/bitcoin/actions/runs/10895856163/job/30234757205#step:7:2006: (43 minutes with 21% ccache hit rate)

### Pull

* https://github.com/bitc
...
💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2354840124)
> (Unrelated to this issue, one could consider moving to the M1 runners, given that they may speed up the build. Is the removal of `sudo xcode-select --switch /Applications/Xcode_15.0.app` required? Personally I find it useful to have the minimum supported xcode version documented and checked by CI.)

Proposed alternative in https://github.com/bitcoin/bitcoin/pull/30913
💬 maflcko commented on pull request "doc: update NeedsRedownload() and nStatus comment":
(https://github.com/bitcoin/bitcoin/pull/29624#issuecomment-2354854297)
re-ACK af9f9878934f88036423021c70ef523b6c9e1c90
🤔 willcl-ark reviewed a pull request: "ci: Use macos-14 GHA image"
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2308747588)
Concept ACK.

Very much in favour of speeding these jobs up! Changes look good to me so far, just one Q on the jobname with this change in architecture.
💬 willcl-ark commented on pull request "ci: Use macos-14 GHA image":
(https://github.com/bitcoin/bitcoin/pull/30913#discussion_r1762621194)
This is still building using x86_64-apple-darwin as the host (from 00_setup_env_mac_native.sh). I wonder if the "native" here is therefore confusing?
💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2354875962)
I am seeing a 2h timeout on running `clang --version`, even before any CI-specific code is run: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/10897268811/job/30238399156

So I fail to see how this could be anything other than a GitHub hardware, software, or data corruption bug.

I don't think this can be fixed on our side, apart from removing the task.
💬 TheCharlatan commented on pull request "depends: sqlite 3.46.1":
(https://github.com/bitcoin/bitcoin/pull/29991#issuecomment-2354884459)
Guix build (aarch64):
```
773a4d0dbc05ceb91439802ce3374b252f45d2a4c69c3fa5930d9450d8c1f340 guix-build-def6dd0c597f/output/aarch64-linux-gnu/SHA256SUMS.part
c7719110515db339e6ca87f8255338eb21ed2df2fa89e2ae72a10860a6fff2a5 guix-build-def6dd0c597f/output/aarch64-linux-gnu/bitcoin-def6dd0c597f-aarch64-linux-gnu-debug.tar.gz
83a75cb1242f725f234e16799791890d0aebd2efab1632ac3db300b73ad664a9 guix-build-def6dd0c597f/output/aarch64-linux-gnu/bitcoin-def6dd0c597f-aarch64-linux-gnu.tar.gz
f6d09f3839
...
💬 maflcko commented on pull request "ci: Use macos-14 GHA image":
(https://github.com/bitcoin/bitcoin/pull/30913#discussion_r1762647527)
That sounds like a bug in the CI config. However, it should be harmless, because `HOST` is unused, because depends isn't used here.

I'll add an unrelated bugfix commit to remove the wrong config value.
💬 stratospher commented on pull request "addrman: change internal id counting to int64_t":
(https://github.com/bitcoin/bitcoin/pull/30568#discussion_r1762661233)
I'm fine with the version on the branch now because `Serialise()` and `Unserialise()` are symmetrical. `nIndex` is [written as int](https://github.com/bitcoin/bitcoin/blob/51f7668d31e2624e41c7ce77fe33162802808f3f/src/addrman.cpp#L220) into peers.dat in `Serialise()` and [read as int](https://github.com/bitcoin/bitcoin/blob/51f7668d31e2624e41c7ce77fe33162802808f3f/src/addrman.cpp#L325) from peers.dat in `Unserialise()`.
💬 stratospher commented on pull request "addrman: change internal id counting to int64_t":
(https://github.com/bitcoin/bitcoin/pull/30568#issuecomment-2354910571)
ACK 51f7668d31e2624e41c7ce77fe33162802808f3f. I think it's a good change to make the nId space large(64 bits) so that the nId values are distinct.
🤔 hebasto reviewed a pull request: "build: speed up by flattening the dependency graph"
(https://github.com/bitcoin/bitcoin/pull/30911#pullrequestreview-2308818404)
Concept ACK.

a39a915c226775396f6505efe77a4aba4d0ae3ad
Did you consider setting `CMAKE_ADD_CUSTOM_COMMAND_DEPENDS_EXPLICIT_ONLY` globally?

aaf30ef89465c18228b4e46fd1378b9087274cbe
> turns out this triggers an [upstream bug](https://gitlab.kitware.com/cmake/cmake/-/issues/24058), which I guess can be considered an edge-case until fixed in CMake. I've added 2 per-lib opt-outs as a result.

This bug may also affect the `crc32c` library after [switching](https://github.com/bitcoin/bitcoin/p
...
💬 Sjors commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1762668345)
02c6a1ed58867fa14b1e0c42f476ef45e64ea7ea `-> (result: BlockTemplate);`
🚀 fanquake merged a pull request: "guix: (explicitly) build Linux GCC with `--enable-cet`"
(https://github.com/bitcoin/bitcoin/pull/30438)
💬 maflcko commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2354922939)
> ... though currently only Ninja benefits from this being set, and only CMake >= 3.27 understands it.

I wonder if one CI task should be using Ninja (and cmake >= 3.27), if it isn't too hard to implement. Otherwise this config will remain untested and errors may sneak in to the master branch, only being detected after merge.
💬 hebasto commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#discussion_r1762679548)
A note for reviewers. The Guix builds are using CMake 3.24.2 (the `cmake-minimal` package).
🤔 hebasto reviewed a pull request: "ci: Use macos-14 GHA image"
(https://github.com/bitcoin/bitcoin/pull/30913#pullrequestreview-2308880257)
Concept ACK.

Perhaps explicitly mention the switch from the `x86_64` architecture to `arm64`?
👋 fanquake's pull request is ready for review: "depends: Qt 5.15.15"
(https://github.com/bitcoin/bitcoin/pull/30774)
💬 fanquake commented on pull request "depends: Qt 5.15.15":
(https://github.com/bitcoin/bitcoin/pull/30774#issuecomment-2355041535)
> but wasn't the idea to use qt6 once cmake was done? IIRC there was a tracking/meta issue, but I can't find it?

Maybe #24798? It's not really clear what the status of any migration is.
💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2355045583)
> So I think the best way forward would be to have all workers of the same type on the same machine. (I am still running some performance tests to get an estimate if this make sense at all)

Did a quick sanity check with a 100G shared ccache on a single machine `cpx51` and all tasks finished in under a hour. Some tasks were faster than the AX52 run above, some were slower:

![Screenshot 2024-09-17 at 11-11-05 Merge commit 'fa99e4521b6fc0e7f6636d40bc0d6a7325227374' into main-with-ci - Cirrus
...
💬 maflcko commented on pull request "ci: Use macos-14 GHA image (x86_64-apple-darwin22.6.0 -> arm64-apple-darwin23.6.0)":
(https://github.com/bitcoin/bitcoin/pull/30913#issuecomment-2355068096)
> Perhaps explicitly mention the switch from the `x86_64` architecture to `arm64`?

Sure, done