Skip to main content

max / makenotwork

Check the version against the tag, not against the working copy The version preflight read the ordinary checkout, which stopped being the tree a release builds when releases moved into their own worktrees. It answered a question about a tree nothing compiles: releasing v0.4.4 while main had moved to 0.4.5 was refused for "version drift", and a Cargo.toml that disagreed with the tag it was tagged in passed. It now runs after the pin and reads each version source out of the tag with `git show <tag>:./<path>` on a build host -- one host is enough, since the barrier has already proven they are all on the same commit. No worktree path of its own and no assumption that the daemon holds a copy of the repo. `check_version_consistency` keeps its local-filesystem reader for the resolve-the-version path; the judgement half is split out as `versions_agree` so both feed the same rule.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-24 18:13 UTC
Signed with PGP, not checked
Commit: 337d8b6b6f10e35aecb7aea4d21c3ff676f535f7
Parent: e69ee8e
2 files changed, +226 insertions, -44 deletions
M bento/daemon/src/engine.rs +78 -35
@@ -900,51 +900,84 @@
900 900 expected: &Version,
901 901 ) -> Result<()> {
902 902 let root = expand_tilde(repo);
903 - // (human-readable source label, parsed version) for every source present.
904 - let mut found: Vec<(String, Version)> = Vec::new();
903 + let mut sources: Vec<(String, String)> = Vec::new();
904 + for rel in version_sources(version_path) {
905 + let path = root.join(&rel);
906 + // A source that is absent is skipped — a library crate with only a
907 + // `Cargo.toml` has nothing to disagree with — but one the app NAMES
908 + // must be readable, or the check would pass by failing to look.
909 + match std::fs::read_to_string(&path) {
910 + Ok(raw) => sources.push((rel, raw)),
911 + Err(e) if version_path == Some(rel.as_str()) => {
912 + return Err(e).with_context(|| format!("reading version file {}", path.display()));
913 + }
914 + Err(_) => {}
915 + }
916 + }
917 + versions_agree(repo, &sources, version_path, expected)
918 + }
905 919
906 - let mut consider = |rel: &str, raw: &str, as_json: bool| -> Result<()> {
920 + /// The files a repo can state its version in, in the order they are read:
921 + /// whatever the app names, then the two conventional ones.
922 + ///
923 + /// The app's own `version_path` is never read twice, which is why this is a
924 + /// function rather than a constant.
925 + pub fn version_sources(version_path: Option<&str>) -> Vec<String> {
926 + let mut rels: Vec<String> = version_path.into_iter().map(str::to_string).collect();
927 + for conventional in ["src-tauri/tauri.conf.json", "Cargo.toml"] {
928 + if version_path != Some(conventional) {
929 + rels.push(conventional.to_string());
930 + }
931 + }
932 + rels
933 + }
934 +
935 + /// The judgement half of [`check_version_consistency`], over sources somebody
936 + /// else read.
937 + ///
938 + /// Split out so the same rule can be applied to files read out of the release
939 + /// TAG on a build host, which is where the question actually belongs: the tree
940 + /// a release compiles is the tag's, so a `Cargo.toml` that disagrees with the
941 + /// tag it is tagged in is the drift worth refusing. Reading the working copy
942 + /// instead answered a question about a tree the release does not build.
943 + ///
944 + /// `where_` is only for the error message — a path, or a tag and a host.
945 + pub fn versions_agree(
946 + where_: &str,
947 + sources: &[(String, String)],
948 + version_path: Option<&str>,
949 + expected: &Version,
950 + ) -> Result<()> {
951 + let mut found: Vec<(String, Version)> = Vec::new();
952 + for (rel, raw) in sources {
953 + // The app's own `version_path` can be either shape, so it is decided by
954 + // extension; the two conventional sources are what they are.
955 + let as_json = std::path::Path::new(rel)
956 + .extension()
957 + .is_some_and(|e| e.eq_ignore_ascii_case("json"));
907 958 let ver = if as_json {
908 959 version_from_tauri_json(raw)
909 960 } else {
910 - version_from_cargo_toml(raw)
961 + // A Cargo.toml with neither `[package].version` nor
962 + // `[workspace.package].version` (a pure virtual workspace) carries
963 + // no version to check — skip it rather than fail. An app that NAMED
964 + // this file is held to it.
965 + match version_from_cargo_toml(raw) {
966 + Ok(v) => Ok(v),
967 + Err(e) if version_path == Some(rel.as_str()) => Err(e),
968 + Err(_) => continue,
969 + }
911 970 }?;
912 - let parsed = Version::parse(&ver).map_err(|e| anyhow::anyhow!(e))?;
913 - found.push((rel.to_string(), parsed));
914 - Ok(())
915 - };
916 -
917 - if let Some(vp) = version_path {
918 - let path = root.join(vp);
919 - let raw = std::fs::read_to_string(&path)
920 - .with_context(|| format!("reading version file {}", path.display()))?;
921 - let is_json = std::path::Path::new(vp)
922 - .extension()
923 - .is_some_and(|e| e.eq_ignore_ascii_case("json"));
924 - consider(vp, &raw, is_json)?;
925 - }
926 - let tauri_conf = root.join("src-tauri").join("tauri.conf.json");
927 - if version_path != Some("src-tauri/tauri.conf.json") && tauri_conf.exists() {
928 - let raw = std::fs::read_to_string(&tauri_conf)
929 - .with_context(|| format!("reading {}", tauri_conf.display()))?;
930 - consider("src-tauri/tauri.conf.json", &raw, true)?;
931 - }
932 - let cargo_toml = root.join("Cargo.toml");
933 - if version_path != Some("Cargo.toml") && cargo_toml.exists() {
934 - // A Cargo.toml with neither `[package].version` nor
935 - // `[workspace.package].version` (a pure virtual workspace) carries no
936 - // version to check — skip it rather than fail.
937 - if let Ok(raw) = std::fs::read_to_string(&cargo_toml)
938 - && version_from_cargo_toml(&raw).is_ok()
939 - {
940 - consider("Cargo.toml", &raw, false)?;
941 - }
971 + found.push((
972 + rel.clone(),
973 + Version::parse(&ver).map_err(|e| anyhow::anyhow!(e))?,
974 + ));
942 975 }
943 976
944 977 let disagree: Vec<&(String, Version)> = found.iter().filter(|(_, v)| v != expected).collect();
945 978 anyhow::ensure!(
946 979 disagree.is_empty(),
947 - "version drift in {repo}: building {expected} but {}",
980 + "version drift in {where_}: building {expected} but {}",
948 981 disagree
949 982 .iter()
950 983 .map(|(src, v)| format!("{src} says {v}"))
@@ -954,6 +987,16 @@
954 987 Ok(())
955 988 }
956 989
990 + /// Read one file as it exists in `tag`, without checking anything out.
991 + ///
992 + /// `<rev>:./<path>` resolves the path relative to `-C`, so this is asked from
993 + /// the app's own directory and needs no knowledge of where that sits inside the
994 + /// repository. A non-zero exit means the file is not in the tag, which is the
995 + /// same "absent, so nothing to disagree with" the local read treats it as.
996 + pub fn git_show_file_cmd(dir: &str, tag: &str, rel: &str) -> String {
997 + format!("git -C \"{dir}\" show \"{tag}:./{rel}\"")
998 + }
999 +
957 1000 /// Every `X.Y.Z`-shaped version embedded in an artifact file name. Each maximal
958 1001 /// run of digits-and-dots contributes its first three numeric fields:
959 1002 /// `GoingsOn_0.5.0_aarch64.dmg` and `demo-9.9.9.bin` both yield one version (the
M bento/daemon/src/runner.rs +148 -9
@@ -283,6 +283,58 @@
283 283 sha: String,
284 284 }
285 285
286 + /// Confirm the tag states the version being built, in every file that states a
287 + /// version.
288 + ///
289 + /// Read out of the tag with `git show` on a build host, so it needs no worktree
290 + /// path of its own and no assumption that the daemon has a copy of the repo:
291 + /// one host is enough, since the barrier has already proven they are all on the
292 + /// same commit.
293 + ///
294 + /// Skipped when nothing was pinned, which is `pin_release_sha = false` — tests,
295 + /// whose repos are plain directories with no tag to read.
296 + async fn check_version_at_tag(
297 + state: &AppState,
298 + app: &AppId,
299 + version: &Version,
300 + pinned: &Pinned,
301 + ) -> Result<()> {
302 + let Some((host, dir)) = pinned.build_dirs.first() else {
303 + return Ok(());
304 + };
305 + let Some(cfg) = state.topo.app(app) else {
306 + return Ok(());
307 + };
308 + let Some(exec) = state.executors.get(host) else {
309 + return Ok(());
310 + };
311 + let tag = cfg.tag_for(version);
312 + let version_path = cfg.version_path.as_deref();
313 +
314 + let mut sources: Vec<(String, String)> = Vec::new();
315 + for rel in engine::version_sources(version_path) {
316 + let step = OpStep::shell(Action::Build, engine::git_show_file_cmd(dir, &tag, &rel));
317 + let mut sink = DiscardSink;
318 + let out = exec
319 + .run_streaming(&step, &mut sink)
320 + .await
321 + .with_context(|| format!("reading {rel} from {tag} on `{host}`"))?;
322 + if out.status.success() {
323 + sources.push((rel, String::from_utf8_lossy(&out.stdout).into_owned()));
324 + } else if version_path == Some(rel.as_str()) {
325 + // A file the app NAMES as its version source has to be there, or
326 + // the check would pass by failing to look.
327 + anyhow::bail!("`{rel}` is not in {tag} on `{host}`, but the app declares it");
328 + }
329 + }
330 + engine::versions_agree(
331 + &format!("{tag} (read on `{host}`)"),
332 + &sources,
333 + version_path,
334 + version,
335 + )
336 + }
337 +
286 338 /// First 12 chars of a sha for a readable error.
287 339 fn short(sha: &str) -> &str {
288 340 sha.get(..12).unwrap_or(sha)
@@ -345,15 +397,6 @@
345 397 version: Version,
346 398 targets: Vec<Target>,
347 399 ) -> Result<i64> {
348 - // Preflight: every version source in the repo must agree with the version
349 - // being built, before any host pulls or compiles. `version_from_repo` reads
350 - // one file, so a tauri.conf.json/Cargo.toml (or explicit-version) mismatch
351 - // would otherwise sail through and file artifacts under the wrong version.
352 - if let Some(cfg) = state.topo.app(&app) {
353 - engine::check_version_consistency(&cfg.repo, cfg.version_path.as_deref(), &version)
354 - .context("version preflight")?;
355 - }
356 -
357 400 // Pin every build host to the release tag and verify they agree, before any
358 401 // target task spawns. Off in tests (their repos aren't git checkouts).
359 402 let pinned = Arc::new(if state.cfg.pin_release_sha {
@@ -367,6 +410,20 @@
367 410 }
368 411 });
369 412
413 + // Then: every version source in the TAG must agree with the version being
414 + // built, before a single host compiles. `version_from_repo` reads one file,
415 + // so a `tauri.conf.json`/`Cargo.toml` (or explicit-version) mismatch would
416 + // otherwise sail through and file artifacts under the wrong version.
417 + //
418 + // After the pin rather than before it, and read from the tag rather than
419 + // from the checkout, because those are two different trees now. Asking the
420 + // checkout answered a question about a tree the release does not build: a
421 + // working copy one commit ahead failed a release of the tag behind it, and
422 + // a `Cargo.toml` that disagreed with the tag it was tagged in passed.
423 + check_version_at_tag(&state, &app, &version, &pinned)
424 + .await
425 + .context("version preflight")?;
426 +
370 427 let build_id: i64 = sqlx::query_scalar(
371 428 "INSERT INTO builds (app, version, status, created_at) VALUES (?, ?, 'running', ?) RETURNING id",
372 429 )
@@ -2904,6 +2961,88 @@
2904 2961 assert_eq!(repo_status(&repo), "", "and the checkout was never in it");
2905 2962 }
2906 2963
2964 + /// A working copy ahead of the tag does not fail the version check.
2965 + ///
2966 + /// The check used to read the checkout, so releasing v0.0.1 while `main` had
2967 + /// already moved to 0.0.2 was refused for "version drift" — about a tree the
2968 + /// release does not build. It reads the tag now, and the tag says 0.0.1.
2969 + #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2970 + async fn a_working_copy_ahead_of_the_tag_does_not_fail_the_version_check() {
2971 + let tmp = tempfile::tempdir().unwrap();
2972 + let repo = tmp.path().join("demo");
2973 + init_git_app(&repo, "0.0.1", Some("v0.0.1"));
2974 + // main moves on, untagged, exactly as it does the day after a release.
2975 + let git = git_in(&repo);
2976 + std::fs::write(
2977 + repo.join("src-tauri/tauri.conf.json"),
2978 + "{\"version\":\"0.0.2\"}",
2979 + )
2980 + .unwrap();
2981 + git(&["add", "-A"]);
2982 + git(&["commit", "-q", "-m", "0.0.2"]);
2983 +
2984 + let mut cfg = Config::for_tests(tmp.path());
2985 + cfg.pin_release_sha = true;
2986 + let pool = crate::db::open(&cfg.db_path).await.unwrap();
2987 + let state = test_state(pool.clone(), one_host_topo(&repo), cfg);
2988 + let build_id = start_build(
2989 + state,
2990 + AppId::new("demo"),
2991 + Version::parse("0.0.1").unwrap(),
2992 + vec!["linux/x86_64".parse().unwrap()],
2993 + )
2994 + .await
2995 + .expect("releasing the tag behind main must not be version drift");
2996 + let (status, error) = await_target(&pool, build_id).await;
2997 + assert_eq!(status, "ok", "({error})");
2998 + }
2999 +
3000 + /// A tag that disagrees with itself is refused, naming the file.
3001 + ///
3002 + /// This is the drift worth catching and the one the old check could not see:
3003 + /// `tauri.conf.json` at 0.0.1 and `Cargo.toml` still at 0.0.2, committed and
3004 + /// tagged that way. Reading the checkout would have compared against
3005 + /// whatever the working copy happened to say instead.
3006 + #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
3007 + async fn a_tag_whose_manifest_disagrees_with_it_is_refused() {
3008 + let tmp = tempfile::tempdir().unwrap();
3009 + let repo = tmp.path().join("demo");
3010 + std::fs::create_dir_all(&repo).unwrap();
3011 + std::fs::write(
3012 + repo.join("Cargo.toml"),
3013 + "[package]\nname = \"demo\"\nversion = \"0.0.2\"\n",
3014 + )
3015 + .unwrap();
3016 + init_git_app(&repo, "0.0.1", Some("v0.0.1"));
3017 +
3018 + let mut cfg = Config::for_tests(tmp.path());
3019 + cfg.pin_release_sha = true;
3020 + let pool = crate::db::open(&cfg.db_path).await.unwrap();
3021 + let state = test_state(pool.clone(), one_host_topo(&repo), cfg);
3022 + let err = start_build(
3023 + state,
3024 + AppId::new("demo"),
3025 + Version::parse("0.0.1").unwrap(),
3026 + vec!["linux/x86_64".parse().unwrap()],
3027 + )
3028 + .await
3029 + .unwrap_err();
3030 + let msg = format!("{err:#}");
3031 + assert!(
3032 + msg.contains("version drift") && msg.contains("Cargo.toml says 0.0.2"),
3033 + "the refusal must name the file and what it says, got: {msg}"
3034 + );
3035 + assert!(
3036 + msg.contains("v0.0.1"),
3037 + "and say which tag it read, got: {msg}"
3038 + );
3039 + let builds: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM builds")
3040 + .fetch_one(&pool)
3041 + .await
3042 + .unwrap();
3043 + assert_eq!(builds, 0, "a refused version preflight writes no build row");
3044 + }
3045 +
2907 3046 /// A preflight that refuses on a LATER host leaves every checkout alone.
2908 3047 ///
2909 3048 /// The hosts are prepared one at a time, so when the second is refused the