Repository navigation
ci: Compile with minimum supported g++-10 in olddeps - #366
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline and AI policy for information on the review process.
If your review is incorrectly listed, please copy-paste ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
5bf9e90 to
fa56334
Compare
| CI_DIR=build-olddeps | ||
| # Pin olddeps to an older Nixpkgs channel, since compiling the old CMake | ||
| # requires an older GCC. | ||
| # Pin olddeps to an older Nixpkgs channel, to be able to test an older GCC. |
There was a problem hiding this comment.
Possible followup: this comment could keep the old reason too, and say that 25.05 is the last channel with GCC 10 (25.11 and unstable only have gcc13 and later), so whoever bumps the channel knows the job's GCC version has to change with it. Maybe something like:
# Pin olddeps to an older Nixpkgs channel, since nixos-25.05 is the last
# channel providing GCC 10, and compiling the old CMake requires an older GCC.| CI_NIXPKGS_CHANNEL=nixos-25.05 | ||
| export CXXFLAGS="-Werror -Wall -Wextra -Wpedantic -Wno-unused-parameter -Wno-error=array-bounds" | ||
| NIX_ARGS=(--argstr capnprotoVersion "0.9.2" --argstr cmakeVersion "3.12.4" --argstr gccVersion "11") | ||
| CAPNP_CHECKOUT=v0.9.2 # Use a checkout to compile it with the selected GCC |
There was a problem hiding this comment.
Possible followup: it might not be obvious why this builds Cap'n Proto from source when nix has a package for it. The comment could say what this is trying to test, something like:
# Build Cap'n Proto from source so it is compiled with the same old GCC, like
# on a system where that GCC is the default compiler. Nix packages are built
# with the channel's default GCC instead.
ryanofsky
left a comment
There was a problem hiding this comment.
Code review ACK fa56334. Thanks for also building Cap'n Proto (not just libmultiprocess) with GCC 10.
Some background for anyone reading later:
- This PR updates the olddeps CI job, which checks that libmultiprocess works with the oldest supported dependencies. It lowers the job's compiler from GCC 11 to GCC 10, the oldest version known to work, and builds Cap'n Proto 0.9.2 from source with GCC 10 instead of using the nix package. This is now the only CI job that compiles with GCC 10.
- Cap'n Proto is built from source because nix builds its packages with the channel's default compiler, GCC 14. Before this PR, the job compiled libmultiprocess with GCC 11 and linked it against a Cap'n Proto built with GCC 14.
- Which GCC builds Cap'n Proto doesn't matter very much, because Cap'n Proto's headers were already compiled with the old GCC as part of building libmultiprocess, so most problems an old compiler could cause would have shown up anyway. Building both with GCC 10 is more realistic, though: someone using GCC 10 is probably on a system where it is the default compiler, and builds Cap'n Proto with it too. It also checks that the oldest supported Cap'n Proto and GCC versions really work together.
- The new setup works: in the olddeps CI log, Cap'n Proto and libmultiprocess are both compiled with GCC 10.5.0, and the tests pass.
- The
ci.shchange is needed because building Cap'n Proto from source rancmake --install, which requires CMake 3.15, and this job uses CMake 3.12.cmake --build . --target installdoes the same thing on older versions. - The job stays pinned to nixos-25.05 because it is the last nixpkgs channel with GCC 10, and building the old CMake also needs an older GCC.
I left two small comment suggestions for a followup but plan to merge as is.
6fd2a9c90e Merge bitcoin-core/libmultiprocess#367: mpgen: Change Cap'n Proto link order to fix undefined references in static builds f1042b1a60 Merge bitcoin-core/libmultiprocess#366: ci: Compile with minimum supported g++-10 in olddeps a6ecdc113f Merge bitcoin-core/libmultiprocess#364: util: Error out at compile time if KJ_NO_EXCEPTIONS is set 56cfdaa884 Change CapnProto library linkage for mpgen fa5633467a ci: Compile with minimum supported g++-10 in olddeps 4a1f400f8b util: Error out at compile time if KJ_NO_EXCEPTIONS is set git-subtree-dir: src/ipc/libmultiprocess git-subtree-split: 6fd2a9c90e1e082635600f338a42e088fb6dc4b0
Follow up to #330 (comment)