ci: Re-add configs removed in cmake migration #30740
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
| @@ -72,7 +72,7 @@ jobs: | |||
| run: | | |||
| # Run tests on commits after the last merge commit and before the PR head commit | |||
| # Use clang++, because it is a bit faster and uses less memory than g++ | |||
| git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && CC=clang CXX=clang++ cmake -B build -DBUILD_GUI=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DWITH_BDB=ON -DWITH_NATPMP=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON && cmake --build build -j $(nproc) && ctest --test-dir build -j $(nproc) && ./build/test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }} | |||
| git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && CC=clang CXX=clang++ cmake -B build -DWITH_ZMQ=ON -DBUILD_GUI=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DWITH_BDB=ON -DWITH_NATPMP=ON -DWITH_MINIUPNPC=ON -DWITH_USDT=ON && cmake --build build -j $(nproc) && ctest --test-dir build -j $(nproc) && ./build/test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }} | |||
There was a problem hiding this comment.
Should this job also be installing libzmq3-dev ?
There was a problem hiding this comment.
I am using python3-zmq as a proxy for that, which should probably be enough, no? And if libzmq was missing, the CI would be failing, no?
There was a problem hiding this comment.
Sure, it's likely fine, just inconsistent with the other CIs.
There was a problem hiding this comment.
Yeah, I am happy to review a different pull request touching those lines, but I haven't touched them in this pull request, so I'll leave this as-is for now.
There was a problem hiding this comment.
Looks like this doesn't work, given: https://github.com/bitcoin/bitcoin/actions/runs/10612371177/job/29413871228?pr=30749 ?
There was a problem hiding this comment.
Yeah, sorry. This CI task doesn't run on pulls with one commit only.
There was a problem hiding this comment.
Jup, I was wrong. Do you mind fixing it up in your pull?
There was a problem hiding this comment.
No worries, I've pushed a commit into #30749.
Nice catch. It's not clear why a bunch of CI configs would have been removed.. |
Sorry for overlooking them. |
No worries. It is my fault for not reviewing them properly before. Also, it looks like the other reviewers missed them as well, so not your fault again. |
ee22bf5 doc: Update and amend MSVC build guide (Hennadii Stepanov) c07fdd6 fuzz: Don't compile BDB-specific code on MSVC in `wallet_bdb_parser.cpp` (Hennadii Stepanov) e07a3ed ci: Add missed configuration options to "Win64 native" job (Hennadii Stepanov) Pull request description: Some build options were overlooked when the CMake staging branch dropped package autodetection. This PR restores them. Similar to #30740. ACKs for top commit: maflcko: review-only ACK ee22bf5 fanquake: ACK ee22bf5 Tree-SHA512: dbbfdf4347fbcda6ee24f7bd6041c383cfd9853fa717cfe0fbf2474cdd28435eba96da51c01684967c007b5346532c14fd923fcc3428efd607690a42175e39ad
In commit 9730288 many configs were removed from the CI without explanation.
Fix it by adding them back.
Can be reviewed by looking at: