From 8c8bbf0e43f9100a7043f2e76001b6611afd8eb6 Mon Sep 17 00:00:00 2001 From: Zsolt Dollenstein Date: Wed, 25 Mar 2026 16:20:29 +0000 Subject: [PATCH] Make dry runs exercise the mirror publishing code --- .github/workflows/release.yml | 17 +++++-- Justfile | 10 ++++ src/github.rs | 91 +++++++++++++++++------------------ src/github_api_tester.py | 41 ++++++++++++++++ 4 files changed, 108 insertions(+), 51 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e4a01675e..b58430899 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -101,7 +101,6 @@ jobs: dist/*.tar.zst - name: Publish to Astral mirror - if: ${{ github.event.inputs.dry-run == 'false' }} env: AWS_ACCESS_KEY_ID: ${{ secrets.MIRROR_R2_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.MIRROR_R2_SECRET_ACCESS_KEY }} @@ -110,11 +109,19 @@ jobs: R2_BUCKET: ${{ secrets.MIRROR_R2_BUCKET_NAME }} PROJECT: python-build-standalone VERSION: ${{ github.event.inputs.tag }} + DRY_RUN: ${{ github.event.inputs.dry-run }} run: | - just release-upload-mirror \ - ${R2_BUCKET} \ - github/${PROJECT}/releases/download/${VERSION}/ \ - ${VERSION} + if [ "${DRY_RUN}" = 'true' ]; then + just release-upload-mirror-dry-run \ + ${R2_BUCKET} \ + github/${PROJECT}/releases/download/${VERSION}/ \ + ${VERSION} + else + just release-upload-mirror \ + ${R2_BUCKET} \ + github/${PROJECT}/releases/download/${VERSION}/ \ + ${VERSION} + fi publish-versions: needs: release diff --git a/Justfile b/Justfile index 5bac11f64..57804d014 100644 --- a/Justfile +++ b/Justfile @@ -103,6 +103,16 @@ release-upload-mirror bucket prefix tag: --bucket {{bucket}} \ --prefix {{prefix}} +# Dry-run the mirror upload without writing to the bucket. +# Requires `release-run` or `release-dry-run` to have been run so that dist/SHA256SUMS exists. +release-upload-mirror-dry-run bucket prefix tag: + uv run python -m pythonbuild.mirror \ + --dist dist \ + --tag {{tag}} \ + --bucket {{bucket}} \ + --prefix {{prefix}} \ + -n + # Perform the release job. Assumes that the GitHub Release has been created. release-run token commit tag: #!/bin/bash diff --git a/src/github.rs b/src/github.rs index 0d3079fe6..40403694a 100644 --- a/src/github.rs +++ b/src/github.rs @@ -448,24 +448,50 @@ pub async fn command_upload_release_distributions(args: &ArgMatches) -> Result<( return Err(anyhow!("missing {} release artifacts", missing.len())); } - let (client, token) = new_github_client(args)?; - let repo_handler = client.repos(organization, repo); - let releases = repo_handler.releases(); + let mut digests = BTreeMap::new(); - let release = if let Ok(release) = releases.get_by_tag(tag).await { - release - } else { - return if dry_run { - println!("release {tag} does not exist; exiting dry-run mode..."); - Ok(()) - } else { - Err(anyhow!( - "release {tag} does not exist; create it via GitHub web UI" - )) + for (source, dest) in &wanted_filenames { + if !filenames.contains(source) { + continue; + } + + let local_filename = dist_dir.join(source); + + // Compute digests in a separate pass so we can always materialize + // SHA256SUMS locally before any GitHub interaction, including in dry-run + // mode. This also avoids trying to reuse the streamed upload body for hashing. + let digest = { + let file = tokio::fs::File::open(local_filename).await?; + let mut stream = tokio_util::io::ReaderStream::with_capacity(file, 1048576); + let mut hasher = Sha256::new(); + while let Some(chunk) = stream.next().await { + hasher.update(&chunk?); + } + hex::encode(hasher.finalize()) }; - }; + digests.insert(dest.clone(), digest); + } - let mut digests = BTreeMap::new(); + let shasums = digests + .iter() + .map(|(filename, digest)| format!("{digest} {filename}\n")) + .collect::>() + .join(""); + + std::fs::write(dist_dir.join("SHA256SUMS"), shasums.as_bytes())?; + + if dry_run { + println!("wrote local SHA256SUMS; skipping GitHub upload and verification"); + return Ok(()); + } + + let (client, token) = new_github_client(args)?; + let repo_handler = client.repos(organization, repo); + let releases = repo_handler.releases(); + let release = releases + .get_by_tag(tag) + .await + .map_err(|_| anyhow!("release {tag} does not exist; create it via GitHub web UI"))?; let retry_policy = ExponentialBackoff::builder().build_with_max_retries(5); let raw_client = Client::new(); @@ -473,12 +499,12 @@ pub async fn command_upload_release_distributions(args: &ArgMatches) -> Result<( { let mut fs = vec![]; - for (source, dest) in wanted_filenames { - if !filenames.contains(&source) { + for (source, dest) in &wanted_filenames { + if !filenames.contains(source) { continue; } - let local_filename = dist_dir.join(&source); + let local_filename = dist_dir.join(source); fs.push(upload_release_artifact( &raw_client, &retry_policy, @@ -486,23 +512,9 @@ pub async fn command_upload_release_distributions(args: &ArgMatches) -> Result<( token.clone(), &release, dest.clone(), - UploadSource::Filename(local_filename.clone()), + UploadSource::Filename(local_filename), dry_run, )); - - // reqwest wants to take ownership of the body, so it's hard for us to do anything - // clever with reading the file once and calculating the sha256sum while we read. - // So we open and read the file again. - let digest = { - let file = tokio::fs::File::open(local_filename).await?; - let mut stream = tokio_util::io::ReaderStream::with_capacity(file, 1048576); - let mut hasher = Sha256::new(); - while let Some(chunk) = stream.next().await { - hasher.update(&chunk?); - } - hex::encode(hasher.finalize()) - }; - digests.insert(dest.clone(), digest.clone()); } let mut buffered = futures::stream::iter(fs).buffer_unordered(16); @@ -512,14 +524,6 @@ pub async fn command_upload_release_distributions(args: &ArgMatches) -> Result<( } } - let shasums = digests - .iter() - .map(|(filename, digest)| format!("{digest} {filename}\n")) - .collect::>() - .join(""); - - std::fs::write(dist_dir.join("SHA256SUMS"), shasums.as_bytes())?; - upload_release_artifact( &raw_client, &retry_policy, @@ -534,11 +538,6 @@ pub async fn command_upload_release_distributions(args: &ArgMatches) -> Result<( // Check that content wasn't munged as part of uploading. This once happened // and created a busted release. Never again. - if dry_run { - println!("skipping SHA256SUMs check"); - return Ok(()); - } - let release = releases .get_by_tag(tag) .await diff --git a/src/github_api_tester.py b/src/github_api_tester.py index 9ff4bc07a..314a70294 100755 --- a/src/github_api_tester.py +++ b/src/github_api_tester.py @@ -328,6 +328,47 @@ async def test_upload(server, upload_release_distributions, tag): assert assets[0].contents == f"{SHA256_20MEG} {filename}\n".encode() +async def test_dry_run_writes_shasums_without_contacting_github(tmp_path): + dist = tmp_path / "dist" + dist.mkdir() + + filename = dist / FILENAME + filename.touch() + os.truncate(filename, 20_000_000) + + tag = "missing-release" + with trio.fail_after(300): + await trio.run_process( + [ + "cargo", + "run", + "--", + "upload-release-distributions", + "--github-uri", + # Use a guaranteed-bad loopback port so this fails fast if the + # command unexpectedly tries to contact GitHub in dry-run mode. + "http://127.0.0.1:1", + "--token", + "no-token-needed", + "--dist", + dist, + "--datetime", + "19700101T1234", + "--ignore-missing", + "--tag", + tag, + "-n", + ] + ) + + release_filename = FILENAME.replace("3.0.0", f"3.0.0+{tag}").replace( + "-19700101T1234", "" + ) + assert (dist / "SHA256SUMS").read_bytes() == ( + f"{SHA256_20MEG} {release_filename}\n".encode() + ) + + # Work around https://github.com/pgjones/hypercorn/issues/238 not being in a release # Without it, test failures are unnecessarily noisy hypercorn.trio.lifespan.LifespanFailureError = trio.Cancelled