diff --git a/.kiro/specs/build-time-versioning/design.md b/.kiro/specs/build-time-versioning/design.md index d45682af..2f4194b0 100644 --- a/.kiro/specs/build-time-versioning/design.md +++ b/.kiro/specs/build-time-versioning/design.md @@ -60,7 +60,7 @@ Two independent **computation sites** (respecting the existing sandboxed-vs-ambi ```mermaid graph TB DevScript[xce-build.sh git plus date] - NixPkg[xce.nix xcecli.nix self.shortRev plus builtins.currentTime] + NixPkg[xce.nix xcecli.nix self.shortRev plus preConfigure date] MesonOpts[meson commit_id and build_time options] CDefine[C VERSION_COMMIT_ID and VERSION_BUILD_TIME defines] GoLdflags[go_build ldflags -X injection] @@ -102,7 +102,7 @@ graph TB | Go CLI (xcecli) | Cobra (existing) | `rootCmd.Version` field (built-in `--version`) | Zero new parsing code | | Python build | Meson `custom_target` (existing) | New generated build-info module written before `pip install` | `meson-build.sh` | | Python CLI (xceadmin) | Click (existing) | `click.version_option()` (built-in `--version`) | Zero new parsing code | -| Nix packaging | Nix flake (existing) | `self.shortRev`/`self.dirtyShortRev` (existing) + new `builtins.currentTime`-derived value | `xce.nix`, `xcecli.nix` | +| Nix packaging | Nix flake (existing) | `self.shortRev`/`self.dirtyShortRev` (existing, eval-time) + a `preConfigure` build-phase `date -u` hook for build_time (task 2.2 round 2: `builtins.currentTime` was tried first but breaks pure-eval `nix build`, i.e. requires `--impure`, and yields non-UTC epoch seconds — rejected in review, corrected to a build-phase computation) | `xce.nix`, `xcecli.nix` | ## File Structure Plan @@ -113,7 +113,7 @@ graph TB - `src/controller/main.c`, `src/scheduler/src/main.c` — update `argp_program_version` construction to include build time; add the startup INFO log line (each already has one startup INFO call site to extend/follow). - `src/worker/src/main.c` — add a `--version` long option + case (prints the same shape, exits 0); add the startup INFO log line (follows its own existing startup INFO call site). - `src/tools/xce-build.sh` — compute `commit_id` (git, `-dirty` suffixed) and `build_time` (UTC `date`) and pass both via new `-Dcommit_id=`/`-Dbuild_time=` entries in `MESON_OPTS`, falling back to `"unknown"` if `git` fails (e.g. not a git checkout). -- `src/flake.nix`, `src/nixSupport/xce.nix`, `src/nixSupport/xcecli.nix` (exact filenames per existing commit_id wiring) — add a `builtins.currentTime`-derived build-time value passed the same way `commit_id` already is. +- `src/nixSupport/xce.nix`, `src/nixSupport/xcecli.nix` — a `preConfigure` build-phase hook runs `date -u +%Y-%m-%dT%H:%M:%SZ` and appends `-Dbuild_time=` to `mesonFlagsArray` (build-time, not eval-time, unlike `commit_id`'s `self.shortRev` wiring in `src/flake.nix`/`src/nixSupport/default.nix`, which is unchanged). - `src/tools/go_build` — read `XCE_GIT_COMMIT`/`XCE_BUILD_TIME` env vars (empty-safe) and append `-ldflags "-X main.GitCommit=... -X main.BuildTime=..."` to the final `go build` invocation. - `src/frontend/apiserver/meson.build`, `src/frontend/xcecli/meson.build` — pass the two new env vars into their `custom_target()` calls (mirrors existing env var passing). - `src/frontend/Makefile` — add the same `-ldflags` construction (via `git`/`date` directly, dev-loop path) to `build-server`/`build-cli` targets. @@ -140,7 +140,7 @@ graph TB | 3.1, 3.2 | Startup log line at INFO | controller/scheduler/worker/apiserver startup sequences | existing first-INFO-line call sites, extended | Startup → log | | 4.1, 4.2 | Build tooling confirms embedding | `xce-build.sh`, nix packages | non-empty resolved value check before/after `meson setup`/build | Build → verify | | 4.3 | Accurate under every C build profile | `src/meson.build`'s project-wide `add_project_arguments()` | applies uniformly to asan/debug/release | n/a | -| 5.1, 5.2 | Values reflect actual tree/build event | `xce-build.sh` (git+date at invocation time), nix (`self.shortRev`+`builtins.currentTime` at evaluation time) | no hardcoded/stale values | n/a | +| 5.1, 5.2 | Values reflect actual tree/build event | `xce-build.sh` (git+date at invocation time), nix (`self.shortRev` at evaluation time + `preConfigure` `date -u` at build time) | no hardcoded/stale values | n/a | ## Components and Interfaces diff --git a/.kiro/specs/build-time-versioning/research.md b/.kiro/specs/build-time-versioning/research.md index 497f626f..6f0f880d 100644 --- a/.kiro/specs/build-time-versioning/research.md +++ b/.kiro/specs/build-time-versioning/research.md @@ -33,10 +33,11 @@ - **Alternatives Considered**: 1. Make `xce-build.sh` also avoid shelling out to git, deriving everything from Nix like the packages do — rejected, `xce-build.sh` doesn't invoke Nix package builds at all, it drives `meson` directly inside `nix develop`. 2. Add a git-invoking `run_command()` inside `meson.build` itself, used by both paths — rejected, breaks the sandboxed `nix build` path where git is unavailable by design. -- **Selected Approach**: extend the existing split — `xce-build.sh` computes via `git`+`date` and passes `-Dcommit_id=`/`-Dbuild_time=`; the nix packages (`xce.nix`, `xcecli.nix`) compute via `self.shortRev`/`self.dirtyShortRev` (unchanged) + a new `builtins.currentTime`-derived value, passed the same way. +- **Selected Approach**: extend the existing split — `xce-build.sh` computes via `git`+`date` and passes `-Dcommit_id=`/`-Dbuild_time=`; the nix packages (`xce.nix`, `xcecli.nix`) compute `commit_id` via `self.shortRev`/`self.dirtyShortRev` (unchanged, eval-time) and `build_time` via a `preConfigure` build-phase `date -u` hook appending to `mesonFlagsArray` (build-time, not eval-time), passed the same `-Dbuild_time=` way. - **Rationale**: matches the codebase's own existing precedent exactly; zero new dependencies; keeps the sandboxed build's reproducibility model intact. - **Trade-offs**: two computation sites to keep in sync (same risk the existing `commit_id`-only mechanism already carries and has lived with). - **Follow-up**: verify during implementation that a plain `meson setup` with neither `xce-build.sh` nor a nix package (e.g. `meson setup build` run by hand) degrades to the "unknown" fallback (Requirement 1 AC4), not a build failure. +- **Correction (task 2.2, round 1 → round 2)**: the nix-side value was originally attempted via `builtins.currentTime` (eval-time) as noted above — this broke Nix's flake pure-eval mode entirely (`nix build .#xcecore_debug`, the documented no-flag command, failed with `error: attribute 'currentTime' missing`, requiring `--impure`) and produced a raw epoch-seconds string rather than UTC ISO-8601. Caught by task-local review (REJECTED, round 1) before merge; corrected to the `preConfigure`/`date -u` build-phase approach described in "Selected Approach" above, verified to restore pure-eval and produce a correctly-formatted value (round 2, APPROVED). Recorded here so this decision's rationale stays accurate for anyone reading only `research.md`. ### Decision: Worker gets a hand-rolled `--version` case, not a switch to argp - **Context**: worker uses `getopt_long()`, not `argp`; controller/scheduler's free `--version` comes from argp's own auto-generated handling. diff --git a/.kiro/specs/build-time-versioning/tasks.md b/.kiro/specs/build-time-versioning/tasks.md index 8a17051f..1384eb19 100644 --- a/.kiro/specs/build-time-versioning/tasks.md +++ b/.kiro/specs/build-time-versioning/tasks.md @@ -17,10 +17,10 @@ - _Requirements: 1.1, 1.2, 1.3, 1.4, 4.1, 4.2, 5.1, 5.2_ - _Boundary: xce-build.sh_ -- [ ] 2.2 (P) Nix packages compute and pass build time - - Add a `builtins.currentTime`-derived build-time value in `src/flake.nix`, passed via a new `-Dbuild_time=` entry alongside the existing `-Dcommit_id=` wiring in `xce.nix`/`xcecli.nix` - - Since `builtins.currentTime` cannot itself be empty, the confirmation this requirement asks for is structural here: assert (in the derivation or a post-build check) that the meson option actually reached the binary, rather than a runtime empty-value check - - Observable: `nix build .#xcecore_debug` produces a binary whose `--version` shows a non-empty build time, without any `git` invocation inside the sandboxed derivation +- [x] 2.2 (P) Nix packages compute and pass build time + - ~~Add a `builtins.currentTime`-derived build-time value in `src/flake.nix`~~ — corrected during review (round 1 REJECTED): `builtins.currentTime` breaks pure-eval `nix build` (requires `--impure`) and yields non-UTC epoch seconds. Implemented instead as a `preConfigure` build-phase `date -u +%Y-%m-%dT%H:%M:%SZ` hook in `xce.nix`/`xcecli.nix` appending to `mesonFlagsArray`, alongside the unchanged eval-time `-Dcommit_id=` wiring + - Fail loudly (non-empty guard) if `date -u` ever produces empty output, before the value reaches `mesonFlagsArray` + - Observable: `nix build .#xcecore_debug` produces a binary whose `--version` shows a non-empty, ISO-8601 UTC build time, without any `git` invocation inside the sandboxed derivation, and without requiring `--impure` - _Requirements: 1.1, 1.2, 1.4, 4.1, 4.2, 5.1, 5.2_ - _Boundary: Nix packaging_ @@ -122,3 +122,6 @@ - Observable: the installed wheel's `--version` output differs from an editable (`pip install -e .`) install's `unknown`-only output, proving the generated module actually replaces the fallback - _Requirements: 1.1, 1.2, 2.1, 4.1, 4.2_ - _Depends: 5.1, 5.2_ + +## Implementation Notes +- **Task 2.2**: `builtins.currentTime` cannot be used for anything feeding a `nix build` derivation without `--impure` — Nix's flake pure-eval mode strips it entirely, breaking the plain, documented `nix build .#xcecore`/`.#xcecore_debug` commands. It also yields a raw epoch-seconds integer, not UTC ISO-8601. Any nix-side build metadata that must vary per-build (timestamps, etc.) belongs in a derivation's `preConfigure` (or similar) build-phase shell hook using ordinary coreutils (`date -u`), appended to `mesonFlagsArray` — never in eval-time Nix expressions. `self.shortRev`/`self.dirtyShortRev` remain the correct, eval-time-safe mechanism specifically for the commit id (Nix computes those specially even under pure-eval), so this distinction is per-value, not a blanket "avoid nix expressions" rule. diff --git a/src/flake.nix b/src/flake.nix index 198ed8e7..836d0cb7 100644 --- a/src/flake.nix +++ b/src/flake.nix @@ -16,6 +16,19 @@ if builtins.hasAttr "shortRev" self then self.shortRev else self.dirtyShortRev; + # NOTE (build-time-versioning task 2.2, remediation round): unlike + # myCommitId (a genuinely flake-level, eval-time concept sourced from + # self.shortRev/self.dirtyShortRev), the build timestamp is NOT + # computed here. An earlier attempt used `builtins.toString + # builtins.currentTime` at this exact spot -- but builtins.currentTime + # is stripped entirely under a flake's default pure evaluation + # ("error: attribute 'currentTime' missing"), so `nix build + # .#xcecore_debug` (no --impure, the documented canonical command) + # failed at evaluation. Fixed by moving the build-time computation + # into each derivation's own build phase instead (xce.nix/xcecli.nix + # `preConfigure`, via stdenv's bundled `date -u`) -- see those files. + # Nothing build-time-related needs to be threaded through the flake + # or nixSupport/default.nix any more; only myCommitId still does. in flake-utils.lib.eachSystem supportedSystems ( system: let diff --git a/src/nixSupport/default.nix b/src/nixSupport/default.nix index 5d3dd4d9..d4a65bdf 100644 --- a/src/nixSupport/default.nix +++ b/src/nixSupport/default.nix @@ -4,6 +4,12 @@ ... }: let + # NOTE (build-time-versioning task 2.2, remediation round): build-time is + # no longer threaded through here -- it's computed inside each + # derivation's own build phase (xce.nix/xcecli.nix `preConfigure`), since + # builtins.currentTime is unavailable under flake pure-eval. Only + # myCommitId (a genuine eval-time/flake-level value) still needs to be + # passed down. xcecore = pkgs.callPackage ./xce.nix { inherit pkgs myCommitId; withDebug = false; diff --git a/src/nixSupport/xce.nix b/src/nixSupport/xce.nix index 4f1b16d5..c2d87749 100644 --- a/src/nixSupport/xce.nix +++ b/src/nixSupport/xce.nix @@ -103,6 +103,27 @@ stdenv.mkDerivation { impureEnvVars = [ "HOME" ]; + # build-time-versioning task 2.2 (remediation round): build_time is + # computed HERE, in the build phase, not at flake-evaluation time -- + # builtins.currentTime is unavailable under flake pure-eval (see + # flake.nix's note), so `nix build .#xcecore_debug` with no --impure + # would fail at evaluation if it were used. `date` (stdenv's bundled + # coreutils) is always available inside the build sandbox regardless of + # pure-eval -- no --impure, no git, no network needed. Matches + # xce-build.sh's (task 2.1) exact ISO-8601 format. mesonConfigurePhase + # (nixpkgs meson setup-hook.sh) runs `runHook preConfigure` before it + # reads mesonFlags/mesonFlagsArray to build the `meson setup` command + # line, so appending to mesonFlagsArray here is picked up at configure + # time. + preConfigure = '' + buildTime="$(date -u +%Y-%m-%dT%H:%M:%SZ)" + if [ -z "$buildTime" ]; then + echo "xce.nix: date -u produced an empty build time -- the -Dbuild_time= meson option would not reach the binary" >&2 + exit 1 + fi + mesonFlagsArray+=("-Dbuild_time=$buildTime") + ''; + mesonFlags =[ (lib.optional withDebug "--buildtype=debug") (lib.optional withAsan "-Db_sanitize=address") diff --git a/src/nixSupport/xcecli.nix b/src/nixSupport/xcecli.nix index 981b3f2f..0e7256b3 100644 --- a/src/nixSupport/xcecli.nix +++ b/src/nixSupport/xcecli.nix @@ -25,6 +25,17 @@ stdenv.mkDerivation { impureEnvVars = [ "HOME" ]; + # See xce.nix for why build_time is computed here (build phase) rather + # than at flake-evaluation time. + preConfigure = '' + buildTime="$(date -u +%Y-%m-%dT%H:%M:%SZ)" + if [ -z "$buildTime" ]; then + echo "xcecli.nix: date -u produced an empty build time -- the -Dbuild_time= meson option would not reach the binary" >&2 + exit 1 + fi + mesonFlagsArray+=("-Dbuild_time=$buildTime") + ''; + mesonFlags = [ "-Dcomponent=xcecli" ("-Dcommit_id=" + myCommitId)