ci: Use DOCKER_BUILDKIT for lint image #28083
Merged
Conversation
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
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. 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. 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. |
Can be reviewed with: --color-moved=dimmed-zebra --ignore-all-space
maflcko
force-pushed
the
2307-ci-lint-docker-build-
branch
from
fa852a4 to
fa2f18a
Compare
jamesob
approved these changes
Jul 18, 2023
Contributor
jamesob
left a comment
jamesob
left a comment
There was a problem hiding this comment.
ACK fa2f18a
Built the new linter container locally, verified it works as before. Change definitely simplifies things thanks to BUILDKIT.
Curiously, when I exercised the linter (by adding an unused variable to test/functional/feature_dbcrash.py), running the linter with docker run --rm -v $(pwd):/bitcoin -w /bitcoin -it bitcoin-linter didn't actually return a non-zero error code:
11:18:54 james@tacoma src/bitcoin (? ackr/28083.1.MarcoFalke.ci_use_docker_buildkit_f fa2f18a) % docker run --rm -v $(pwd):/bitcoin -w /bitcoin -it bitcoin-linter
src/crypto/ctaes in HEAD currently refers to tree 1b6c31139a71f80245c09597c343936a8e41d021
src/crypto/ctaes in HEAD was last updated in commit 8501bedd7508ac514385806e191aec21ee978891 (tree 1b6c31139a71f80245c09597c343936a8e41d021)
GOOD
src/secp256k1 in HEAD currently refers to tree 375f60db0c4a41474d0e7c2464fae64c1ba712a4
src/secp256k1 in HEAD was last updated in commit 901336eee751de088465e313dd8b500dfaf462b2 (tree 375f60db0c4a41474d0e7c2464fae64c1ba712a4)
GOOD
src/minisketch in HEAD currently refers to tree a749309e1b4edf48091edb57da6de2e8a16498a5
src/minisketch in HEAD was last updated in commit e9f1d8c27261070d209a28570ad36fe91531cdd2 (tree a749309e1b4edf48091edb57da6de2e8a16498a5)
GOOD
src/leveldb in HEAD currently refers to tree bd49d3c60051c1a8d8f030fa4ba7a29237fe7479
src/leveldb in HEAD was last updated in commit 1a463c70a368fb20316e03efac273438cf47baa3 (tree bd49d3c60051c1a8d8f030fa4ba7a29237fe7479)
GOOD
src/crc32c in HEAD currently refers to tree 5a1b5f7188bd2181e2a71d431fff282d50058e46
src/crc32c in HEAD was last updated in commit 08269e54a9a74e06c9fb72720a216d8c4d4532a2 (tree 5a1b5f7188bd2181e2a71d431fff282d50058e46)
GOOD
Args used : 209
Args documented : 220
Args undocumented: 0
set()
Args unknown : 11
{'-zmqpubhashblockhwm', '-zmqpubrawtx', '-zmqpubhashtxhwm', '-zmqpubhashblock', '-zmqpubsequence', '-zmqpubrawblock', '-zmqpubsequencehwm', '-zmqpubrawtxhwm', '-zmqpubhashtx', '-includeconf', '-zmqpubrawblockhwm'}
test/functional/feature_dbcrash.py:56:9: F841 local variable 'a' is assigned to but never used
^---- failure generated from lint-python.py
11:19:44 james@tacoma src/bitcoin (?± ackr/28083.1.MarcoFalke.ci_use_docker_buildkit_f fa2f18a) % echo $?
0Though I think this is an issue unrelated to this PR. (Returning 0 as an error code here prevents using the linter usefully within git hooks.)
Member
Author
|
Done in #28103 |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jul 19, 2023
fanquake
added a commit
to bitcoin-core/gui
that referenced
this pull request
Jul 20, 2023
…6_script.sh ffff4b5 lint: Add missing `set -ex` to ci/lint/06_script.sh (MarcoFalke) fadc523 doc: Add doc comment to ci/test_imagefile (MarcoFalke) Pull request description: Requested in bitcoin/bitcoin#28083 (review). Also, one doc commit. ACKs for top commit: fanquake: ACK ffff4b5 jamesob: ACK ffff4b5 ([`jamesob/ackr/28103.1.MarcoFalke.test_add_missing_set_ex`](https://github.com/jamesob/bitcoin/tree/ackr/28103.1.MarcoFalke.test_add_missing_set_ex)) Tree-SHA512: 99e67aeaae460319c2c428eab5297dbe1f1dc7f162f6592380bc5d2005308300c391cc187959cb2ace486dfe3411a8b0477f703ff11b5fe33944942c210a2d32
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jul 21, 2023
ffff4b5 lint: Add missing `set -ex` to ci/lint/06_script.sh (MarcoFalke) fadc523 doc: Add doc comment to ci/test_imagefile (MarcoFalke) Pull request description: Requested in bitcoin#28083 (review). Also, one doc commit. ACKs for top commit: fanquake: ACK ffff4b5 jamesob: ACK ffff4b5 ([`jamesob/ackr/28103.1.MarcoFalke.test_add_missing_set_ex`](https://github.com/jamesob/bitcoin/tree/ackr/28103.1.MarcoFalke.test_add_missing_set_ex)) Tree-SHA512: 99e67aeaae460319c2c428eab5297dbe1f1dc7f162f6592380bc5d2005308300c391cc187959cb2ace486dfe3411a8b0477f703ff11b5fe33944942c210a2d32
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 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.
Currently the lint docker/podman image has many issues:
cd ./ci/lint/before the build step..python-versionfile, but a hardcoded version.Fix all issues by using the recommended
DOCKER_BUILDKIT=1to generate the image.Also:
/tmp/pythonto/python_build.pip installcommands into one..python-version.