From 8a8f525e591c795360bb9bb3a8748ff4c15341f9 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Wed, 28 Oct 2026 03:03:52 -0700 Subject: [PATCH 1/2] feat(version): stamp the git build id into fbuild and fbuild-daemon Two builds of the same workspace version were indistinguishable (#1742): an editable install that kept yesterday's binaries printed the same version as a fresh one, and a daemon left running from an older checkout looked current. Implement follow-up 1 of #1742. fbuild-core gains a build script that stamps FBUILD_BUILD_COMMIT (git rev-parse --short=12 HEAD) and FBUILD_BUILD_DIRTY (tracked-file status), plus a build_id module that reads them. Because every version-reporting crate depends on fbuild-core, one probe keeps the CLI and the daemon agreeing instead of duplicating git logic per binary. The probe is best-effort: no git or no .git degrades the stamp to "unknown" and never fails a build. Surfaced as: - fbuild --version: 2.5.47+git.5fca4d49135b[-dirty] (clap long_version; -V still prints the bare version, and the id is semver build metadata) - fbuild daemon status / daemon list: a Build: line, from the new build_id field on /health and /api/daemon/info. A differing daemon names both builds; a pre-#1742 daemon says so instead of printing blank. No fingerprint, cache key or endpoint derivation changed; this is diagnostic surface only. --- crates/fbuild-cli/src/cli/daemon_cmd.rs | 46 ++++++++ crates/fbuild-cli/src/cli/dispatch.rs | 30 ++++- .../fbuild-cli/src/daemon_client/identity.rs | 1 + crates/fbuild-cli/src/daemon_client/tests.rs | 1 + crates/fbuild-cli/src/daemon_client/types.rs | 4 + crates/fbuild-core/build.rs | 70 ++++++++++++ crates/fbuild-core/src/build_id.rs | 106 ++++++++++++++++++ crates/fbuild-core/src/lib.rs | 1 + crates/fbuild-daemon/src/broker/backend.rs | 2 + crates/fbuild-daemon/src/handlers/health.rs | 2 + crates/fbuild-daemon/src/models.rs | 9 ++ 11 files changed, 270 insertions(+), 2 deletions(-) create mode 100644 crates/fbuild-core/build.rs create mode 100644 crates/fbuild-core/src/build_id.rs diff --git a/crates/fbuild-cli/src/cli/daemon_cmd.rs b/crates/fbuild-cli/src/cli/daemon_cmd.rs index 1836b423a..ca4d8dcfc 100644 --- a/crates/fbuild-cli/src/cli/daemon_cmd.rs +++ b/crates/fbuild-cli/src/cli/daemon_cmd.rs @@ -42,6 +42,7 @@ pub async fn run_daemon(action: DaemonAction) -> fbuild_core::Result<()> { output::result(format!(" Port: {}", info.port)); output::result(format!(" Uptime: {}", uptime)); output::result(format!(" Version: {}", info.version)); + output::result(format!(" Build: {}", build_label(&info.build_id))); output::result(format!( " Mode: {}", if info.dev_mode { "dev" } else { "prod" } @@ -308,6 +309,7 @@ pub async fn run_daemon_list(client: &DaemonClient) -> fbuild_core::Result<()> { output::result(format!(" Port: {}", info.port)); output::result(format!(" Uptime: {}", uptime)); output::result(format!(" Version: {}", info.version)); + output::result(format!(" Build: {}", build_label(&info.build_id))); output::result(format!( " Mode: {}", if info.dev_mode { "dev" } else { "prod" } @@ -815,3 +817,47 @@ fn clear_daemon_records() -> Vec<&'static str> { } removed } + +/// Render the daemon's build id for `daemon status` / `daemon list`. +/// +/// A daemon from this same checkout prints its id alone. Anything else is +/// spelled out, because "same version, different build" is exactly the +/// confusion FastLED/fbuild#1742 was filed for: an older daemon happily serves +/// a newer CLI, and a same-version editable install can be from any commit. +fn build_label(daemon_build_id: &str) -> String { + let cli_build_id = fbuild_core::build_id::build_id(); + let daemon_build_id = daemon_build_id.trim(); + if daemon_build_id.is_empty() { + return format!("{cli_build_id} (daemon predates build ids; reported by this CLI)"); + } + if daemon_build_id == cli_build_id { + return daemon_build_id.to_string(); + } + format!("{daemon_build_id} (this CLI: {cli_build_id})") +} + +#[cfg(test)] +mod build_id_tests { + use super::*; + + #[test] + fn matching_daemon_reports_the_bare_build_id() { + let cli = fbuild_core::build_id::build_id(); + assert_eq!(build_label(&format!(" {cli} ")), cli); + } + + #[test] + fn differing_daemon_names_both_builds() { + let cli = fbuild_core::build_id::build_id(); + let label = build_label("0000000000aa"); + assert!(label.starts_with("0000000000aa")); + assert!(label.contains(&format!("this CLI: {cli}")), "{label}"); + } + + #[test] + fn older_daemon_without_a_stamp_still_prints_a_build() { + let label = build_label(""); + assert!(label.contains(&fbuild_core::build_id::build_id())); + assert!(label.contains("predates build ids")); + } +} diff --git a/crates/fbuild-cli/src/cli/dispatch.rs b/crates/fbuild-cli/src/cli/dispatch.rs index b675900f7..9d6e24ff0 100644 --- a/crates/fbuild-cli/src/cli/dispatch.rs +++ b/crates/fbuild-cli/src/cli/dispatch.rs @@ -1,7 +1,6 @@ //! Top-level async dispatcher: parse argv, set up tracing + Ctrl+C, then //! fan out to per-subcommand handlers in the topic modules. -use clap::Parser; use std::path::Path; use crate::{daemon_client, lib_select, mcp, output, update_check}; @@ -39,13 +38,40 @@ use super::symbols_cmd::run_symbols; use super::sync_cmd::run_sync_cmd; use super::usb_recovery::run_hidden_helper; +/// Parse argv, naming the stamped build id in the long `--version` output +/// (FastLED/fbuild#1742). +/// +/// `-V` keeps printing the bare workspace version, so scripts that parse it are +/// unaffected; only the long form gains the `+git.` build metadata. +/// The value is attached here rather than through a `#[command(version)]` +/// literal because it is computed by `fbuild-core`'s build script. +fn parse_cli(args: I) -> Cli +where + I: IntoIterator, + T: Into + Clone, +{ + use clap::{CommandFactory, FromArgMatches}; + // clap's `long_version` borrows for the process lifetime and the workspace + // does not enable clap's `string` feature, so the computed id is leaked + // instead of widening a dependency feature for one startup string. + let long_version: &'static str = + Box::leak(fbuild_core::build_id::long_version().into_boxed_str()); + let matches = Cli::command() + .long_version(long_version) + .get_matches_from(args); + match Cli::from_arg_matches(&matches) { + Ok(cli) => cli, + Err(error) => error.exit(), + } +} + #[expect( clippy::cognitive_complexity, clippy::too_many_lines, reason = "baseline, zackees/ci.yml#229" )] pub async fn async_main() { - let cli = Cli::parse_from(rewrite_args()); + let cli = parse_cli(rewrite_args()); if cli.fix_win_helper { if let Err(error) = super::windows_long_paths::run_helper() { diff --git a/crates/fbuild-cli/src/daemon_client/identity.rs b/crates/fbuild-cli/src/daemon_client/identity.rs index c7adefc81..23ae252c0 100644 --- a/crates/fbuild-cli/src/daemon_client/identity.rs +++ b/crates/fbuild-cli/src/daemon_client/identity.rs @@ -139,6 +139,7 @@ mod tests { status: "running".to_string(), uptime_seconds: 1.0, version: version.to_string(), + build_id: "test".to_string(), pid: 123, port: 8765, dev_mode: fbuild_paths::is_dev_mode(), diff --git a/crates/fbuild-cli/src/daemon_client/tests.rs b/crates/fbuild-cli/src/daemon_client/tests.rs index 1f39fcb75..34cca79be 100644 --- a/crates/fbuild-cli/src/daemon_client/tests.rs +++ b/crates/fbuild-cli/src/daemon_client/tests.rs @@ -66,6 +66,7 @@ fn daemon_info_for_cache_identity( status: "running".to_string(), uptime_seconds: 1.0, version: "2.2.29".to_string(), + build_id: "test".to_string(), pid: 123, port: 8765, dev_mode: fbuild_paths::is_dev_mode(), diff --git a/crates/fbuild-cli/src/daemon_client/types.rs b/crates/fbuild-cli/src/daemon_client/types.rs index 966efd7ea..a50f0961d 100644 --- a/crates/fbuild-cli/src/daemon_client/types.rs +++ b/crates/fbuild-cli/src/daemon_client/types.rs @@ -233,6 +233,10 @@ pub struct DaemonInfoResponse { pub status: String, pub uptime_seconds: f64, pub version: String, + /// Compile-time build id; absent on daemons that predate + /// FastLED/fbuild#1742. + #[serde(default)] + pub build_id: String, pub pid: u32, pub port: u16, pub dev_mode: bool, diff --git a/crates/fbuild-core/build.rs b/crates/fbuild-core/build.rs new file mode 100644 index 000000000..faca50335 --- /dev/null +++ b/crates/fbuild-core/build.rs @@ -0,0 +1,70 @@ +//! Stamp the workspace build identity into `fbuild-core` (FastLED/fbuild#1742). +//! +//! Two builds of the same workspace version used to be indistinguishable: +//! `fbuild --version` and `fbuild daemon status` both printed `2.5.46`, so an +//! editable install that kept yesterday's binaries — or a daemon left running +//! from an older checkout — looked current, and a missing feature was blamed on +//! fbuild instead of on the stale binary (#1742, and the daemon confusion in +//! #1739). +//! +//! The commit and dirty flag are baked in at compile time and reported by both +//! binaries. `fbuild-core` carries the stamp for the whole workspace: every +//! crate that reports a version depends on it, so a single probe keeps the CLI +//! and the daemon agreeing instead of duplicating git logic per binary. +//! +//! The probe is best-effort by design. A missing `git`, a source tarball with +//! no `.git`, or a repository owned by another user must never fail a build — +//! the stamp degrades to `unknown`, which the CLI prints verbatim. + +use std::process::Command; + +fn main() { + // `cargo:rerun-if-changed` on the ref/index files is what actually makes + // the stamp follow HEAD: the build script reruns when either moves, and a + // changed `rustc-env` value recompiles `fbuild-core` and relinks its + // dependents. `git status` cannot be watched that way — the dirty flag is + // therefore as fresh as the last HEAD/index movement, which is the case + // that matters (a *different build*, not an in-place edit). + for relative in ["../.git/HEAD", "../.git/index"] { + if std::path::Path::new(relative).exists() { + println!("cargo:rerun-if-changed={relative}"); + } + } + println!("cargo:rerun-if-changed=build.rs"); + + let commit = run_git(&["rev-parse", "--short=12", "HEAD"]).unwrap_or_else(unknown); + let dirty = match run_git(&["status", "--porcelain", "--untracked-files=no"]) { + Some(porcelain) => !porcelain.is_empty(), + None => false, + }; + + println!("cargo:rustc-env=FBUILD_BUILD_COMMIT={commit}"); + println!( + "cargo:rustc-env=FBUILD_BUILD_DIRTY={}", + if dirty { "1" } else { "0" } + ); +} + +/// Placeholder used when the checkout cannot be fingerprinted. +fn unknown() -> String { + "unknown".to_string() +} + +/// Run `git -C ` and return trimmed stdout, or `None` when +/// git is absent, the directory is not a repository, or the command fails. +fn run_git(args: &[&str]) -> Option { + let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").ok()?; + // `fbuild-core`'s subprocess wrappers cannot be linked into a build + // script, and this probe never runs at runtime. + // allow-direct-spawn: compile-time git stamp; build scripts link no fbuild crates. + let output = Command::new("git") + .arg("-C") + .arg(&manifest_dir) + .args(args) + .output() + .ok()?; + if !output.status.success() { + return None; + } + Some(String::from_utf8_lossy(&output.stdout).trim().to_string()) +} diff --git a/crates/fbuild-core/src/build_id.rs b/crates/fbuild-core/src/build_id.rs new file mode 100644 index 000000000..b18cc54f2 --- /dev/null +++ b/crates/fbuild-core/src/build_id.rs @@ -0,0 +1,106 @@ +//! Workspace build identity for `fbuild --version` and `fbuild daemon status` +//! (FastLED/fbuild#1742). +//! +//! The values are stamped at compile time by this crate's `build.rs`, so the +//! CLI and every daemon built from the same checkout report the same id even +//! though they are separate binaries. See that script for why the probe lives +//! in `fbuild-core` rather than in each entry point. + +/// The checkout commit, shortened to 12 hex digits, or [`UNKNOWN`]. +pub const COMMIT: &str = match option_env!("FBUILD_BUILD_COMMIT") { + Some(commit) => commit, + None => UNKNOWN, +}; + +/// Whether tracked files differed from `HEAD` when the crate was compiled. +/// +/// A function rather than a `const`: comparing a `&str` in a constant is not +/// allowed (`E0658`), so the comparison happens here instead. +pub fn dirty() -> bool { + matches!(option_env!("FBUILD_BUILD_DIRTY"), Some("1")) +} + +/// Placeholder when no checkout could be fingerprinted (tarball, no git). +pub const UNKNOWN: &str = "unknown"; + +/// The workspace version compiled into this binary. +pub fn version() -> &'static str { + env!("CARGO_PKG_VERSION") +} + +/// This binary's build id: ``, `-dirty`, or `unknown`. +pub fn build_id() -> String { + format_build_id(COMMIT, dirty()) +} + +/// `--version` long form: the workspace version with the build id as semver +/// build metadata (`2.5.46+git.5fca4d49c9ab-dirty`). +/// +/// `metadata` is deliberately semver-legal so `fbuild -V` stays the bare +/// version and any tool that parses the long form can still read the version. +pub fn long_version() -> String { + format_long_version(version(), &build_id()) +} + +fn format_build_id(commit: &str, dirty: bool) -> String { + if commit.is_empty() || commit == UNKNOWN { + return UNKNOWN.to_string(); + } + if dirty { + format!("{commit}-dirty") + } else { + commit.to_string() + } +} + +fn format_long_version(version: &str, build_id: &str) -> String { + format!("{version}+git.{build_id}") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn commit_is_stamped_or_unknown() { + assert!( + COMMIT == UNKNOWN + || (COMMIT.len() <= 12 && COMMIT.chars().all(|c| c.is_ascii_hexdigit())), + "unexpected build commit {COMMIT:?}" + ); + } + + #[test] + fn clean_build_id_is_the_bare_commit() { + assert_eq!(format_build_id("5fca4d49c9ab", false), "5fca4d49c9ab"); + } + + #[test] + fn dirty_build_id_says_so() { + assert_eq!(format_build_id("5fca4d49c9ab", true), "5fca4d49c9ab-dirty"); + } + + #[test] + fn unstamped_builds_report_unknown_even_when_dirty() { + assert_eq!(format_build_id(UNKNOWN, true), UNKNOWN); + assert_eq!(format_build_id("", true), UNKNOWN); + } + + #[test] + fn long_version_keeps_the_workspace_version_readable() { + assert_eq!( + format_long_version("2.5.46", "5fca4d49c9ab"), + "2.5.46+git.5fca4d49c9ab" + ); + assert_eq!( + format_long_version("2.5.46", "unknown"), + "2.5.46+git.unknown" + ); + } + + #[test] + fn this_binary_reports_a_usable_version() { + assert_eq!(version(), env!("CARGO_PKG_VERSION")); + assert!(long_version().starts_with(&format!("{}+git.", version()))); + } +} diff --git a/crates/fbuild-core/src/lib.rs b/crates/fbuild-core/src/lib.rs index ffb276a6e..742c09cd8 100644 --- a/crates/fbuild-core/src/lib.rs +++ b/crates/fbuild-core/src/lib.rs @@ -7,6 +7,7 @@ //! - Size info parsing (avr-size / arm-none-eabi-size output) pub mod archive; +pub mod build_id; pub mod build_log; pub mod channel; pub mod compiler_flags; diff --git a/crates/fbuild-daemon/src/broker/backend.rs b/crates/fbuild-daemon/src/broker/backend.rs index cee7d3f02..82c904408 100644 --- a/crates/fbuild-daemon/src/broker/backend.rs +++ b/crates/fbuild-daemon/src/broker/backend.rs @@ -143,6 +143,7 @@ fn health_response(ctx: &DaemonContext) -> HealthResponse { status: "healthy".to_string(), uptime_seconds: ctx.started_at.elapsed().as_secs_f64(), version: env!("CARGO_PKG_VERSION").to_string(), + build_id: fbuild_core::build_id::build_id(), pid: std::process::id(), source_mtime: ctx.source_mtime, source_exe: ctx.source_exe.clone(), @@ -165,6 +166,7 @@ fn daemon_info_response(ctx: &DaemonContext) -> DaemonInfoResponse { status: "running".to_string(), uptime_seconds: ctx.started_at.elapsed().as_secs_f64(), version: env!("CARGO_PKG_VERSION").to_string(), + build_id: fbuild_core::build_id::build_id(), pid: std::process::id(), port: ctx.port, started_at: ctx.started_at_unix, diff --git a/crates/fbuild-daemon/src/handlers/health.rs b/crates/fbuild-daemon/src/handlers/health.rs index 24b780a82..208dbd17a 100644 --- a/crates/fbuild-daemon/src/handlers/health.rs +++ b/crates/fbuild-daemon/src/handlers/health.rs @@ -28,6 +28,7 @@ pub async fn health_check(State(ctx): State>) -> Json>) -> Json[-dirty]`); absent on daemons that + /// predate FastLED/fbuild#1742. + pub build_id: String, pub pid: u32, pub source_mtime: f64, /// Path of the running daemon binary; lets a restarting CLI name the image @@ -281,6 +284,8 @@ pub struct DaemonInfoResponse { pub status: String, pub uptime_seconds: f64, pub version: String, + /// Compile-time build id (`[-dirty]`, FastLED/fbuild#1742). + pub build_id: String, pub pid: u32, pub port: u16, pub started_at: f64, @@ -928,6 +933,7 @@ mod tests { status: "healthy".into(), uptime_seconds: 42.5, version: "2.0.0".into(), + build_id: "test".into(), pid: 1234, source_mtime: 1700000000.0, source_exe: "/opt/fbuild/fbuild-daemon".into(), @@ -949,6 +955,7 @@ mod tests { status: "running".into(), uptime_seconds: 10.0, version: "2.0.0".into(), + build_id: "test".into(), pid: 5678, port: 8765, started_at: 1700000000.0, @@ -997,6 +1004,7 @@ mod tests { status: "running".into(), uptime_seconds: 10.0, version: "2.0.0".into(), + build_id: "test".into(), pid: 5678, port: 8765, started_at: 1700000000.0, @@ -1027,6 +1035,7 @@ mod tests { status: "running".into(), uptime_seconds: 10.0, version: "2.0.0".into(), + build_id: "test".into(), pid: 5678, port: 8765, started_at: 1700000000.0, From b4abbbaec503a78d35aa1e280298f762063443dc Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Wed, 28 Oct 2026 03:06:48 -0700 Subject: [PATCH 2/2] chore(platform-boundary): refresh the inventory line numbers The nested HealthResponse/DaemonInfoResponse literals grew by one line in handlers/health.rs, and the inventory records line numbers. --- ci/platform_boundary_research.tsv | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ci/platform_boundary_research.tsv b/ci/platform_boundary_research.tsv index 5d77da2ce..15f7232c9 100644 --- a/ci/platform_boundary_research.tsv +++ b/ci/platform_boundary_research.tsv @@ -133,10 +133,10 @@ crates/fbuild-daemon/Cargo.toml 74 native_dependency libc process host_mechanic crates/fbuild-daemon/src/handlers/emulator/tests_npm_cache.rs 224 attr_cfg #[cfg(windows)] host_executable host_artifact_policy crates/fbuild-daemon/src/handlers/emulator/tests_process.rs 9 attr_cfg #[cfg(windows)] host_executable host_artifact_policy crates/fbuild-daemon/src/handlers/emulator/tests_process.rs 21 attr_cfg #[cfg(not(windows))] host_executable host_artifact_policy -crates/fbuild-daemon/src/handlers/health.rs 51 attr_cfg #[cfg(target_os=)] host host_mechanic -crates/fbuild-daemon/src/handlers/health.rs 53 attr_cfg #[cfg(not(target_os=))] host host_mechanic -crates/fbuild-daemon/src/handlers/health.rs 105 attr_cfg #[cfg(target_os=)] host host_mechanic -crates/fbuild-daemon/src/handlers/health.rs 107 attr_cfg #[cfg(not(target_os=))] host host_mechanic +crates/fbuild-daemon/src/handlers/health.rs 52 attr_cfg #[cfg(target_os=)] host host_mechanic +crates/fbuild-daemon/src/handlers/health.rs 54 attr_cfg #[cfg(not(target_os=))] host host_mechanic +crates/fbuild-daemon/src/handlers/health.rs 106 attr_cfg #[cfg(target_os=)] host host_mechanic +crates/fbuild-daemon/src/handlers/health.rs 108 attr_cfg #[cfg(not(target_os=))] host host_mechanic crates/fbuild-daemon/src/handlers/websockets_tests.rs 7 attr_cfg #[cfg(target_os=)] host host_mechanic crates/fbuild-daemon/src/handlers/websockets_tests.rs 20 native_path libc:: process host_mechanic crates/fbuild-daemon/src/handlers/websockets_tests.rs 32 native_path libc:: process host_mechanic