diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 00d71d562f..0d6f93eac9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,15 +38,6 @@ jobs: with: name: d2chaos path: ./d2chaos/out - nofixups: - runs-on: ubuntu-latest - timeout-minutes: 5 - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - - run: git submodule update --init - - run: COLOR=1 ./ci/sub/bin/nofixups.sh - env: - GITHUB_TOKEN: ${{ github.token }} signed: runs-on: ubuntu-latest timeout-minutes: 5 diff --git a/ci/release/README.md b/ci/release/README.md index 59e720dfd8..7e151198a0 100644 --- a/ci/release/README.md +++ b/ci/release/README.md @@ -95,10 +95,14 @@ After the workflow succeeds, review and merge the release PR, then publish the c draft manually on GitHub. `release.sh` rejects `--publish`, `--skip-build`, and `--rebuild`, and never repurposes an existing version. To recover a failed run without changing code or artifacts, use **Re-run failed jobs** on that run. Code or build changes require a new version, -even if the old version is still a draft. Its Actions artifacts are immutable: a full -rerun fails rather than replacing them. If draft upload was interrupted, the failed job -resumes by accepting identical existing assets and attaching only missing ones. Artifacts -are retained for 30 days; recover failures within that window. +even if the old version is still a draft. Individual Actions artifacts are immutable, but +reruns can create newer artifacts with the same names; downloads by name select the newest. +Retry failed jobs to reuse completed build outputs. If draft upload was interrupted, the +failed job accepts identical existing assets and attaches only missing ones. A GitHub upload +failure can leave an empty `starter` asset; after rechecking the tag, draft, and asset identity, +the uploader deletes only that zero-byte placeholder and retries its upload. Changed or +completed assets are never deleted. Artifacts are retained for 30 days; recover failures +within that window. Run recovery without another uploader modifying the same draft. Release-related PRs exercise the same archive and native smoke jobs and, when the WiX gate below is enabled, the MSI build. PRs never attest or write release assets. CI is called diff --git a/ci/release/build.sh b/ci/release/build.sh index c45111698c..0df6ab1f1a 100755 --- a/ci/release/build.sh +++ b/ci/release/build.sh @@ -132,12 +132,12 @@ main() { runjob windows/amd64 'OS=windows ARCH=amd64 build' runjob windows/arm64 'OS=windows ARCH=arm64 build' else - runjob linux/amd64 'OS=linux ARCH=amd64 build' & - runjob linux/arm64 'OS=linux ARCH=arm64 build' & - runjob macos/amd64 'OS=macos ARCH=amd64 build' & - runjob macos/arm64 'OS=macos ARCH=arm64 build' & - runjob windows/amd64 'OS=windows ARCH=amd64 build' & - runjob windows/arm64 'OS=windows ARCH=arm64 build' & + runjob_bg linux/amd64 'OS=linux ARCH=amd64 build' + runjob_bg linux/arm64 'OS=linux ARCH=arm64 build' + runjob_bg macos/amd64 'OS=macos ARCH=amd64 build' + runjob_bg macos/arm64 'OS=macos ARCH=arm64 build' + runjob_bg windows/amd64 'OS=windows ARCH=amd64 build' + runjob_bg windows/arm64 'OS=windows ARCH=arm64 build' waitjobs fi write_checksums diff --git a/ci/release/upload-draft.py b/ci/release/upload-draft.py index f0e41fd4ff..67544d6160 100644 --- a/ci/release/upload-draft.py +++ b/ci/release/upload-draft.py @@ -94,6 +94,25 @@ def existing_asset(release, path, expected_digest, expected_size, allow_pending= return True +def starter_asset_id(asset): + if (asset.get("state") == "starter" and type(asset.get("size")) is int and asset["size"] == 0 + and asset.get("digest") is None and type(asset.get("id")) is int and asset["id"] > 0): + return asset["id"] + return None + + +def preflight_assets(release, assets): + starters = {} + for asset in assets: + matches = [item for item in release["assets"] if item["name"] == asset[0].name] + asset_id = starter_asset_id(matches[0]) if len(matches) == 1 else None + if asset_id is not None: + starters[asset[0].name] = asset_id + continue + existing_asset(release, *asset) + return starters + + def upload(repository, version, commit, assets): verify_tag(repository, version, commit) release = find_release(repository, version) @@ -111,9 +130,18 @@ def upload(repository, version, commit, assets): verify_draft(release, version) release_id = release["id"] prerelease = release["prerelease"] - # Preflight every existing asset before the first upload. Retries accept only identical bytes. - for asset in assets: - existing_asset(release, *asset) + # A failed GitHub upload can leave an empty starter. Never delete completed assets. + starters = preflight_assets(release, assets) + for name, asset_id in starters.items(): + release = api(f"repos/{repository}/releases/{release_id}") + verify_draft(release, version, release_id, prerelease) + if preflight_assets(release, assets).get(name) != asset_id: + raise ValueError(f"starter asset changed before cleanup: {name}") + asset = api(f"repos/{repository}/releases/assets/{asset_id}") + if asset.get("name") != name or starter_asset_id(asset) != asset_id: + raise ValueError(f"starter asset changed before cleanup: {name}") + verify_tag(repository, version, commit) + gh("api", "--method", "DELETE", f"repos/{repository}/releases/assets/{asset_id}") for path, expected_digest, expected_size in assets: verify_tag(repository, version, commit) release = api(f"repos/{repository}/releases/{release_id}") diff --git a/ci/release/upload_draft_test.py b/ci/release/upload_draft_test.py index aefc57092b..9349ede99b 100644 --- a/ci/release/upload_draft_test.py +++ b/ci/release/upload_draft_test.py @@ -25,17 +25,38 @@ def __init__(self, assets=(), published=False): self.commit = COMMIT self.writes = [] self.change_after_upload = None + self.change_before_release_read = None + self.change_before_asset_read = None + self.fail_upload_once = False + self.next_asset_id = 1000 for asset in assets: self.add_asset(*asset) def add_asset(self, path, digest, size): - self.release["assets"].append({"name": path.name, "state": "uploaded", "digest": digest, "size": size}) + asset = {"id": self.next_asset_id, "name": path.name, "state": "uploaded", "digest": digest, "size": size} + self.next_asset_id += 1 + self.release["assets"].append(asset) + return asset + + def add_starter(self, path): + asset = self.add_asset(path, None, 0) + asset["state"] = "starter" + return asset def api(self, path): if "/git/ref/tags/" in path: return {"object": {"type": "commit", "sha": self.commit}} if path.endswith("/releases/123"): + if self.change_before_release_read: + self.change_before_release_read(self) + self.change_before_release_read = None return copy.deepcopy(self.release) + if "/releases/assets/" in path: + asset = next(asset for asset in self.release["assets"] if asset["id"] == int(path.rsplit("/", 1)[1])) + if self.change_before_asset_read: + self.change_before_asset_read(asset) + self.change_before_asset_read = None + return copy.deepcopy(asset) raise AssertionError(path) def gh(self, *args): @@ -46,9 +67,19 @@ def gh(self, *args): self.release = FakeGitHub().release elif args[:2] == ("release", "upload"): path = Path(args[3]) + assert "--clobber" not in args + assert not any(asset["name"] == path.name for asset in self.release["assets"]) + if self.fail_upload_once: + self.fail_upload_once = False + self.add_starter(path) + self.writes.append(args) + raise RuntimeError("502 upload failure left an empty starter") self.add_asset(path, uploader.digest(path), path.stat().st_size) if self.change_after_upload: self.change_after_upload(self) + elif args[:3] == ("api", "--method", "DELETE"): + asset_id = int(args[3].rsplit("/", 1)[1]) + self.release["assets"] = [asset for asset in self.release["assets"] if asset["id"] != asset_id] else: raise AssertionError(args) self.writes.append(args) @@ -123,6 +154,97 @@ def test_conflict_preflight_prevents_partial_upload(self): self.run_upload(fake) self.assertEqual(fake.writes, []) + def test_starter_cleanup_recovers_repeated_upload_failure(self): + fake = FakeGitHub(self.assets[:2]) + completed = copy.deepcopy(fake.release["assets"]) + first_id = fake.add_starter(self.assets[2][0])["id"] + fake.fail_upload_once = True + with self.assertRaisesRegex(RuntimeError, "502"): + self.run_upload(fake) + second_id = fake.release["assets"][-1]["id"] + self.assertNotEqual(first_id, second_id) + self.run_upload(fake) + writes = copy.deepcopy(fake.writes) + self.run_upload(fake) + self.assertEqual(fake.writes, writes) + self.assertEqual(fake.release["assets"][:2], completed) + self.assertEqual(len(fake.release["assets"]), 9) + self.assertEqual([call[3] for call in fake.writes if call[:3] == ("api", "--method", "DELETE")], + [f"repos/test/repo/releases/assets/{first_id}", f"repos/test/repo/releases/assets/{second_id}"]) + + def test_unsafe_starter_metadata_is_never_deleted(self): + for field, value in (("state", "uploading"), ("state", "uploaded"), ("size", 1), ("size", False), + ("digest", "sha256:" + "b" * 64), ("id", "1000"), ("id", -1), ("id", True), ("id", None)): + with self.subTest(field=field, value=value): + fake = FakeGitHub() + fake.add_starter(self.assets[0][0])[field] = value + with self.assertRaisesRegex(ValueError, "incomplete metadata"): + self.run_upload(fake) + self.assertEqual(fake.writes, []) + + def test_starter_change_before_cleanup_prevents_deletion(self): + path, digest, size = self.assets[0] + changes = ({"id": 456}, {"state": "uploading"}, {"size": 1}, {"digest": digest}, + {"state": "uploaded", "size": size, "digest": digest}) + for phase in ("release", "asset"): + for change in changes: + with self.subTest(phase=phase, change=change): + fake = FakeGitHub() + starter = fake.add_starter(path) + if phase == "release": + fake.change_before_release_read = lambda state: starter.update(change) + else: + fake.change_before_asset_read = lambda asset: asset.update(change) + with self.assertRaises(ValueError): + self.run_upload(fake) + self.assertEqual(fake.writes, []) + + def test_starter_name_change_at_exact_asset_read_prevents_deletion(self): + fake = FakeGitHub() + fake.add_starter(self.assets[0][0]) + fake.change_before_asset_read = lambda asset: asset.update(name="unrelated.tar.gz") + with self.assertRaisesRegex(ValueError, "starter asset changed"): + self.run_upload(fake) + self.assertEqual(fake.writes, []) + + def test_conflict_elsewhere_prevents_starter_cleanup(self): + for phase in ("preflight", "cleanup"): + with self.subTest(phase=phase): + fake = FakeGitHub([self.assets[-1]]) + fake.add_starter(self.assets[0][0]) + def conflict(state): + state.release["assets"][0]["digest"] = "sha256:" + "b" * 64 + if phase == "preflight": + conflict(fake) + else: + fake.change_before_release_read = conflict + with self.assertRaisesRegex(ValueError, "different bytes"): + self.run_upload(fake) + self.assertEqual(fake.writes, []) + + def test_duplicate_starters_are_never_deleted(self): + fake = FakeGitHub() + fake.add_starter(self.assets[0][0]) + fake.add_starter(self.assets[0][0]) + with self.assertRaisesRegex(ValueError, "incomplete metadata"): + self.run_upload(fake) + self.assertEqual(fake.writes, []) + + def test_release_or_tag_change_before_cleanup_prevents_deletion(self): + for field, value in (("id", 456), ("prerelease", True), ("draft", False), ("tag_name", "v9.9.9"), ("commit", "b" * 40)): + with self.subTest(field=field): + fake = FakeGitHub() + fake.add_starter(self.assets[0][0]) + def change(state): + if field == "commit": + state.commit = value + else: + state.release[field] = value + fake.change_before_release_read = change + with self.assertRaises(ValueError): + self.run_upload(fake) + self.assertEqual(fake.writes, []) + def test_published_release_is_never_modified(self): fake = FakeGitHub(published=True) with self.assertRaisesRegex(ValueError, "remain a draft"): diff --git a/ci/sub b/ci/sub index da53bbdc63..a91fb17310 160000 --- a/ci/sub +++ b/ci/sub @@ -1 +1 @@ -Subproject commit da53bbdc63eb22d1083e4d8b4b104cf59c9d3c2b +Subproject commit a91fb1731094f9408a5543ea09040dfd69dbe2ad