aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormerge-script <fanquake@gmail.com>2024-06-26 10:28:44 +0100
committermerge-script <fanquake@gmail.com>2024-06-26 10:28:44 +0100
commit2cd7c6bd939e4e37b00b5b822b62eb933f720dc6 (patch)
tree582e2ddfa93bee36d126ce8be45fe4a2ee828e0e
parentbe4df59696a41cec5a33383524313a17e1c5e4da (diff)
parent3ab25201909bece9066ac6191670bcee09791d54 (diff)
Merge bitcoin/bitcoin#30147: contrib: Fixup verify-binaries OS platform parsing
3ab25201909bece9066ac6191670bcee09791d54 contrib: Fixup verify-binaries OS platform parsing (Ben Westgate) Pull request description: Closes #30145. This PR solves two major issues in the `parse_version_string` function of verify-binaries: 1. `-aarch64` binaries cannot be specifically downloaded. The -platform string gets interpreted as a release candidate that doesn't exist due to containing sub-string "rc". 2. Specifying a platform with a "-" in the name causes the parser to ignore both "-platform" AND "-rcN" and download the potentially wrong (non-rc) version for every platform. This also prevented specifying just one platform binary the user wished to download. It also updates the accompanying `test.py` to cover problem two and adds two examples that were formerly broken to `README.md` to show what is now possible. Including the most useful case of downloading only 1 specific platform's binary. This improves the Bitcoin verify-binaries tools user experience by not: 1. Failing to download for inexplicable reasons, 2. Downloading more files than what the user told it to, or in the worst case 3. Downloading only the wrong files. * A test was added to cover the command `verify-binaries/verify.py pub 22.0-x86_64-linux-gnu.tar.gz` which checks that _bitcoin-22.0-x86_64-linux-gnu.tar.gz_ downloads successfully AND ONLY _bitcoin-22.0-x86_64-linux-gnu.tar.gz_ downloads. * The steps to reproduce each bug are in the referenced issue #30145. Explanation of the potential issue as well as reasoning for the way the bug was fixed are in my commit descriptions. * This delivers the promised feature of "only download the binaries for a certain platform", by allowing strings with '-' to be accepted, allowing for single file downloads for any specific platform which was not always possible before. * Removes 6 lines of code from the offending `parse_version_string` function, while fixing the bugs/errors, and extending the functionality to be practical for users with slow connections. * Makes the error message more helpful when no file matches the provided platform string, now prints "Did you mean: `closest-match`" to help correct typos. Thanks for reading my PR. I look forward to getting this helpful tool in its best shape yet. Log of this branch passing the new test.py: ``` python3 test.py ✓ 'Nonexistent version should fail' passed ✓ 'Malformed version should fail' passed ✓ '--min-good-sigs 20 should fail' passed - testing verification (22.0-x86_64-linux-gnu.tar.gz) ✓ '22.0-x86_64-linux-gnu.tar.gz should succeed' passed - testing verification (22.0) ✓ '22.0 should succeed' passed ``` Log of master failing the new test.py: ``` python3 test.py ✓ 'Nonexistent version should fail' passed ✓ 'Malformed version should fail' passed ✓ '--min-good-sigs 20 should fail' passed - testing verification (22.0-x86_64-linux-gnu.tar.gz) ✓ '22.0-x86_64-linux-gnu.tar.gz should succeed' passed Traceback (most recent call last): File "/home/ben/Documents/GitHub/bitcoin/contrib/verify-binaries/test.py", line 74, in <module> main() File "/home/ben/Documents/GitHub/bitcoin/contrib/verify-binaries/test.py", line 27, in main assert len(v) == 1 ^^^^^^^^^^^ AssertionError ``` ACKs for top commit: stickies-v: re-ACK 3ab25201909bece9066ac6191670bcee09791d54 willcl-ark: re-ACK 3ab25201909bece9066ac6191670bcee09791d54 Tree-SHA512: 6093228bb876cd0ac84d1cd2630074e17a3f09c4b23325b9419d859a5721a802f928844575233b135df52b882287dd18d6fadf4419d88ec0a2cdf82db315329e
-rw-r--r--contrib/verify-binaries/README.md10
-rwxr-xr-xcontrib/verify-binaries/test.py15
-rwxr-xr-xcontrib/verify-binaries/verify.py26
3 files changed, 32 insertions, 19 deletions
diff --git a/contrib/verify-binaries/README.md b/contrib/verify-binaries/README.md
index 04d683e69b..0f3e16a5bc 100644
--- a/contrib/verify-binaries/README.md
+++ b/contrib/verify-binaries/README.md
@@ -50,6 +50,7 @@ Get JSON output and don't prompt for user input (no auto key import):
```sh
./contrib/verify-binaries/verify.py --json pub 22.0-x86
+./contrib/verify-binaries/verify.py --json pub 23.0-rc5-linux-gnu
```
Rely only on local GPG state and manually specified keys, while requiring a
@@ -57,14 +58,15 @@ threshold of at least 10 trusted signatures:
```sh
./contrib/verify-binaries/verify.py \
--trusted-keys 74E2DEF5D77260B98BC19438099BAD163C70FBFA,9D3CC86A72F8494342EA5FD10A41BDC3F4FAFF1C \
- --min-good-sigs 10 pub 22.0-x86
+ --min-good-sigs 10 pub 22.0-linux
```
-If you only want to download the binaries for a certain platform, add the corresponding suffix, e.g.:
+If you only want to download the binaries for a certain architecture and/or platform, add the corresponding suffix, e.g.:
```sh
-./contrib/verify-binaries/verify.py pub 24.0.1-darwin
-./contrib/verify-binaries/verify.py pub 23.1-rc1-win64
+./contrib/verify-binaries/verify.py pub 25.2-x86_64-linux
+./contrib/verify-binaries/verify.py pub 24.1-rc1-darwin
+./contrib/verify-binaries/verify.py pub 27.0-win64-setup.exe
```
If you do not want to keep the downloaded binaries, specify the cleanup option.
diff --git a/contrib/verify-binaries/test.py b/contrib/verify-binaries/test.py
index 22d718ece3..875606ec22 100755
--- a/contrib/verify-binaries/test.py
+++ b/contrib/verify-binaries/test.py
@@ -12,6 +12,21 @@ def main():
expect_code(run_verify("", "pub", '0.32.awefa.12f9h'), 11, "Malformed version should fail")
expect_code(run_verify('--min-good-sigs 20', "pub", "22.0"), 9, "--min-good-sigs 20 should fail")
+ print("- testing verification (22.0-x86_64-linux-gnu.tar.gz)", flush=True)
+ _220_x86_64_linux_gnu = run_verify("--json", "pub", "22.0-x86_64-linux-gnu.tar.gz")
+ try:
+ result = json.loads(_220_x86_64_linux_gnu.stdout.decode())
+ except Exception:
+ print("failed on 22.0-x86_64-linux-gnu.tar.gz --json:")
+ print_process_failure(_220_x86_64_linux_gnu)
+ raise
+
+ expect_code(_220_x86_64_linux_gnu, 0, "22.0-x86_64-linux-gnu.tar.gz should succeed")
+ v = result['verified_binaries']
+ assert result['good_trusted_sigs']
+ assert len(v) == 1
+ assert v['bitcoin-22.0-x86_64-linux-gnu.tar.gz'] == '59ebd25dd82a51638b7a6bb914586201e67db67b919b2a1ff08925a7936d1b16'
+
print("- testing verification (22.0)", flush=True)
_220 = run_verify("--json", "pub", "22.0")
try:
diff --git a/contrib/verify-binaries/verify.py b/contrib/verify-binaries/verify.py
index 12e6e10d8a..6c07b36c9d 100755
--- a/contrib/verify-binaries/verify.py
+++ b/contrib/verify-binaries/verify.py
@@ -97,23 +97,17 @@ def bool_from_env(key, default=False) -> bool:
VERSION_FORMAT = "<major>.<minor>[.<patch>][-rc[0-9]][-platform]"
-VERSION_EXAMPLE = "22.0-x86_64 or 23.1-rc1-darwin"
+VERSION_EXAMPLE = "22.0 or 23.1-rc1-darwin.dmg or 27.0-x86_64-linux-gnu"
def parse_version_string(version_str):
- parts = version_str.split('-')
- version_base = parts[0]
- version_rc = ""
- version_os = ""
- if len(parts) == 2: # "<version>-rcN" or "version-platform"
- if "rc" in parts[1]:
- version_rc = parts[1]
- else:
- version_os = parts[1]
- elif len(parts) == 3: # "<version>-rcN-platform"
- version_rc = parts[1]
- version_os = parts[2]
+ # "<version>[-rcN][-platform]"
+ version_base, _, platform = version_str.partition('-')
+ rc = ""
+ if platform.startswith("rc"): # "<version>-rcN[-platform]"
+ rc, _, platform = platform.partition('-')
+ # else "<version>" or "<version>-platform"
- return version_base, version_rc, version_os
+ return version_base, rc, platform
def download_with_wget(remote_file, local_file):
@@ -514,7 +508,9 @@ def verify_published_handler(args: argparse.Namespace) -> ReturnCode:
# Extract hashes and filenames
hashes_to_verify = parse_sums_file(SUMS_FILENAME, [os_filter])
if not hashes_to_verify:
- log.error("no files matched the platform specified")
+ available_versions = ["-".join(line[1].split("-")[2:]) for line in parse_sums_file(SUMS_FILENAME, [])]
+ closest_match = difflib.get_close_matches(os_filter, available_versions, cutoff=0, n=1)[0]
+ log.error(f"No files matched the platform specified. Did you mean: {closest_match}")
return ReturnCode.NO_BINARIES_MATCH
# remove binaries that are known not to be hosted by bitcoincore.org