Actually offer a preview the release it precedes, and fix two more gaps
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m39s
Build App (Preview) / build-windows (pull_request) Successful in 4m50s
Build App (Preview) / build-linux (pull_request) Successful in 7m25s
Build App (Preview) / prune-previews (pull_request) Successful in 6s
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-macos (pull_request) Successful in 2m39s
Build App (Preview) / build-windows (pull_request) Successful in 4m50s
Build App (Preview) / build-linux (pull_request) Successful in 7m25s
Build App (Preview) / prune-previews (pull_request) Successful in 6s
An Opus review of the previous commit found its headline claim didn't hold: a preview and the release it precedes compute to the identical numeric version by construction, but check_for_updates compared with a strict `>` against the bare CARGO_PKG_VERSION (never the suffixed display string), so `(0,4,13) > (0,4,13)` is false and the release was never offered. Plain semver ordering doesn't make a `-preview.<sha>` suffix sort below the same numeric release on its own here, since the comparison never sees the suffix at all. pick_update now takes is_preview_build, derived from whether TRIPLE_C_BUILD_SUFFIX was baked in, and relaxes that one comparison to `>=` — so "a release exists at my own number" reads as an update. A production build still requires strictly newer. Also: ported build-app.yml's `git tag --points-at HEAD` guard into the preview version computation. Without it, workflow_dispatch (which this workflow allows on main, not just PR builds) run on a commit a release was already cut from would compute one past that release — reintroducing "preview outranks production" through the manual-dispatch door. And corrected two comments that claimed the prerelease filter was currently a no-op: backfill-releases.yml mirrors every Gitea release to GitHub unfiltered, prerelease flag included, so it's real defence-in-depth against a dispatched backfill leaking a preview release, not a no-op. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FGjXq6fqtAFHdbhk4f3PfZ
This commit is contained in:
@@ -43,13 +43,18 @@ name: Build App (Preview)
|
|||||||
# prunes previous previews itself, keeping the newest few. Bundles are ~130 MB a
|
# prunes previous previews itself, keeping the newest few. Bundles are ~130 MB a
|
||||||
# release; the point of a preview is the build you are testing now.
|
# release; the point of a preview is the build you are testing now.
|
||||||
#
|
#
|
||||||
# Nothing here reaches GitHub: `build-app.yml` is the only workflow that
|
# A preview release is not meant to reach GitHub. `build-app.yml`'s inline
|
||||||
# mirrors a release there, and it does so inline, gated on `prerelease: false`
|
# mirror never sees one (it only runs for its own `push`-triggered release),
|
||||||
# — a preview release is never a candidate. (The previous mechanism here,
|
# but `backfill-releases.yml` pulls every Gitea release unfiltered and would
|
||||||
# `sync-release.yml`, was `workflow_dispatch`-only and read
|
# faithfully forward a preview's `prerelease: true` if it were ever dispatched
|
||||||
|
# while one existed — so `GitHubRelease::prerelease` in `update_commands.rs`
|
||||||
|
# is real defence, not a no-op, even though the `preview-<sha>` tag shape
|
||||||
|
# (never valid semver) already blocks it independently. (The previous
|
||||||
|
# mechanism here, `sync-release.yml`, was `workflow_dispatch`-only and read
|
||||||
# `gitea.event.release.*` fields that are only ever populated by a `release`
|
# `gitea.event.release.*` fields that are only ever populated by a `release`
|
||||||
# trigger, so it could never have actually run; deleted rather than fixed,
|
# trigger, so it could never have actually run; deleted rather than fixed,
|
||||||
# since build-app.yml's inline mirror already does its job. See triple-c#32.)
|
# since build-app.yml's inline mirror already does what it was meant to do
|
||||||
|
# for real releases. See triple-c#32.)
|
||||||
|
|
||||||
env:
|
env:
|
||||||
GITEA_URL: ${{ gitea.server_url }}
|
GITEA_URL: ${{ gitea.server_url }}
|
||||||
@@ -115,15 +120,37 @@ jobs:
|
|||||||
# Reading the same `v${MAJOR_MINOR}.*` tags (including the `-mac`
|
# Reading the same `v${MAJOR_MINOR}.*` tags (including the `-mac`
|
||||||
# / `-win` suffixed ones a partially-published release can leave
|
# / `-win` suffixed ones a partially-published release can leave
|
||||||
# behind) means a preview built right before a release computes the
|
# behind) means a preview built right before a release computes the
|
||||||
# exact number that release is about to take — so semver already
|
# exact number that release is about to take — e.g. `0.4.13` for
|
||||||
# orders `0.4.12-preview.<sha> < 0.4.12`, and a preview user is
|
# both. That makes the two numerically *equal*, not "preview less
|
||||||
# offered the real release the moment it ships. See triple-c#32.
|
# than release" — plain semver ordering does not make a
|
||||||
|
# `-preview.<sha>` suffix sort lower on its own here, because
|
||||||
|
# `check_for_updates` compares against the bare, stripped
|
||||||
|
# `CARGO_PKG_VERSION`, never the suffixed display string. What
|
||||||
|
# closes the loop is `update_commands.rs`'s `is_preview_build`
|
||||||
|
# check, which relaxes that one comparison to `>=` specifically so
|
||||||
|
# "a release exists at my own number" reads as an update. See
|
||||||
|
# triple-c#32.
|
||||||
HIGHEST=$(git tag -l "v${MAJOR_MINOR}.*" \
|
HIGHEST=$(git tag -l "v${MAJOR_MINOR}.*" \
|
||||||
| grep -E "^v${MAJOR_MINOR}\.[0-9]+(-mac|-win)?$" \
|
| grep -E "^v${MAJOR_MINOR}\.[0-9]+(-mac|-win)?$" \
|
||||||
| sed -E "s/^v${MAJOR_MINOR}\.([0-9]+).*/\1/" \
|
| sed -E "s/^v${MAJOR_MINOR}\.([0-9]+).*/\1/" \
|
||||||
| sort -n | tail -1 || true)
|
| sort -n | tail -1 || true)
|
||||||
|
|
||||||
if [ -n "$HIGHEST" ]; then
|
# Mirrors build-app.yml's own `EXISTING` guard: this workflow is
|
||||||
|
# also `workflow_dispatch`-able on `main`, not just PR-triggered, so
|
||||||
|
# HEAD can be a commit a release was already cut from. Without this,
|
||||||
|
# dispatching a preview there would compute `HIGHEST + 1` — one past
|
||||||
|
# that release — and produce exactly the "preview outranks
|
||||||
|
# production" failure triple-c#32 was filed over, just reintroduced
|
||||||
|
# through the manual-dispatch door instead of the automatic one.
|
||||||
|
EXISTING=$(git tag --points-at HEAD \
|
||||||
|
| grep -E "^v${MAJOR_MINOR}\.[0-9]+$" \
|
||||||
|
| sed -E "s/^v${MAJOR_MINOR}\.([0-9]+)$/\1/" \
|
||||||
|
| sort -n | tail -1 || true)
|
||||||
|
|
||||||
|
if [ -n "$EXISTING" ]; then
|
||||||
|
echo "HEAD is already tagged v${MAJOR_MINOR}.${EXISTING} — matching it"
|
||||||
|
PATCH="${EXISTING}"
|
||||||
|
elif [ -n "$HIGHEST" ]; then
|
||||||
echo "Highest patch already used on this line: ${HIGHEST}"
|
echo "Highest patch already used on this line: ${HIGHEST}"
|
||||||
PATCH=$((HIGHEST + 1))
|
PATCH=$((HIGHEST + 1))
|
||||||
else
|
else
|
||||||
|
|||||||
@@ -69,7 +69,19 @@ pub async fn check_for_updates() -> Result<Option<UpdateInfo>, String> {
|
|||||||
&[".AppImage", ".deb", ".rpm"]
|
&[".AppImage", ".deb", ".rpm"]
|
||||||
};
|
};
|
||||||
|
|
||||||
match pick_update(&releases, current_semver, platform_extensions) {
|
// `current_version` above is always the bare, stripped `CARGO_PKG_VERSION`
|
||||||
|
// — the preview workflow patches `Cargo.toml` with that before compiling,
|
||||||
|
// never the `-preview.<sha>`-suffixed one `get_app_version()` reports —
|
||||||
|
// so a preview build and the release it precedes compile to the identical
|
||||||
|
// numeric tuple by construction (see `build-app-preview.yml`'s "highest
|
||||||
|
// tag used, +1" computation). A strict `>` therefore never fires for the
|
||||||
|
// one release a preview most needs to be offered. `is_preview_build`
|
||||||
|
// relaxes that one comparison to `>=` so "there is a real release at my
|
||||||
|
// own number" reads as an update, without touching the production case
|
||||||
|
// — see `pick_update`.
|
||||||
|
let is_preview_build = option_env!("TRIPLE_C_BUILD_SUFFIX").is_some_and(|s| !s.is_empty());
|
||||||
|
|
||||||
|
match pick_update(&releases, current_semver, platform_extensions, is_preview_build) {
|
||||||
Some(release) => {
|
Some(release) => {
|
||||||
// Only include assets matching the current platform
|
// Only include assets matching the current platform
|
||||||
let assets = release
|
let assets = release
|
||||||
@@ -108,15 +120,22 @@ pub async fn check_for_updates() -> Result<Option<UpdateInfo>, String> {
|
|||||||
///
|
///
|
||||||
/// Three filters, all of which must pass: not a prerelease (see the long
|
/// Three filters, all of which must pass: not a prerelease (see the long
|
||||||
/// comment on `GitHubRelease::prerelease`), at least one asset for this
|
/// comment on `GitHubRelease::prerelease`), at least one asset for this
|
||||||
/// platform, and a tag that parses as semver *and* is newer than what is
|
/// platform, and a tag that parses as semver *and* beats what is running. A
|
||||||
/// running. A tag that does not parse — a `-preview.<sha>` suffix, most
|
/// tag that does not parse — a `-preview.<sha>` suffix, most realistically —
|
||||||
/// realistically — is skipped rather than erroring, the same as it always
|
/// is skipped rather than erroring, the same as it always has been; nothing
|
||||||
/// has been; nothing here changes what an update tag is expected to look
|
/// here changes what an update tag is expected to look like, only what
|
||||||
/// like, only what channel it is allowed to come from.
|
/// channel it is allowed to come from.
|
||||||
|
///
|
||||||
|
/// `is_preview_build` relaxes "beats" from `>` to `>=`. A preview build's
|
||||||
|
/// `current_semver` is the bare number it was compiled with, which is by
|
||||||
|
/// construction identical to the release it precedes — see the comment at
|
||||||
|
/// `check_for_updates`'s call site — so a strict `>` would never fire for
|
||||||
|
/// exactly the release a preview install most needs to be told about.
|
||||||
fn pick_update<'a>(
|
fn pick_update<'a>(
|
||||||
releases: &'a [GitHubRelease],
|
releases: &'a [GitHubRelease],
|
||||||
current_semver: (u32, u32, u32),
|
current_semver: (u32, u32, u32),
|
||||||
platform_extensions: &[&str],
|
platform_extensions: &[&str],
|
||||||
|
is_preview_build: bool,
|
||||||
) -> Option<&'a GitHubRelease> {
|
) -> Option<&'a GitHubRelease> {
|
||||||
releases
|
releases
|
||||||
.iter()
|
.iter()
|
||||||
@@ -127,7 +146,13 @@ fn pick_update<'a>(
|
|||||||
.any(|a| platform_extensions.iter().any(|ext| a.name.ends_with(ext)))
|
.any(|a| platform_extensions.iter().any(|ext| a.name.ends_with(ext)))
|
||||||
})
|
})
|
||||||
.filter_map(|r| parse_semver_from_tag(&r.tag_name).map(|ver| (r, ver)))
|
.filter_map(|r| parse_semver_from_tag(&r.tag_name).map(|ver| (r, ver)))
|
||||||
.filter(|(_, ver)| *ver > current_semver)
|
.filter(|(_, ver)| {
|
||||||
|
if is_preview_build {
|
||||||
|
*ver >= current_semver
|
||||||
|
} else {
|
||||||
|
*ver > current_semver
|
||||||
|
}
|
||||||
|
})
|
||||||
.max_by_key(|(_, ver)| *ver)
|
.max_by_key(|(_, ver)| *ver)
|
||||||
.map(|(r, _)| r)
|
.map(|(r, _)| r)
|
||||||
}
|
}
|
||||||
@@ -205,19 +230,19 @@ mod tests {
|
|||||||
#[test]
|
#[test]
|
||||||
fn a_prerelease_is_never_offered_even_if_its_tag_would_otherwise_win() {
|
fn a_prerelease_is_never_offered_even_if_its_tag_would_otherwise_win() {
|
||||||
let releases = vec![release("v9.9.9", true, &["app-9.9.9.AppImage"])];
|
let releases = vec![release("v9.9.9", true, &["app-9.9.9.AppImage"])];
|
||||||
assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS).is_none());
|
assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS, false).is_none());
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_release_with_no_asset_for_this_platform_is_skipped() {
|
fn a_release_with_no_asset_for_this_platform_is_skipped() {
|
||||||
let releases = vec![release("v0.4.12", false, &["app-0.4.12.msi"])];
|
let releases = vec![release("v0.4.12", false, &["app-0.4.12.msi"])];
|
||||||
assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS).is_none());
|
assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS, false).is_none());
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn a_release_that_is_not_newer_is_not_offered() {
|
fn a_release_that_is_not_newer_is_not_offered() {
|
||||||
let releases = vec![release("v0.4.10", false, &["app.AppImage"])];
|
let releases = vec![release("v0.4.10", false, &["app.AppImage"])];
|
||||||
assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS).is_none());
|
assert!(pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS, false).is_none());
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -228,7 +253,7 @@ mod tests {
|
|||||||
release("preview-a1b2c3d", false, &["app.AppImage"]),
|
release("preview-a1b2c3d", false, &["app.AppImage"]),
|
||||||
release("v0.4.12", false, &["app.AppImage"]),
|
release("v0.4.12", false, &["app.AppImage"]),
|
||||||
];
|
];
|
||||||
let best = pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS).unwrap();
|
let best = pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS, false).unwrap();
|
||||||
assert_eq!(best.tag_name, "v0.4.12");
|
assert_eq!(best.tag_name, "v0.4.12");
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -239,9 +264,37 @@ mod tests {
|
|||||||
release("v0.4.13", false, &["app.AppImage"]),
|
release("v0.4.13", false, &["app.AppImage"]),
|
||||||
release("v0.4.12", false, &["app.AppImage"]),
|
release("v0.4.12", false, &["app.AppImage"]),
|
||||||
];
|
];
|
||||||
let best = pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS).unwrap();
|
let best = pick_update(&releases, (0, 4, 10), LINUX_EXTENSIONS, false).unwrap();
|
||||||
assert_eq!(best.tag_name, "v0.4.13");
|
assert_eq!(best.tag_name, "v0.4.13");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── is_preview_build (>= instead of >) ─────────────────────────────────
|
||||||
|
|
||||||
|
/// The exact scenario triple-c#32 was filed to fix: a preview compiled as
|
||||||
|
/// `0.4.12-preview.<sha>` (bare `CARGO_PKG_VERSION` "0.4.12") must be
|
||||||
|
/// offered the `v0.4.12` release that follows it, even though the two
|
||||||
|
/// compute to the identical numeric tuple.
|
||||||
|
#[test]
|
||||||
|
fn a_preview_build_is_offered_the_release_it_precedes() {
|
||||||
|
let releases = vec![release("v0.4.12", false, &["app.AppImage"])];
|
||||||
|
assert!(pick_update(&releases, (0, 4, 12), LINUX_EXTENSIONS, false).is_none());
|
||||||
|
let best = pick_update(&releases, (0, 4, 12), LINUX_EXTENSIONS, true).unwrap();
|
||||||
|
assert_eq!(best.tag_name, "v0.4.12");
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_preview_build_is_not_offered_an_older_release() {
|
||||||
|
let releases = vec![release("v0.4.11", false, &["app.AppImage"])];
|
||||||
|
assert!(pick_update(&releases, (0, 4, 12), LINUX_EXTENSIONS, true).is_none());
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn a_production_build_still_requires_strictly_newer() {
|
||||||
|
// A production build must never treat "equal" as an update — that
|
||||||
|
// would perpetually re-offer the version already running.
|
||||||
|
let releases = vec![release("v0.4.12", false, &["app.AppImage"])];
|
||||||
|
assert!(pick_update(&releases, (0, 4, 12), LINUX_EXTENSIONS, false).is_none());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Check whether a newer container image is available in the registry.
|
/// Check whether a newer container image is available in the registry.
|
||||||
|
|||||||
@@ -29,10 +29,18 @@ pub struct GitHubRelease {
|
|||||||
/// Whether GitHub itself has this release marked as a prerelease.
|
/// Whether GitHub itself has this release marked as a prerelease.
|
||||||
/// `#[serde(default)]` rather than required: every response GitHub sends
|
/// `#[serde(default)]` rather than required: every response GitHub sends
|
||||||
/// carries this, but nothing here should refuse to parse the rest of a
|
/// carries this, but nothing here should refuse to parse the rest of a
|
||||||
/// release over one missing field. No production release is ever
|
/// release over one missing field. Defaults to `false` (offered) rather
|
||||||
/// mirrored with this `true` today — see `check_for_updates`, which
|
/// than `true` (excluded) — a missing field only happens if GitHub's API
|
||||||
/// filters on it explicitly rather than relying on that being an
|
/// shape changes, and "API changed, therefore updates silently stop
|
||||||
/// accident of what happens to get mirrored.
|
/// working forever" is the worse failure of the two.
|
||||||
|
///
|
||||||
|
/// `build-app.yml`'s own mirror never publishes a prerelease, but
|
||||||
|
/// `.gitea/workflows/backfill-releases.yml` forwards every Gitea release
|
||||||
|
/// unfiltered, `prerelease` included — so if it were ever dispatched
|
||||||
|
/// while a preview release existed, this field is what stops
|
||||||
|
/// `check_for_updates` from offering it (the `preview-<sha>` tag shape
|
||||||
|
/// already fails semver parsing independently, but this is real
|
||||||
|
/// defence-in-depth, not a no-op).
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub prerelease: bool,
|
pub prerelease: bool,
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user