diff --git a/.github/scripts/publish_hastegeo_wheel.py b/.github/scripts/publish_hastegeo_wheel.py index 39a9fa38..a6c32d8e 100644 --- a/.github/scripts/publish_hastegeo_wheel.py +++ b/.github/scripts/publish_hastegeo_wheel.py @@ -84,7 +84,19 @@ def validate_wheel( f"Wheel filename {wheel_path.name!r} does not match " f"{expected_name!r}" ) - if not wheel_path.is_file() or not zipfile.is_zipfile(wheel_path): + if not wheel_path.is_file(): + raise ValueError( + f"Expected wheel not found: {wheel_path}. The downloaded " + "artifact does not contain a file matching the re-resolved " + f"version {expected_version!r}. This is unexpected: either " + "the build artifact was not uploaded correctly, or the " + "version re-resolved here no longer matches what the build " + "produced (e.g. a wheel was published for this target " + "version by another run after this one was built); re-run " + "the build workflow to obtain a wheel matching the " + "currently available version." + ) + if not zipfile.is_zipfile(wheel_path): raise ValueError(f"Not a valid wheel ZIP file: {wheel_path}") with zipfile.ZipFile(wheel_path) as archive: diff --git a/.github/workflows/hastegeo-publish.yml b/.github/workflows/hastegeo-publish.yml index 9341e0a5..e789227d 100644 --- a/.github/workflows/hastegeo-publish.yml +++ b/.github/workflows/hastegeo-publish.yml @@ -11,6 +11,16 @@ permissions: actions: read contents: read +# Serialize entire workflow runs (not just the publish-rc job) so that +# version resolution in "prepare" always reflects the outcome of every +# earlier run. Without this, two concurrent PR builds can both resolve +# the same "next" RC number (e.g. rc1) before either one publishes, +# and the second run's build artifact permanently mismatches the RC +# number re-resolved by its "prepare" job on any retry. +concurrency: + group: hastegeo-publish + cancel-in-progress: false + jobs: prepare: if: >- diff --git a/hastelib/tests/build/test_release_scripts.py b/hastelib/tests/build/test_release_scripts.py index fd5eea80..2a7a4d85 100644 --- a/hastelib/tests/build/test_release_scripts.py +++ b/hastelib/tests/build/test_release_scripts.py @@ -68,6 +68,17 @@ def test_validate_wheel_rejects_stable_version_on_rc_channel(self): with self.assertRaisesRegex(ValueError, "requires an rcN"): publish_hastegeo_wheel.validate_wheel(wheel, "1.0.26", "rc") + def test_validate_wheel_reports_missing_file_clearly(self): + with tempfile.TemporaryDirectory() as temp_dir: + missing = Path(temp_dir) / "hastegeo-1.0.26rc2-py3-none-any.whl" + + with self.assertRaisesRegex( + ValueError, "Expected wheel not found" + ): + publish_hastegeo_wheel.validate_wheel( + missing, "1.0.26rc2", "rc" + ) + @patch.object( publish_hastegeo_wheel, "list_release_assets", diff --git a/hastelib/tests/build/test_release_workflows.py b/hastelib/tests/build/test_release_workflows.py index 2b0f6aa1..2973014f 100644 --- a/hastelib/tests/build/test_release_workflows.py +++ b/hastelib/tests/build/test_release_workflows.py @@ -117,6 +117,28 @@ def test_rc_and_stable_are_both_automatic_but_kill_switched(self): self.assertIn("contents: write", workflow) self.assertNotIn("--clobber", publisher) + def test_publish_workflow_runs_are_serialized(self): + """Concurrent runs must not race for the same "next" RC number. + + "prepare" re-resolves the version independently of the triggering + build. If two builds for different commits both find the release + with no RC yet for their target version, they both compute the + same next number, and whichever publishes second permanently fails + because its already-uploaded build artifact can never match a + re-resolved, higher RC number. A workflow-level concurrency group + (not just the publish-rc job's) prevents this by ensuring only one + run's "prepare" step is ever resolving a version against the + release state at a time. + """ + workflow = ( + REPO_ROOT / ".github/workflows/hastegeo-publish.yml" + ).read_text(encoding="utf-8") + pre_jobs = workflow.split("\njobs:", 1)[0] + + self.assertIn("concurrency:", pre_jobs) + self.assertIn("group: hastegeo-publish", pre_jobs) + self.assertIn("cancel-in-progress: false", pre_jobs) + def test_pr_workflow_does_not_build_images_twice(self): workflow = ( REPO_ROOT / ".github/workflows/hastegeo-build.yml"