diff --git a/CLAUDE.md b/CLAUDE.md index 1a3f927e..072cc142 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -305,9 +305,10 @@ a release labelled with the PREVIOUS version, in four places at once.** ``/`` is the only source: `iscc` reads it off the built exe for the installer filename, `release.yml` reads the same resource to name the zip, `Settings.Version` is `Assembly.GetExecutingAssembly()` so the About -tab shows it, and `Updater.cs` compares *that* against `status.txt`. The tag comes -from the `release-notes` branch instead (`scripts/publish_release.py`), and is -created at `main`'s tip. +tab shows it, and `Updater.cs` compares *that* against `status.txt`. Which version +to publish comes from the `release-notes` branch instead +(`scripts/publish_release.py`), and the tag used to be created at `main`'s tip — +which is the whole mechanism below. **Worked example, PK-0.9.19 (2026-10-02, issue #21).** Published 09:47 UTC; the bump `cf7e91e` was committed 09:45 but sat on the PR #23 branch and merged at @@ -325,27 +326,51 @@ bump `cf7e91e` was committed 09:45 but sat on the PR #23 branch and merged at About-tab link regrouping) are not in the assets either, because they merged in the same window. -⚠️ **`publish_release.py`'s version note USED to be informational, and it fired -on exactly this and was read as benign.** It is asymmetric now, which is the -point: *behind* means the version does not exist yet, *ahead* means the tag claims -a version whose code is not what the tree has. Both now die when publishing would -CREATE the release — the build labels its assets from the tree either way, so -`release.yml` refuses them and you are left with a published release carrying no -downloads at all. Re-applying notes to an existing release re-runs no build, so -there a difference stays a note; the script tells the two apart by whether the tag -exists yet. `--allow-version-mismatch` overrides, with that no-assets outcome. -`release.yml` also asserts the built exe's version equals the tag before -attaching anything. Belt and braces, 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). - -⚠️ **Not fixed structurally, deliberately.** The right fix is to derive the exe's -version from the tag via GitVersion, which is already in the build for -`language.csproj` — then a mislabelled release is unrepresentable. It was not done -because there is no .NET toolchain in the container, so CI is the only compiler -(a wrong guess costs a full round), and because GitVersion's output on an -untagged commit would change what every development build's About tab reads. The -two gates are the cheap half; the derivation is still the real answer. +⚠️ **`publish_release.py` now tags the commit whose csproj DECLARES the version, +not `main`'s tip** — and because `release.yml` checks out the tag, that is what +makes the assets carry the right number. The sibling repos had this (it is +`commit_for_version()`, with four hard-won traps in its docstring: oldest match +not newest, no early exit, `--first-parent`, and a `:(top)` pathspec); this copy +was taken from them in `e894a3a` before the pin existed, which is why #21 +happened here and not there. A later merge drifting the csproj forward is then +harmless, and the script says so rather than warning. + +⚠️ **The pin cannot help when NO commit declares the version** — the bump has +not merged, so there is nothing to pin to — and that is exactly #21. Creating +the release is refused there. Only when creating: re-applying notes to an +existing release runs no build, so the tree's version is then irrelevant, and +`release_exists()` tells the two apart. ⚠️ **A tag existing is a different +question** — a deleted release, a hand-pushed tag, or a build that died before +`gh release create` each leave a tag with no release. Only a 404 counts as "no +release"; anything else is treated as creating, the stricter reading, because an +auth or network failure reading as success is how the check goes quiet. +⚠️ **A shallow clone reaches the same refusal for a different reason** — the +search could not see the commit — so unshallowing comes before +`--allow-version-mismatch`, which publishes the release the gate exists to +prevent. `release.yml` also asserts the built exe's version equals the tag +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). + +⚠️ **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 +almost every legitimate publish. The condition is "no commit declares it", not +"the numbers differ". + +**The GitVersion derivation is still the deeper answer, and still not done.** +Deriving the exe's version from the tag would make a mislabelled release +unrepresentable rather than merely refused. It is not done because there is no +.NET toolchain in the container, so CI is the only compiler (a wrong guess costs +a full round), and because GitVersion's output on an untagged commit would change +what every development build's About tab reads. + +⚠️ **`scripts/publish_release.py` is byte-identical across `qmk_firmware`, +`PolyKybdHost` and `wincompose`, and nothing checks it.** It had already +diverged in both directions at once: the siblings held the pin and the +`make_latest` correctness, this copy held the create-time gate, and each was +the newer one for a different thing. `md5sum */scripts/publish_release.py` is +the check. **`status.txt` is written by CI since 0.9.20** — the `status` job in `release.yml`, after the assets are attached, from the version resource of the binary it just diff --git a/RELEASE.md b/RELEASE.md index 68880809..d129a4f8 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -18,19 +18,21 @@ tag gets published — see ③. `GitVersion.yml` sets `tag-prefix: PK-` so GitVe recognises our tags instead of computing 0.1.0. ⚠️ **The bump has to be MERGED to `main` before you publish.** The release builds -from the tag, and the tag is created at `main`'s tip, so a bump still sitting on a -branch means the release ships assets carrying the *previous* version. 0.9.19 did -this (issue #21): published 50 minutes before its bump merged, it carries +from the tag, and the tag can only be pinned to a commit that already declares the +version, so a bump still sitting on a branch leaves nothing to pin to — publishing +is refused (it used to ship assets carrying the *previous* version instead). +0.9.19 did this (issue #21): published 50 minutes before its bump merged, it carries `WinCompose-Setup-0.9.18.exe`, reads 0.9.18 in its About tab, is missing two changes its own notes describe, and could not be announced at all — an install reporting 0.9.18 would have been offered an endless update to itself. -Two things now refuse rather than let that through, so this is a check you no -longer have to remember. `scripts/publish_release.py` dies when `main`'s version -is not the one the tag claims — in either direction, and only when publishing -would CREATE the release, since re-applying notes to one that already exists -re-runs no build. `release.yml` fails before attaching anything if the built -exe's version is not the tag's. +Three things now stand between you and that, so it is a check you no longer have +to remember. `scripts/publish_release.py` pins the tag to the commit that declares +the version, so a csproj that has drifted forward since the notes were prepared is +harmless. It dies when **no** commit declares it, and only when publishing would +CREATE the release, since re-applying notes to one that already exists re-runs no +build. And `release.yml` fails before attaching anything if the built exe's +version is not the tag's. ⚠️ `--allow-version-mismatch` overrides the first of those and leaves you a published release with **no assets**, because the second still refuses them. It @@ -53,12 +55,23 @@ that file; without it the release falls back to GitHub's auto-generated notes. python scripts/publish_release.py --dry-run # show what it would do python scripts/publish_release.py --tag PK-0.9.16 -The tag comes from the `release-notes` branch, **not** from the csproj version: -with no `--tag`, it publishes the newest prepared `PK-.md` found there. The -version on the default branch is read only to print a note when the two disagree, -so a csproj bump that landed after the notes were prepared does not change what -gets published. Pass `--tag` whenever more than one is prepared, and read the -"newest prepared tag" line it prints before letting it publish. +Which version to publish comes from the `release-notes` branch, **not** from the +csproj: with no `--tag`, it publishes the newest prepared `PK-.md` found +there. Pass `--tag` whenever more than one is prepared, and read the "newest +prepared tag" line it prints before letting it publish. + +**Where the tag lands is decided by the csproj, though.** The script tags the +commit whose `wincompose.csproj` declares that version, not `main`'s tip — and +since the release workflow checks out the tag, that is what makes the assets +carry the right number. A later merge drifting the csproj forward is then +harmless, and it says so. + +If **no** commit declares the version, the bump has not merged and there is +nothing to pin to, so creating the release is refused. Merge the bump and +re-run. `--allow-version-mismatch` overrides, and publishes notes with no +assets — the release workflow refuses to attach assets whose version does not +match the tag. In a shallow clone the search can miss the commit, so +`git fetch origin --unshallow` comes before reaching for that flag. Auth comes from `GH_TOKEN` / `GITHUB_TOKEN`, else `gh auth token`. @@ -77,6 +90,14 @@ download. It never moves `Latest` backwards, so re-dispatching an old tag to re-attach its assets cannot un-announce a newer release. Check the job went green; if it could not push, its error says so and names the one-line edit. +⚠️ **Deleting a release does not un-set it.** The job asks whether the release +exists *before* it bumps, and nothing looks again afterwards. So a release deleted +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 +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, every install is pointed at a release that is not there — and the `Installer:` / `Portable:` URLs resolve `releases/latest`, so nobody gets a 404, they are handed @@ -94,6 +115,18 @@ included. Fix the default branch, then start the workflow by hand that already exists. Leaving `tag` empty builds the assets as artifacts and releases nothing — that is the safe smoke test. +Two things about that dispatch are easy to get wrong: + +- **It re-applies the notes from `release-notes`.** Whenever a `PK-.md` + is on that branch, the dispatch runs `gh release edit --title --notes-file`, so + a title or body edited by hand on GitHub is thrown away. Fix the file on the + branch *first*, then dispatch. (With no notes file, the existing notes are kept + and only the assets are replaced.) +- **It creates the release when there is none** (`gh release create`), so the same + dispatch recovers a release that was deleted, not only one whose build failed. + It builds from the ref you dispatch on, normally `main`, never from the tag's + own commit — which is what the version assertion compares the tag against. + Building locally ---------------- diff --git a/scripts/publish_release.py b/scripts/publish_release.py index 2bbc5648..b660be7c 100644 --- a/scripts/publish_release.py +++ b/scripts/publish_release.py @@ -13,7 +13,16 @@ whatever branch you have checked out. 2. Reads the prepared release notes for that tag from the unprotected `release-notes` branch (`.md`, first line `# `, rest = body). - 3. Creates + publishes the GitHub Release (or updates it if it already exists). + 3. Creates + publishes the GitHub Release (or updates it if it already exists), + tagging the commit that DECLARES the prepared version rather than the + branch head — so a merge landing between preparing the notes and + publishing them cannot ship a binary whose version differs from its label. + When NO commit declares it — the bump has not merged yet — there is + nothing to pin to, and creating the release is refused rather than + shipping a build that is not the version on the label + (`--allow-version-mismatch` overrides; read its --help, since what that + costs differs per repo). Creating is also refused when the tag ALREADY + exists somewhere other than the pin, because a release never moves a tag. Firmware and wincompose: publishing fires the `release: published` workflow, which builds and attaches the assets (.bin/.uf2 / the installer + portable zip + SHA256SUMS) — you do NOT attach anything by hand. @@ -21,10 +30,10 @@ Auth: uses `GH_TOKEN` / `GITHUB_TOKEN` if set, else `gh auth token`. No token and no `gh` -> it tells you how to fix it. `gh` is optional; a token alone is enough. -Tags: - firmware PolyKybd-fw-v<version> (target branch: PolyKybd) - host v<version> (target branch: main) - wincompose PK-<version> (target branch: main) +Tags (created at the commit declaring <version>, found on the branch named): + firmware PolyKybd-fw-v<version> (branch: PolyKybd) + host v<version> (branch: main) + wincompose PK-<version> (branch: main) """ import argparse import json @@ -36,12 +45,32 @@ import urllib.request +# ⚠️ Every urlopen() here passes this. Without it urllib uses the socket +# default, which is None -- block forever -- so a connection GitHub accepts and +# then stops answering hangs the script instead of failing. That defeats +# release_exists()'s whole design, which is to return None on any trouble and +# let the caller take the stricter path, and it hangs --dry-run, the one command +# that must stay quick and safe. (Found by Greptile on PolyKybdHost#318, for +# release_exists; api() had it too and is fixed in the same pass.) +API_TIMEOUT = 30 + + def run(cmd): # Force UTF-8: git output (release notes) is UTF-8, but on Windows the # default is the locale codec (cp1252), which raises UnicodeDecodeError on # emoji/em-dashes and silently drops the output. - return subprocess.run(cmd, capture_output=True, text=True, - encoding="utf-8", errors="replace") + try: + return subprocess.run(cmd, capture_output=True, text=True, + encoding="utf-8", errors="replace") + except OSError as e: + # The executable isn't installed / isn't runnable. Report it as an + # ordinary non-zero result so callers can fall back, instead of letting + # it escape as a traceback. get_token() probes `gh`, which plenty of + # machines don't have — on Windows that surfaced as a raw + # "[WinError 2] The system cannot find the file specified" traceback + # instead of the "no GitHub token…" message that was already written + # for exactly this case. + return subprocess.CompletedProcess(cmd, 127, stdout="", stderr=str(e)) def die(msg): @@ -95,7 +124,8 @@ def parse_version(kind, text): def version_tuple(v): - """'0.9.20' -> (0, 9, 20), so 0.10.0 sorts after 0.9.20.""" + """'0.9.20' -> (0, 9, 20), so 0.10.0 sorts after 0.9.20 where a string + compare would not.""" return tuple(int(x) for x in v.split(".")) @@ -111,15 +141,7 @@ def get_token(): for var in ("GH_TOKEN", "GITHUB_TOKEN"): if os.environ.get(var): return os.environ[var] - try: - r = run(["gh", "auth", "token"]) - except OSError: - # gh is not installed. "no token" is the honest answer, and the caller - # that needs one already dies with a message naming the three ways to - # supply it. Raising here instead took --dry-run down with a traceback - # -- the one command that must work with no credentials at all, since - # it exists to tell you what WOULD happen. (Found by Greptile on #25.) - return None + r = run(["gh", "auth", "token"]) if r.returncode == 0 and r.stdout.strip(): return r.stdout.strip() return None @@ -136,7 +158,7 @@ def api(token, method, path, payload=None): "User-Agent": "polykybd-publish-release", }) try: - with urllib.request.urlopen(req) as resp: + with urllib.request.urlopen(req, timeout=API_TIMEOUT) as resp: return resp.status, json.load(resp) except urllib.error.HTTPError as e: try: @@ -150,10 +172,12 @@ def release_exists(owner, repo, tag, token): A GET on a public repo's release needs no token, so this answers under --dry-run too. None means "do not know" (network trouble, a rate limit, a - private repo with no token) and the caller must then assume the stricter - case -- a tag existing is NOT the same question, since a deleted release, - a hand-pushed tag, or a release build that died before `gh release create` - all leave a tag with no release behind it. + private repo with no token), and the caller must then assume the stricter + case. + + ⚠️ Whether a TAG exists is a different question and not a usable + substitute: a deleted release, a hand-pushed tag, or a release build that + died before `gh release create` all leave a tag with no release behind it. """ headers = { "Accept": "application/vnd.github+json", @@ -166,14 +190,160 @@ def release_exists(owner, repo, tag, token): f"https://api.github.com/repos/{owner}/{repo}/releases/tags/{tag}", method="GET", headers=headers) try: - with urllib.request.urlopen(req) as resp: + with urllib.request.urlopen(req, timeout=API_TIMEOUT) as resp: return resp.status == 200 except urllib.error.HTTPError as e: + # ⚠️ Only a 404 is "no release". Folding 401/403/5xx in with it is what + # lets an auth or network failure read as success. (Found by Revix and + # Greptile on wincompose#25.) return False if e.code == 404 else None except Exception: return None +def _get_json(owner, repo, path, token): + """GET a public endpoint; (status, payload) or (None, None) if unreachable.""" + headers = { + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "polykybd-publish-release", + } + if token: + headers["Authorization"] = f"Bearer {token}" + req = urllib.request.Request( + f"https://api.github.com/repos/{owner}/{repo}{path}", method="GET", headers=headers) + try: + with urllib.request.urlopen(req, timeout=API_TIMEOUT) as resp: + return resp.status, json.load(resp) + except urllib.error.HTTPError as e: + return e.code, None + except Exception: + return None, None + + +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. + + ⚠️ 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 + already there, the pin computed by commit_for_version() is not what gets + built, and reporting it would be a lie. Live example: wincompose PK-0.9.20's + tag sits on the pre-bump commit while the pin resolves the bump commit. + + ⚠️ An ANNOTATED tag's ref points at the tag OBJECT, not the commit, so it is + dereferenced one step. + """ + st, ref = _get_json(owner, repo, f"/git/ref/tags/{tag}", token) + if st != 200 or not ref: + return None + obj = ref.get("object") or {} + if obj.get("type") == "commit": + return obj.get("sha") + 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 None + + +def published_latest_version(owner, repo, tag_prefix, token): + """Version of the release GitHub currently calls "latest", or None. + + ⚠️ This is the question that decides the Latest badge, and it is NOT the same + as "the newest prepared notes file". Notes can be staged for a version that + has not shipped, and comparing against them withholds the badge from the + release being published -- leaving `releases/latest`, which the download + links resolve, on an OLDER release while the update file names the new one. + Every install is then offered the new version and handed the old one, which + is the 0.9.18 failure over again. (Found by Greptile on wincompose#27.) + + 404 means there is no published release yet, so whatever is being published + is the newest; that is reported as the empty string rather than None, which + means "could not tell". + """ + st, rel = _get_json(owner, repo, "/releases/latest", token) + if st == 404: + return "" + if st != 200 or not rel: + return None + name = rel.get("tag_name") or "" + if not name.startswith(tag_prefix): + return None + rest = name[len(tag_prefix):] + return rest if re.fullmatch(r"\d+\.\d+\.\d+", rest) else None + + +def commit_for_version(kind, vpath, default_branch, version): + """OLDEST commit on the default branch whose <vpath> declares `version`. + + The release must be tagged at a commit that actually reports the version the + notes describe, not at whatever the branch head happens to be. Tagging the + head means any merge between preparing the notes and publishing them ships a + binary whose version differs from its label -- and since `bump-version.yml` + has no paths filter, *every* merge bumps, docs-only ones included. Left + unfixed that is not merely cosmetic: on 2026-08-29 nine merges landed inside + that window, one of them the FW-9 engine-pack signing fix, which would have + shipped under notes that never mentioned it. + + Resolved by reading the version file at each commit rather than by matching a + `chore: bump ... version to X` message: it checks the property we actually + care about, and it works for wincompose, which has no bump commits at all. + + ⚠️ OLDEST, not newest, and the difference is not academic. A version stays + declared from its bump commit until the next bump, so several commits report + it -- and the later ones are the NEXT release's work, sitting in the window + before its own bump lands. Taking the newest match resolved v0.15.14 to a + commit from PR #231, which actually shipped in 0.15.15; the oldest match is + the bump commit, which is what every historical release was in fact tagged + at. + + ⚠️ The whole history is walked, with NO early exit. An earlier version broke + out on the first non-match after a match, on the theory that a version + occupies one contiguous run. That is true in time but FALSE in a path-limited + `git log`, where a PR's own commits and its merge commit interleave with the + mainline: the break stopped at the end of the first run and returned a commit + from a later release. Measured on this repo, 9 of 154 versions mis-resolved -- + 0.15.17 to a PR #234 feature commit that shipped in 0.16.0, and 0.15.18 to + that PR's merge commit. A full walk is ~1.1 s, so the break bought nothing. + + ⚠️ `--first-parent` is load-bearing, for the same reason as the missing + break. `git log` orders by commit DATE, so commits merged in from a branch + interleave with the mainline and "last in the newest-first list" is not + topologically oldest -- measured, 6 of 151 versions then resolved to a + commit that is not even an ancestor of their bump. Restricting to the + mainline removes the interleaving at its source: 150 of 151 land exactly on + the `chore: bump ...` commit. The one exception (0.9.2) had its bump made on + a side branch, so first-parent resolves it to the merge that brought it onto + the mainline -- which still declares 0.9.2, so the label-matches-binary + invariant holds, which is the property that actually matters here. + + ⚠️ The pathspec is `:(top)`-prefixed because a plain `-- <path>` is + CWD-RELATIVE. Run from a subdirectory -- which this script explicitly + supports -- it matches nothing and `git log` exits 0 with an EMPTY list, so + the returncode guard never fires and the caller silently falls back to the + branch head. (`show()` is unaffected: `<rev>:<path>` is always root-relative, + so nothing else looks wrong.) + """ + r = run(["git", "log", "--format=%H", "--first-parent", + f"origin/{default_branch}", "--", f":(top){vpath}"]) + if r.returncode: + return None + found = None + for sha in r.stdout.split(): # git log is newest-first + text = show(f"{sha}:{vpath}") + if not text: + continue + try: + if parse_version(kind, text) == version: + found = sha # keep walking back; the last match is the oldest + except SystemExit: + # An older revision the current parser can't read: skip it rather + # than abort the publish. + continue + return found + + def prepared_tags(prefix): """All prepared <prefix><X.Y.Z>.md files on the release-notes branch, as (version_tuple, tag) sorted ascending.""" @@ -194,10 +364,14 @@ def main(): ap.add_argument("--dry-run", action="store_true", help="show what would happen, change nothing") ap.add_argument("--tag", help="publish a specific prepared tag instead of the newest one") ap.add_argument("--allow-version-mismatch", action="store_true", - help="publish a NEW release even though the default branch's version is not " - "this tag's. The build labels its assets from the tree, so the release " - "workflow will refuse to attach them; use this only to get the notes up, " - "and attach the assets later with a workflow_dispatch after bumping") + help="publish a NEW release even though no commit declares its version and " + "the branch head's differs. What this costs depends on the repo: " + "wincompose's workflow asserts the built version against the tag and " + "refuses the assets, leaving a release with no downloads; the firmware " + "workflow names its assets FROM THE TAG without checking, so it ships a " + "binary whose filename and reported version disagree. Prefer merging " + "the bump; use this only to get notes up, and attach assets later with " + "a workflow_dispatch") args = ap.parse_args() # Print UTF-8 (emoji in the notes) even on a cp1252 Windows console. @@ -217,72 +391,177 @@ def main(): # publish* — NOT the tree version, which drifts forward on every PR merge # (each merge auto-bumps the version, so the tree is usually ahead of the # prepared release by the time you publish). Pick the newest prepared tag. + prepared = prepared_tags(tag_prefix) if args.tag: tag = args.tag - notes = show(f"origin/release-notes:{tag}.md") - if not notes or not notes.strip(): - die(f"no prepared notes for {tag} on the release-notes branch " - f"(expected release-notes:{tag}.md).") else: - prepared = prepared_tags(tag_prefix) if not prepared: die("no prepared release notes on the release-notes branch " f"(no {tag_prefix}<X.Y.Z>.md files).\n" " Draft + stage them first with the polykybd-github-release skill.") tag = prepared[-1][1] - notes = show(f"origin/release-notes:{tag}.md") if len(prepared) > 1: others = ", ".join(t for _, t in prepared[:-1]) print(f"note: newest prepared tag is {tag}. Others on the branch: {others}") print(f" (use --tag to publish a specific one.)") + # One guard for both paths. prepared_tags() matches on FILENAME only, so an + # empty or unreadable <TAG>.md reaches here on the auto-select path too -- + # where it used to raise IndexError on lines[0] (or AttributeError on a None + # from show()) instead of saying what was wrong. + notes = show(f"origin/release-notes:{tag}.md") + if not notes or not notes.strip(): + die(f"no prepared notes for {tag} on the release-notes branch " + f"(expected release-notes:{tag}.md, non-empty).") + owner, repo = owner_repo(root) token = get_token() - - # ⚠️ The build labels its assets from the TREE, never from the tag: the - # version is read off the built exe (iscc's filename, the zip name, the - # About tab, the number Updater.cs compares against status.txt). So for a - # release that does not exist yet, any disagreement between the default - # branch's version and the tag is a release whose downloads carry the wrong - # number -- PK-0.9.19 shipped WinCompose-Setup-0.9.18.exe that way (issue - # #21), published 50 minutes before its bump merged. release.yml now refuses - # to attach mislabelled assets, which turns the same mistake into a release - # with NO downloads; this is the check that stops it before anything is - # published at all. - # - # Enforced only when the tag does not exist yet, i.e. when publishing would - # CREATE the release and therefore trigger a build. Re-applying notes to an - # already-published release re-runs nothing, so the tree's version is then - # irrelevant and a difference is just the post-prep merges. + version = tag[len(tag_prefix):] + + # Does this release take the "Latest" badge? /releases/latest is what the + # download links resolve, so it must not be re-pointed at an OLDER release + # (--tag exists to publish one). The comparison is against what is + # PUBLISHED, not against the newest prepared notes: notes can be staged for + # a version that has not shipped, and comparing against them withholds the + # badge from the release being published -- see published_latest_version(). + published = published_latest_version(owner, repo, tag_prefix, token) + if published is None: + # Could not tell. Fall back to the notes comparison, which is the older + # behaviour: it can withhold the badge but never steals it for an older + # release, and says so either way. + newest_tag = prepared[-1][1] if prepared else tag + is_latest = (tag == newest_tag) + latest_why = "could not read the published latest release; judged from the prepared notes" + elif published == "": + is_latest = True + latest_why = "no published release yet" + else: + is_latest = version_tuple(version) >= version_tuple(published) + latest_why = f"published latest is {tag_prefix}{published}" + + # Tag the commit that DECLARES this version, not the branch head -- see + # commit_for_version(). Falls back to the head so a publish never becomes + # impossible, but says so loudly, because the fallback is the old broken + # behaviour and must not pass unnoticed. + target = commit_for_version(kind, vpath, default_branch, version) + pinned = target is not None + shallow = False + if not pinned: + shallow = (run(["git", "rev-parse", "--is-shallow-repository"]) + .stdout.strip() == "true") + print(f"WARNING: no commit on {default_branch} declares version {version}; " + f"falling back to the branch head.") + if shallow: + print(" This clone is SHALLOW, so the search saw truncated " + "history. Run `git fetch origin --unshallow` and retry.") + else: + print(" The published binary may then report a different " + "version than this tag's label. Check before announcing it.") + target = default_branch + target_desc = default_branch + else: + target_desc = f"{target[:10]} (declares {version})" + + # Does publishing CREATE the release? That is what triggers a build, and so + # what both guards below are conditioned on: re-applying notes to an + # already-published release re-runs nothing. + exists = release_exists(owner, repo, tag, token) + # Unknown counts as creating: the stricter case, since guessing the other + # way is what publishes a release whose assets are wrong. + creating = exists is not True + unknown_note = "" + if exists is None: + unknown_note = (f"\n (could not reach the API to check whether {tag} already exists, so\n" + f" this assumes it does not -- the stricter reading.)") + + # ⚠️ An EXISTING tag beats the pin, and nothing about the pin says so. + # `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 + # already there, the build comes from wherever it points and the pin is + # only a claim. Live case: wincompose PK-0.9.20's tag sits on the pre-bump + # commit, its release was deleted, and the pin resolves the bump commit -- + # so the script would have printed the right commit and published the wrong + # one. This fires even when the version check below passes, which is the + # whole point: there the pin found the correct commit and the mismatch + # 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]): + 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" + f" ref that declares {version} to attach assets to the existing tag, or move\n" + f" the tag to {target[:10]} and publish." + unknown_note) + if at and target == default_branch: + print(f"note: the tag {tag} already exists, at {at[:10]}; creating a release does " + f"not move it,\n so that commit is what gets built.") + + # What the default branch declares right now. Read only to describe the + # drift or to refuse; the tag's own commit is what gets published. vtext = show(f"origin/{default_branch}:{vpath}") + tree_ver = None if vtext: try: tree_ver = parse_version(kind, vtext) except SystemExit: - tree_ver = None - tag_ver = tag[len(tag_prefix):] - exists = release_exists(owner, repo, tag, token) - # Unknown counts as creating: the stricter case, since guessing the - # other way is what publishes a release whose assets get refused. - creating = exists is not True - if tree_ver and tree_ver != tag_ver: - where = "behind" if version_tuple(tree_ver) < version_tuple(tag_ver) else "ahead of" - msg = (f"{default_branch} is at {tree_ver}, {where} the prepared {tag}.\n" - f" The build reads its version from {vpath}, not from the tag, so the\n" - f" assets would be labelled {tree_ver} and release.yml will refuse them.\n" - f" Set {vpath} to {tag_ver} on {default_branch} first, then publish.") - if exists is None: - msg += (f"\n (could not reach the API to check whether {tag} already exists,\n" - f" so this assumes it does not -- the stricter reading.)") - if creating and not args.allow_version_mismatch: - die(msg + "\n (--allow-version-mismatch publishes the notes anyway, with no assets.)") - if creating: - print("warning: " + msg) - print(" --allow-version-mismatch given: expect a release with no assets,") - print(" and attach them with a workflow_dispatch once the bump has merged.") - else: - print(f"note: {default_branch} is at {tag_prefix}{tree_ver}; re-applying notes to the " - f"existing {tag} (no build runs, so the difference does not matter).") + pass + + if pinned: + # Drift is normal and harmless here: every merge auto-bumps, so the tree + # is usually ahead of the prepared tag by the time you publish, and the + # later merges ship in the NEXT release rather than silently joining + # this one. The pin is what makes it harmless -- do NOT turn this into a + # refusal, or almost every legitimate firmware publish stops. + if tree_ver and tree_ver != version: + print(f"note: default branch has moved on to {tag_prefix}{tree_ver}; publishing " + f"prepared {tag} at its own commit, so the drift is harmless.") + elif tree_ver and tree_ver != version: + # ⚠️ Here the pin found NOTHING and the head declares something else, so + # the branch head is what gets built and its version is what labels the + # assets. That is how PK-0.9.19 shipped WinCompose-Setup-0.9.18.exe + # (wincompose issue #21): published 50 minutes before its own bump + # merged, so no commit declared the new version and the pin had nothing + # to find. The release workflow asserts the built version against the + # tag now, which turns the same mistake into a published release with NO + # downloads at all -- this is the check that stops it before anything is + # published. + # + # Enforced only when publishing would CREATE the release (see above). + where = "behind" if version_tuple(tree_ver) < version_tuple(version) else "ahead of" + msg = (f"{default_branch} is at {tree_ver}, {where} the prepared {tag}, and no commit\n" + f" on it declares {version}. The build takes its version from {vpath}, not\n" + f" from the tag, so what ships would be built from {tree_ver} while the\n" + f" release says {version}. Merge the bump that sets {vpath} to {version} on\n" + f" {default_branch}, then publish.") + if shallow: + # ⚠️ The search may simply not have SEEN the commit. Say so first: + # told to "merge the bump" for a version that was bumped months ago, + # the obvious next move is --allow-version-mismatch, which publishes + # the very release this is trying to prevent. + msg += ("\n This clone is SHALLOW, so the search saw truncated history and the\n" + " commit may well exist. Run `git fetch origin --unshallow` and retry\n" + " BEFORE reaching for --allow-version-mismatch.") + msg += unknown_note + # ⚠️ What the override actually costs is NOT the same in every repo, so + # do not promise one outcome. wincompose's release workflow asserts the + # built exe's version against the tag and refuses the assets, leaving a + # release with no downloads. The firmware workflow does neither: it + # NAMES its assets from the tag (`VER="${TAG#PolyKybd-fw-v}"`), so the + # override there publishes a .bin whose filename and internal + # FW_VERSION disagree -- which is the mis-flash that sent a firmware + # bisect to a wrong conclusion for several rounds (#258). + # (Found by Greptile on qmk_firmware#356.) + if creating and not args.allow_version_mismatch: + die(msg + "\n (--allow-version-mismatch publishes anyway; read its --help first.)") + if creating: + print("warning: " + msg) + print(" --allow-version-mismatch given. Check what this repo's release") + print(" workflow does with assets whose version is not the tag's: it") + print(" either refuses them, leaving no downloads, or names them from") + print(" the tag and ships a binary that reports something else.") + else: + print(f"note: {default_branch} is at {tag_prefix}{tree_ver}; re-applying notes to the " + f"existing {tag} (no build runs, so the difference does not matter).") lines = notes.splitlines() title = re.sub(r"^#\s*", "", lines[0]).strip() body = "\n".join(lines[1:]).strip("\n") @@ -290,8 +569,12 @@ def main(): die(f"{tag}.md has an empty title line (first line must be '# <title>').") print(f"repo : {owner}/{repo} ({kind})") - print(f"tag : {tag} target: {default_branch}") + print(f"tag : {tag} target: {target_desc}") print(f"title : {title}") + # Always printed, in both directions: which release keeps the Latest badge + # decides where the download links point, and the REASON is what tells you + # the decision came from the published releases rather than from a fallback. + print(f"latest : {'YES' if is_latest else 'NO'} — {latest_why}") print(f"body : {len(body)} chars, {body.count(chr(10)) + 1} lines") print("-" * 60) print(body) @@ -307,7 +590,7 @@ def main(): status, rel = api(token, "GET", f"/repos/{owner}/{repo}/releases/tags/{tag}") if status == 200: st, res = api(token, "PATCH", f"/repos/{owner}/{repo}/releases/{rel['id']}", - {"name": title, "body": body, "make_latest": "true", "draft": False}) + {"name": title, "body": body, "make_latest": str(is_latest).lower(), "draft": False}) if st >= 300: die(f"updating existing release failed ({st}): {res.get('message')}") print(f"updated existing release {tag}") @@ -318,10 +601,10 @@ def main(): st, res = api(token, "POST", f"/repos/{owner}/{repo}/releases", { "tag_name": tag, - "target_commitish": default_branch, + "target_commitish": target, "name": title, "body": body, - "make_latest": "true", + "make_latest": str(is_latest).lower(), "draft": False, "prerelease": False, })