backport: bitcoin#19179, #19983, #20451, #20543, #20545, #20572, #20615, #20658, #20680, #20682, #20691, #20697, #20817, partial #20451#5957
Merged
Conversation
fa7a438 ci: Fix doc typos in .cirrus.yml (MarcoFalke) fa73674 ci: Run i686 centos ci config on cirrus (MarcoFalke) fa1f949 ci: Run nowallet ci config on cirrus (MarcoFalke) Pull request description: Travis CI Org is shutting down, so move the configs to cirrus ci ACKs for top commit: practicalswift: ACK fa7a438: patch looks correct Tree-SHA512: 1b7125c7f0d2288931fb8c5e90345891d5f7c1f00c4af136afbeb36bd2836f72920a1877dacc7be787e2c8d84de2a733c1ca71931e5c101b222c1fea588619b3
added 2 commits
March 26, 2024 21:15
2b35611 ci: no-longer exclude feature_block in TSAN job (fanquake) Pull request description: The TSAN job is now running on Cirrus. Increase the allocated memory to the [maximum allowed](https://cirrus-ci.org/guide/linux/#linux-containers). ACKs for top commit: jonasschnelli: utACK 2b35611 - checked the CI run and confirmed that the feature_block runs: https://cirrus-ci.com/task/6008403543719936?command=ci#L3249 MarcoFalke: review ACK 2b35611 Tree-SHA512: b774995600361c74bc3267b566e12add66a4604bdf88f6e3f69669edbb8d7aff6f20fdbf0ef98187be4730ce4e18b1939bbcecd993a5c5c1ff40b237c7921b71
fa5c4f1 ci: Adjust cirrus ci task names (MarcoFalke) Pull request description: The task names are too long for GitHub to display them properly without truncation in the "checks-view". Fix that by using a new naming scheme: * Native builds don't mention "x86_64 Linux", as it is redundant, they do mention the OS (bionic or focal) in the name suffix * Cross builds mention the target in the prefix and the OS (always bionic) in the suffix * the macos native build simply says "macos native" ACKs for top commit: practicalswift: ACK fa5c4f1: patch looks correct! hebasto: ACK fa5c4f1, I have reviewed the code and it looks OK, I agree it can be merged. RiccardoMasutti: ACK fa5c4f1 Tree-SHA512: 856deb0577c97c70069ef1d369991addc49522135c0ad9e382218fd79ba3d55a95d6c601288dcef0510764b92fbd30a9d7de32b08dc5be55482deab14049b892
knst
force-pushed
the
bp-v22-p1
branch
2 times, most recently
from
March 26, 2024 14:40
53b0bf0 to
32c1125
Compare
PastaPastaPasta
previously approved these changes
Mar 26, 2024
UdjinM6
requested changes
Mar 26, 2024
UdjinM6
left a comment
UdjinM6
left a comment
There was a problem hiding this comment.
linter failed with
contrib/devtools/github-merge.py:183: error: Not all arguments converted during string formatting
Found 1 error in 1 file (checked 239 source files)
PastaPastaPasta
self-requested a review
March 26, 2024 15:14
knst
marked this pull request as draft
March 26, 2024 15:23
UdjinM6
requested changes
Mar 26, 2024
667b6a2 ci: Adjust Cirrus CI task names (follow up) (Hennadii Stepanov) Pull request description: "no depends" implies "only system libs". bitcoin#20545 follow up . ACKs for top commit: practicalswift: ACK 667b6a2 Tree-SHA512: b3bf5dc931b7aeed30932e8d6c5f5b0040e4be5a5676ed23b41675ebad2f8a016b1abb27584b68970ee00bc5b2af7122ca9566f373db5f3a2e38fcf7eee6522a
…faster CI feedback faf2c6e cirrus: Schedule one task with paid credits for faster CI feedback (MarcoFalke) Pull request description: During times of high activity in the repo, the scheduling of Cirrus CI tasks might put them a few hours in the future. This is fine when all the tasks eventually pass. Though for failing tasks, a failure should ideally be shown to the author and reviewer as soon as possible. Compute credits can be used to schedule immediately: https://cirrus-ci.org/pricing/#compute-credits. Running all tasks with compute credits will probably be more expensive than our previous CI invoice. However, they are also more flexible. As a start we could enable only a single task and revisit/re-evaluate the next steps in a month. ACKs for top commit: laanwj: Concept ACK faf2c6e fanquake: ACK faf2c6e practicalswift: cr ACK faf2c6e: patch looks correct Tree-SHA512: df599e1c4cc0394f7f03413ad29954ddc87b163b02640d8bfc0497a5dc8d237b8c963c1dd9d01ac83c4a044f575300763097dac2ea6d1a4a163a1cece342b743
4045a67 ci: Use cpu=1 for linter (Dhruv Mehta) 739d390 ci: Move linter task to cirrus (Dhruv Mehta) Pull request description: Solves bitcoin#20467: Move linter to Cirrus-CI as Travis-CI.org is shutting down ACKs for top commit: MarcoFalke: ACK 4045a67 Tree-SHA512: 9aa7487ac86c91fc68bb584d29134e304dbd46702514a5d47d1ef0de6b877d96d42b7589870fc67ad9a31f5d3a789728446da4418688f336111a9ba0f8de5feb
…in repo facf5e3 ci: Only use credits for pull requests to the main repo (MarcoFalke) Pull request description: No need to spend credits for the `master` branch, because the build shouldn't fail there anyway and it is not time-critical to get a fast feedback. Some other changes: * Disable `stateful` for faster scheduling * Reduce lint memory from 8G to 1G for faster scheduling ACKs for top commit: hebasto: ACK facf5e3, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 4d052e68217086574b9ea3d603cde1b585833c289d47dfed5308ff001d02964dc75ec3b3ebf5b233ccd09c47ad4ff5ba0bef639bf6362d984e7c49fca8fec24b
95487b0 doc: Drop mentions of Travis CI as it is no longer used (Hennadii Stepanov) 09d105e ci: Drop travis_fold feature as Travis CI is no longer used (Hennadii Stepanov) Pull request description: As Travis CI is no longer used, this PR: - drops `travis_fold` feature - drops mentions of Travis CI in docs ACKs for top commit: MarcoFalke: ACK 95487b0 Tree-SHA512: 2e259bb8b1e37bcefc1251737bb2716f06ddb57c490010b373825c4e70f42ca38efae69a2f63f21f577d7cee3725b94097bdddbd313f8ebf499281cf97c53cef
faeb40b ci: Install missing lint packages (MarcoFalke) Pull request description: The cirrus container is vanilla ubuntu, so we need to install the needed packages ACKs for top commit: hebasto: ACK faeb40b, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: e56198108e26ea0ba2a344b1b74bc294652f34e9866cca053a25fb1b83bbd87ea40254c340e5e169fdfcbd4dcb39fdc2078b5157ca729a22a9a1792ec514a33e
3c2478c ci: Print COMMIT_RANGE to the log as it was in Travis CI (Hennadii Stepanov) c123892 ci: Drop Travis-specific workaround for shellcheck (Hennadii Stepanov) 10af252 ci: Drop Travis-specific way to set COMMIT_RANGE variable (Hennadii Stepanov) 93504da ci: Fix COMMIT_RANGE variable value for PRs (Hennadii Stepanov) Pull request description: This PR: - is a bitcoin#20658 and bitcoin#20682 followup - set the `COMMIT_RANGE` variable correctly for PRs - cleans up Travis-specific code - prints COMMIT_RANGE value to the log for convenience as it was in Travis CI ACKs for top commit: MarcoFalke: ACK 3c2478c Tree-SHA512: beb933352b10fd5eb3e66373ddb62439e4f3a03b50fb037ee89fa92c0706cec41d05f2d307f15bb18d1e634e6464f4e123b7e2f88703c8edfd145d8d6eff0b1a
3e15712 Update TSan suppressions (Hennadii Stepanov) Pull request description: It seems possible now to drop some TSan suppressions. Top commit has no ACKs. Tree-SHA512: 94518fd2f3a7168b2989424de0696e42c8f509b833aafbc7e75f4c1180a0b8d9a47f43c50d06b03b26a924643afe86274b2062c9d456c17a68576d19566ed66f
1ef2138 lint: run mypy over contrib/devtools (fanquake) Pull request description: wumpus mentioned on IRC that we don't currently run `mypy` over the `contrib/devtools` directory, and that it would likely be worthwhile given bitcoin#20434. This just adds that dir to the linter, as well as some missing annotations to fix existing errors. Note that now we require Python 3.6 we can make use of variable annotations. master (patched to check contrib devtools): ```bash test/lint/lint-python.sh contrib/devtools/symbol-check.py:154: error: Incompatible types in assignment (expression has type "List[str]", variable has type "str") contrib/devtools/circular-dependencies.py:35: error: Need type annotation for 'deps' (hint: "deps: Dict[<type>, <type>] = ...") contrib/devtools/circular-dependencies.py:67: error: Need type annotation for 'closure' (hint: "closure: Dict[<type>, <type>] = ...") Found 4 errors in 3 files (checked 187 source files) ``` I haven't quite gone as far as to add annotations like ```python CHECKS: Dict[str, List[Tuple[str, Callable[[Any], bool]]]] = {... ``` to `symbol-check.py`. ACKs for top commit: laanwj: ACK 1ef2138 Tree-SHA512: a58c2ece588c640289dc1d35dad5b1b8732788272daa0965d6bf44ee8a7f7c8e8585f94d233ac41c84b9ffcfc97841a00fe2c9acba41f58fd164f01de4b6512b
…ives, bump to codespell 2.0.0 f3ba916 lint: ignore gitian keys file for spelling linter (Sebastian Falbesoner) da289a6 lint: update list of spelling linter false positives (Sebastian Falbesoner) a0022f1 test: bump codespell linter version to 2.0.0 (Sebastian Falbesoner) Pull request description: This small patch updates the ignore list for the spelling linter script (which uses `codespell`), both removing false-positives that are not relevant anymore and adding new ones. As [suggested by jonatack](bitcoin#20762 (comment), whose last name is now also part of the list :)~~. Also changed the linter script to not check the gitian keys file, as [suggested by hebasto](bitcoin#20817 (comment)). The codespell version used is bumped to most recent version 2.0.0, which is more aware of some terms that were previously needed in the ignorelist for v1.17.1, see bitcoin#20817 (comment). Running spelling linter on master branch (repeated findings in the same file are removed to keep the output short): ``` $ ./test/lint/lint-spelling.sh contrib/gitian-keys/keys.txt:16: Atack ==> Attack doc/developer-notes.md:1284: inout ==> input, in out doc/psbt.md:122: Asend ==> Ascend, as end src/bench/verify_script.cpp:27: Keypair ==> Key pair src/blockencodings.h:30: Unser ==> Under, unset, unsure, user src/compressor.h:65: Unser ==> Under, unset, unsure, user src/core_read.cpp:131: presense ==> presence src/index/disktxpos.h:21: blockIn ==> blocking src/net_processing.h:67: anounce ==> announce src/netaddress.h:486: compatiblity ==> compatibility src/primitives/transaction.h:35: nIn ==> inn, min, bin, nine src/qt/bitcoinunits.cpp:101: nIn ==> inn, min, bin, nine src/rpc/blockchain.cpp:2150: nIn ==> inn, min, bin, nine src/rpc/misc.cpp:198: nIn ==> inn, min, bin, nine src/script/bitcoinconsensus.cpp:81: nIn ==> inn, min, bin, nine src/script/bitcoinconsensus.h:63: nIn ==> inn, min, bin, nine src/script/interpreter.cpp:1279: nIn ==> inn, min, bin, nine src/script/interpreter.h:222: nIn ==> inn, min, bin, nine src/script/sign.cpp:17: nIn ==> inn, min, bin, nine src/script/sign.h:39: nIn ==> inn, min, bin, nine src/serialize.h:181: Unser ==> Under, unset, unsure, user src/signet.cpp:142: nIn ==> inn, min, bin, nine src/test/base32_tests.cpp:17: fo ==> of, for src/test/base64_tests.cpp:17: fo ==> of, for src/test/script_tests.cpp:1509: nIn ==> inn, min, bin, nine src/test/sighash_tests.cpp:27: nIn ==> inn, min, bin, nine src/test/validation_tests.cpp:78: excercise ==> exercise src/undo.h:36: Unser ==> Under, unset, unsure, user src/validation.cpp:1403: nIn ==> inn, min, bin, nine src/validation.h:255: nIn ==> inn, min, bin, nine src/wallet/wallet.cpp:1532: nIn ==> inn, min, bin, nine src/wallet/walletdb.cpp:429: Crypted ==> Encrypted test/functional/feature_nulldummy.py:63: unnecssary ==> unnecessary test/functional/wallet_encryption.py:81: crypted ==> encrypted test/functional/wallet_upgradewallet.py:36: fpr ==> for, far, fps ^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/lint-spelling.ignore-words.txt ``` Running spelling linter on PR branch: ``` $ ./test/lint/lint-spelling.sh src/core_read.cpp:131: presense ==> presence src/net_processing.h:67: anounce ==> announce src/netaddress.h:486: compatiblity ==> compatibility src/test/validation_tests.cpp:78: excercise ==> exercise src/wallet/walletdb.cpp:429: Crypted ==> Encrypted test/functional/feature_nulldummy.py:63: unnecssary ==> unnecessary test/functional/wallet_encryption.py:81: crypted ==> encrypted ^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/lint-spelling.ignore-words.txt ``` This list of remaining findings doesn't contain false positives anymore -- the typos are fixed in PR bitcoin#20762. Happy new year! 🍾 ACKs for top commit: hebasto: re-ACK f3ba916, only suggested changes since my [previous](bitcoin#20817 (review)) review. jonatack: ACK f3ba916 I don't know if there are any particular issues with bumping codespell to v2.0.0, but locally running the spelling linter and the cirrus job at https://cirrus-ci.com/task/5004066998714368 both LGTM. Thanks for also verifying and removing the unused words from the ignore list. Tree-SHA512: e92ae6f16c01d4ff3d54f8c3a0ee95e12741f7bfe031d307a785f5cfd8a80525b16b34275f413b914c4a318f5166f9887399c21f2dad9cc7e9be41647042ef37
knst
changed the title
Merged
5 tasks
knst
marked this pull request as ready for review
March 27, 2024 16:36
PastaPastaPasta
added a commit
that referenced
this pull request
Apr 3, 2024
…y default, and document exceptions a47635b feat: drop symbol check from CI to unify with bitcoin, keep it for qt5 only (Konstantin Akimov) 0c38cc3 fix: drop -static-libstc++ from depends/hosts/{linux,mingw32}.mk which is clang-only (Konstantin Akimov) 14a67ee Merge bitcoin#20182: ci: Build with --enable-werror by default, and document exceptions (MarcoFalke) Pull request description: ## Issue being fixed or feature implemented As discovered in #5957: Jobs tsan/ubsan fails with backport bitcoin#20182 it fails, because both tsan and ubsan jobs use clang but all other jobs use gcc. Somehow, after configure there are set incorrect options: ``` clang++-16 -std=c++17 -c -pipe -static-libstdc++ -O1 -Werror -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_DEBUG=1 -I/builds/dashpay/dash/depends/x86_64-pc-linux-gnu/include/ conftest.cpp ``` `clang` doesn't support `-static-llibstdc++` which is supposed to be gcc-only, it cause this failure: ``` clang: error: argument unused during compilation: '-static-libstdc++' [-Werror,-Wunused-command-line-argument] ``` This failure make `autoconf` to think that clang doesn't support `-Werror` and fails with this error. So, you can't activate this flag. ## What was done? Backport bitcoin#20182 and related fixes to make it works ## How Has This Been Tested? CI now succeed with bitcoin#20182. It means, that -Werror activated for clang; also there are not warnings such as: ``` clang: error: argument unused during compilation: '-static-libstdc++' [-Werror,-Wunused-command-line-argument] ``` https://gitlab.com/dashpay/dash/-/jobs/6494328698 ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone Top commit has no ACKs. Tree-SHA512: f912824eaa1ec7513cda7278a3df9e067b0ab48a2d174b18654c8070aa6544bac33a52f494c1e35b4eab10392c1f26df4663e21d12a4dfff7c0a4a6a01ff9551
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Backports from bitcoin v22, mostly CI related, other to improve lints
CI related changes are not really important because we use gitlab runner instead cirrus, but keep them up to date for sake of codebase unification.
What was done?
How Has This Been Tested?
Run unit/functional tests
Breaking Changes
N/A
Checklist: