Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <tag>` 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
Expand Down
36 changes: 35 additions & 1 deletion RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 <tag>` gives you the source the release was made from:

git fetch origin main
git tag -f PK-<version> <commit declaring it>
git push --force origin refs/tags/PK-<version>
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 <tag>` misleads — which is the kind of thing
that sends a bisect wrong.

If the release build fails
--------------------------

Expand Down
26 changes: 21 additions & 5 deletions scripts/publish_release.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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


Expand Down Expand Up @@ -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)
Comment thread
thpoll83 marked this conversation as resolved.
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"
Expand Down
Loading