script: default to necessary tags in test/get_previous_releases.py
#25650
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
in order to run the backwards compatibility tests, specific releases are needed. previously, the list of tags was in test/README.md, but it makes more sense to have them as the default list in script
take the hardcoded list out of the readme. this way, we only need to update the script as new tags are added
|
Concept ACK. It should be possible to drop Though you still need to define something so it's only run for the correct build: bitcoin/ci/test/05_before_script.sh Lines 50 to 52 in fcde5d1 |
thanks for pointing this out! i was not aware the tags were also hardcoded in the ci scripts (even more motivation to have a single source of truth). i added the ci changes in their own commit, but do let me know if there is a better way to go about making code changes + ci changes (doing them in the same PR feels a little weird to me) |
63a6c8b to
21a9e94
Compare
|
force pushed to address feedback from @Sjors ; |
|
re-tACK 21a9e94 |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Aug 5, 2022
theStack
added a commit
to theStack/bitcoin
that referenced
this pull request
Nov 28, 2022
This is a small follow-up to bitcoin#25650 (commit 614d468) with three fixes/improvements: - fix "Checksum did not match" detection, which was not adapted to the new SHA256_SUMS structure and hence never executed (the list of tarball names isn't directly in the dictionary's values anymore, but has to be extracted from the 'tarball' field of each value) - make both help text and default tag download order deterministic by sorting default tags - "--tags" argument help text: add missing space between "for" and "backwards"
maflcko
pushed a commit
that referenced
this pull request
Nov 28, 2022
…es.py 9b5feb7 script: small fixups/improvements for get_previous_releases.py (Sebastian Falbesoner) Pull request description: This is a small follow-up to #25650 (commit 614d468) with three fixes/improvements: - fix "Checksum did not match" detection, which was not adapted to the new `SHA256_SUMS` structure and hence never executed (the list of tarball names isn't directly in the dictionary's values anymore, but has to be extracted from the `'tarball'` field of each value) - make both help text and default tag download order deterministic by sorting default tags - `--tags` argument help text: add missing space between "for" and "backwards" ACKs for top commit: Sjors: tACK 9b5feb7. Tested that if I change a checksum, or remove a release, it catches that. josibake: tested ACK 9b5feb7 Tree-SHA512: 791fa693477eebbda7fd41f3f5ec78fe7eab57df06979aa907ab258a6945534bdc3b931ddfce0fb440c9666b98c88ce5e1b6dc353ed39e129e87d3634855165c
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Dec 1, 2022
…_releases.py 9b5feb7 script: small fixups/improvements for get_previous_releases.py (Sebastian Falbesoner) Pull request description: This is a small follow-up to bitcoin#25650 (commit 614d468) with three fixes/improvements: - fix "Checksum did not match" detection, which was not adapted to the new `SHA256_SUMS` structure and hence never executed (the list of tarball names isn't directly in the dictionary's values anymore, but has to be extracted from the `'tarball'` field of each value) - make both help text and default tag download order deterministic by sorting default tags - `--tags` argument help text: add missing space between "for" and "backwards" ACKs for top commit: Sjors: tACK 9b5feb7. Tested that if I change a checksum, or remove a release, it catches that. josibake: tested ACK bitcoin@9b5feb7 Tree-SHA512: 791fa693477eebbda7fd41f3f5ec78fe7eab57df06979aa907ab258a6945534bdc3b931ddfce0fb440c9666b98c88ce5e1b6dc353ed39e129e87d3634855165c
maflcko
pushed a commit
that referenced
this pull request
Feb 16, 2023
741908a test: previous releases: add v24.0.1 (Sebastian Falbesoner) Pull request description: The same procedure as every release (see dba1231 [v23.0] and d8b705f [v22.0]), only a little simpler now: thanks to #25650, the previous release fetch script defaults to downloading/building the necessary tags, i.e. we don't need to extend the tag list in the CI scripts and test/README.md anymore. ACKs for top commit: Sjors: tACK 741908a Tree-SHA512: a5426e989bd0bba42aa13e7d4cf60f792bf36bd9a6cdb6ef5799f7574d9a8a20979244627bbd0c6219630367e7fd73bac9e677814bc50233f64592ad035e713e
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Almost every time I need to use this script, I forget the tag list is needed and that a specific set of tags is needed for the backwards compatibility tests to work. I end up wasting time reading through the script and googling to find the tag list before remembering it is in
test/README.mdI assume (hope) I'm not the only one this happens to, so I figured it would make more sense to have the script default to downloading/building the necessary tags. This has the added benefit of making the script the source of truth: the script already needs to be updated with the SHA256_SUM of the binary for every new tag that is added, so it makes sense to use
SHA256_SUMSlist as the necessary tag list. This means there is less risk of the README and the script drifting (i.e updating the readme with a new tag and forgetting to update the script, or updating the script and forgetting to update the README). Now all that needs to happen is to update theSHA256_SUMSlist in the script and everything Just Works (TM)