diff --git a/CLAUDE.md b/CLAUDE.md index 072cc142..e3075255 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -352,6 +352,20 @@ before attaching anything, because the script is only one of three ways a release starts (a hand-pushed tag and a `workflow_dispatch` recovery do not go through it). +⚠️ **An EXISTING tag beats the pin, and the pin's own report hides it.** +`target_commitish` is documented as "Unused if the Git tag already exists" — a +release never moves a tag — so the build comes from wherever the tag points while +the script reports the commit it *wanted*. Reached by an ordinary sequence: +publish early, delete the release, merge the bump, try again; the first attempt's +tag is still on the pre-bump commit. **PK-0.9.20 sat exactly there** (tag on +`6cb987cb1a` declaring 0.9.19, pin resolving `113093cfa2`), so the script would +have printed the right commit and published the wrong one. It refuses now, and +⚠️ **that check must fire even when the version gate passes** — there the pin +found a correct commit and the mismatch gate sees nothing wrong. The fix is to +**move the tag** and publish normally, not to dispatch: a dispatch attaches +correct assets while leaving `git checkout ` on a tree that declares the +previous version. `RELEASE.md` → *If the tag is in the wrong place*. + ⚠️ **Do NOT make the gate fire on the tree merely differing from the tag.** The firmware and host tree is normally *ahead* of a prepared tag — every merge auto-bumps — and the pin already makes that safe, so refusing there blocks diff --git a/RELEASE.md b/RELEASE.md index d129a4f8..bfc75ad9 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -95,7 +95,7 @@ exists *before* it bumps, and nothing looks again afterwards. So a release delet after the job has run leaves `Latest` naming a version with no release — the early-bump failure below, arriving by another route. Revert `status.txt` in the same breath as the deletion, or re-create the release (see *If the release build -fails*). 0.9.20 was deleted and re-created on 7 October, and for the window in +fails*). 0.9.20 was deleted and re-created on 7 October, and for the ~4 hours in between every install below 0.9.20 was offered the 0.9.19 download. It was a manual step until 0.9.20, and it went wrong in both directions. Early, @@ -106,6 +106,40 @@ the version they already have and read it as the updater being broken (0.9.18, a happened and nothing about the release itself looks wrong (0.9.19, two weeks, reported from outside as "no notification"). +If the tag is in the wrong place +-------------------------------- + +⚠️ **Publishing never MOVES a tag.** `target_commitish` is documented as "Unused +if the Git tag already exists", so when the tag is already there the build comes +from wherever it points and the commit the script pins is only a claim. +`publish_release.py` refuses to create a release in that case, naming both +commits. + +That is reachable from an ordinary sequence: publish too early, delete the +release, merge the bump, try again. The tag from the first attempt is still on +the pre-bump commit. 0.9.20 did exactly this. + +Two ways out, and the first is better: + +1. **Move the tag to the commit that declares the version**, then publish + normally. The release then builds from a tree that reports the right version, + and `git checkout ` gives you the source the release was made from: + + git fetch origin main + git tag -f PK- + git push --force origin refs/tags/PK- + python scripts/publish_release.py + + ⚠️ **Take the commit from the refusal, which names both** — where the tag is + and where it should be. That is the only place to read them: the tag check + runs before the script prints its summary, so a `--dry-run` on a misplaced + tag never reaches the `target:` line. Safe as long as no release exists on + that tag yet; check before force-pushing. +2. **Dispatch the workflow** (below) to attach correct assets to the tag where it + is. Quicker, but the tag keeps pointing at a tree declaring the *previous* + version, so a later `git checkout ` misleads — which is the kind of thing + that sends a bisect wrong. + If the release build fails -------------------------- diff --git a/scripts/publish_release.py b/scripts/publish_release.py index b660be7c..fbe7bdeb 100644 --- a/scripts/publish_release.py +++ b/scripts/publish_release.py @@ -222,8 +222,15 @@ def _get_json(owner, repo, path, token): def tag_commit(owner, repo, tag, token): - """The commit an EXISTING tag points at; None if there is no such tag or the - lookup failed. + """Where an existing tag points: a sha, "" for a confirmed 404 (no such + tag), or None when it cannot be determined. + + ⚠️ The three are NOT interchangeable, and collapsing them is how this check + goes quiet: the caller SKIPS its guard when there is no tag, so a 5xx or a + timeout reading as "no tag" lets a publish through at whatever commit the + tag really points at. That is the same shape as the failed-lookup bug + already fixed once in release_exists() -- only a 404 is an answer. + (Found by Greptile on PolyKybdHost#318 and qmk_firmware#356.) ⚠️ Needed because `target_commitish` is documented as "Unused if the Git tag already exists" -- creating a release does not move a tag. So when the tag is @@ -235,15 +242,19 @@ def tag_commit(owner, repo, tag, token): dereferenced one step. """ st, ref = _get_json(owner, repo, f"/git/ref/tags/{tag}", token) + if st == 404: + return "" if st != 200 or not ref: return None obj = ref.get("object") or {} if obj.get("type") == "commit": - return obj.get("sha") + return obj.get("sha") or None if obj.get("type") == "tag": st, tg = _get_json(owner, repo, f"/git/tags/{obj.get('sha')}", token) if st == 200 and tg: - return (tg.get("object") or {}).get("sha") + return (tg.get("object") or {}).get("sha") or None + return None + # A ref shape this does not understand is not an answer either. return None @@ -486,7 +497,12 @@ def main(): # guard sees nothing wrong. (Found by Greptile on wincompose#27.) if creating: at = tag_commit(owner, repo, tag, token) - if at and target != default_branch and not at.startswith(target[:10]): + if at is None: + die(f"cannot tell whether the tag {tag} already exists or where it points, so\n" + f" whether the build would come from {target[:10]} is unknown -- and an\n" + f" existing tag is never moved by publishing. Refusing rather than guessing.\n" + f" Retry when the GitHub API is reachable." + unknown_note) + if at and target != default_branch and at != target: die(f"the tag {tag} already exists, at {at[:10]}, and creating a release does\n" f" not move it -- so the build would come from {at[:10]}, not from the\n" f" {target[:10]} this would report. Either dispatch the release workflow on a\n"