test: small fixups/improvements for get_previous_releases.py #26589
Conversation
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"
|
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. |
maflcko
changed the title
There was a problem hiding this comment.
tested ACK 9b5feb7
I modified the sha256 sum for v23 and then ran ./test/get_previous_releseases.py -b v23.0 to verify I got the expected error of "Checksum did not match."
This could be addressed in a follow-up, but I did notice while testing that if the binaries have been previously downloaded the script will use the cached version and skip checking the sha256sum. Seems like it would be better to still check the sha256sum even if using the cached binary as the binary could have been corrupted/modified since it was previously downloaded.
EDIT: regarding cached binaries, we delete the tarball after we check it and it doesn't seem worth it to keep the tarball around just so that we can re-verify the checksum and unzip it every time we want to use it, so nvm
…_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
6 participants
This is a small follow-up to #25650 (commit 614d468) with three fixes/improvements:
SHA256_SUMSstructure 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)--tagsargument help text: add missing space between "for" and "backwards"