Make dry runs exercise the mirror publishing code (#1055)

Author: zsol

.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

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

src/github.rs

@@ -448,61 +448,73 @@ 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::<Vec<_>>()
+        .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();
 
     {
         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,
                 &GitHubUploadRetryStrategy,
                 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::<Vec<_>>()
-        .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

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