diff --git a/.kiro/specs/build-time-versioning/tasks.md b/.kiro/specs/build-time-versioning/tasks.md index 1384eb19..971435cb 100644 --- a/.kiro/specs/build-time-versioning/tasks.md +++ b/.kiro/specs/build-time-versioning/tasks.md @@ -25,19 +25,20 @@ - _Boundary: Nix packaging_ - [ ] 3. Core: C daemon version display -- [ ] 3.1 Extend the shared version macro with build time and unknown fallback - - Extend `MK_VERSION`/`STR` in `src/include/utils.h` so the composed string carries both commit id and build time - - Both fields render as `unknown` when their underlying define is empty, never a blank/garbled string - - Observable: a build with both meson options unset produces a `--version` string containing `unknown` for both fields, not empty gaps +- [x] 3.1 Extend the shared version macro with build time and unknown fallback + - ~~Extend `MK_VERSION`/`STR`~~ — `MK_VERSION`/`STR` cannot express the empty-string-to-`unknown` substitution at the preprocessor level, so implemented as new runtime functions `xce_version_field()`/`xce_compose_version()` (`utils.h`/`utils.c`) instead; `MK_VERSION`/`STR` marked SUPERSEDED with an explicit doc comment and handoff notes for 3.2 (round 1 REJECTED for silently orphaning `MK_VERSION` + undocumented `argp_program_version` file-scope-const-initializer trap; round 2 APPROVED after adding the SUPERSEDED comment + NOTE FOR CALLERS guidance) + - Both fields render as `unknown` when their underlying define is empty (verified via `""`, not just `NULL`), never a blank/garbled string + - Observable: a build with both meson options unset produces a `--version` string containing `unknown` for both fields, not empty gaps -- 5 new unit tests in `test_utils.c` confirm this - _Requirements: 1.4, 2.2_ - _Boundary: utils.h_ - [ ] 3.2 (P) Controller and scheduler: version in --version and startup log - - Update each daemon's `argp_program_version` construction to use the extended macro from 3.1 + - Update each daemon's `argp_program_version` construction to use `xce_compose_version()`/`xce_version_field()` from 3.1 (NOT the now-superseded `MK_VERSION`/`STR`) -- since `argp_program_version` is a file-scope `const char*` global (mutable pointer, not `const char* const`) currently initialized at compile time, this requires computing the composed string into a static buffer inside `main()` and assigning the pointer there, before `argp_parse()` runs, rather than a one-line macro swap - Add the version string to each daemon's existing first startup INFO log line (controller's `"My name is %s"` line, scheduler's `"HA cluster config..."` line) rather than a second, independently-formatted line - - Observable: `xce_controller --version` and `xce_scheduler --version` each print commit id + build time and exit without starting the daemon; each daemon's startup log's first lines include the identical two values + - `src/controller/testsuite/test.c:662` is a THIRD `MK_VERSION`/`STR` call site (the `xce_test` REPL driver's own version output) discovered during 3.1's review, outside this sub-task's original scope -- migrate it too in this same task (same file family, same daemon, same minimal fix shape), so no caller of the superseded macro remains after this task lands + - Observable: `xce_controller --version` and `xce_scheduler --version` each print commit id + build time and exit without starting the daemon; each daemon's startup log's first lines include the identical two values; `xce_test --version` (or equivalent) also reflects the new composed string, not the old commit-id-only one - _Requirements: 1.1, 1.2, 2.1, 2.2, 3.1, 3.2, 4.3_ - - _Boundary: controller/main.c, scheduler/main.c_ + - _Boundary: controller/main.c, scheduler/main.c, controller/testsuite/test.c_ - _Depends: 3.1_ - [ ] 3.3 (P) Worker: add --version and startup log line diff --git a/src/include/utils.h b/src/include/utils.h index a068ca09..12a29ea2 100644 --- a/src/include/utils.h +++ b/src/include/utils.h @@ -61,6 +61,55 @@ #define MK_VERSION(ver, commit) ver commit +/* SUPERSEDED for version-string composition (build-time-versioning task + * 3.1). MK_VERSION/STR (immediately above) cannot express an + * empty-value -> "unknown" fallback: an unset commit_id/build_time meson + * option makes STR(x) expand to an empty string literal ("", not a missing + * token), and a preprocessor macro has no way to branch on "is this string + * empty" at expansion time. xce_version_field()/xce_compose_version() + * (extern decls below, implementation + doc comments in utils.c) are the + * runtime replacement — they DO perform that substitution, and are what + * compose the version string everywhere it now matters. Use those, not + * MK_VERSION, for any new version-string composition. + * + * MK_VERSION/STR are kept in place here (not deleted) only because task 3.2 + * — which will migrate controller/main.c's and scheduler/src/main.c's + * argp_program_version construction to xce_compose_version() — has not + * landed yet. Grep finding (2026-08-20, `grep -rn 'STR(\|MK_VERSION(' src/`, + * excluding NULLSTR/comment/definition matches): STR() has ZERO call sites + * anywhere in this codebase outside this exact MK_VERSION-based + * argp_program_version pattern — controller/main.c:23, + * scheduler/src/main.c:26, AND controller/testsuite/test.c:662 (the + * xce_test REPL tool). That third caller is NOT listed in task 3.2's + * boundary (`controller/main.c, scheduler/main.c` only, per tasks.md) — + * worth confirming whether it's in scope when 3.2 lands, or it will keep + * printing a commit-id-only, build-time-less version string after the + * other two are migrated. STR() itself is a generic single-argument + * stringification macro (not inherently version-specific), but per this + * grep it currently has no unrelated use, so it is not being kept around + * for any other live caller — only in service of the not-yet-migrated + * MK_VERSION call sites above. Once task 3.2 (and any update to + * controller/testsuite/test.c) lands and no caller of MK_VERSION remains, + * both MK_VERSION and this STR usage can be deleted outright — not done + * here, out of this task's boundary. + * + * NOTE FOR TASK 3.2 (or whoever migrates argp_program_version): + * xce_compose_version() is a REGULAR RUNTIME FUNCTION, not a constant + * expression — unlike MK_VERSION's pure string-literal concatenation, it + * CANNOT initialize a file-scope variable. `const char* argp_program_version + * = xce_compose_version(...)` at file scope will not compile (C file-scope + * initializers must be constant expressions). controller/main.c:22-23 and + * scheduler/src/main.c:25-26 currently declare argp_program_version exactly + * that way — a file-scope const initialized at compile time — so this is + * not a one-line macro-call swap. Instead: compute the version string at + * RUNTIME into a `static char buf[...]` (module scope or local to main()), + * via xce_compose_version(), early in main() and BEFORE argp_parse() runs + * (argp reads argp_program_version by pointer when --version/-V is + * processed) — then assign argp_program_version (no longer const-qualified + * at its declaration; assigned at runtime instead of initialized) to point + * at that buffer. See xce_compose_version()'s doc comment in utils.c for + * the buffer-sizing contract. */ + #ifndef FREEUP #define FREEUP(p) \ if ((p) != NULL) { \ @@ -114,6 +163,13 @@ extern int mylog_init(char*, char*, int, int); extern void mylog_fini(void); extern int mylog_setlevel(char*); extern char* verbose_to_loglevel(int); +/* See the SUPERSEDED/NOTE comment above MK_VERSION for what these replace + * and why, and the constraint they impose on any file-scope + * argp_program_version-style global (xce_compose_version() is not a + * constant expression). */ +extern const char* xce_version_field(const char*); +extern int xce_compose_version(char*, size_t, const char*, const char*, + const char*); extern void set_error(char*, char*, int, int); extern ectx_t* get_ectx(void); extern int xce_vprintf(const char*, va_list); diff --git a/src/lib/testsuite/test_utils.c b/src/lib/testsuite/test_utils.c index 316e1832..2ed3c029 100644 --- a/src/lib/testsuite/test_utils.c +++ b/src/lib/testsuite/test_utils.c @@ -140,6 +140,55 @@ void test_parse_jobid2(void) do_parse_jobid_test(jobid); } +/* build-time-versioning task 3.1: xce_version_field()'s empty/NULL -> + * "unknown" fallback, used to keep the composed --version string free of + * blank gaps when VERSION_COMMIT_ID/VERSION_BUILD_TIME are unset. */ +void test_version_field_unknown_fallback(void) +{ + TEST_ASSERT_EQUAL_STRING("unknown", xce_version_field(NULL)); + TEST_ASSERT_EQUAL_STRING("unknown", xce_version_field("")); +} + +void test_version_field_passthrough(void) +{ + TEST_ASSERT_EQUAL_STRING("a1b2c3d", xce_version_field("a1b2c3d")); + TEST_ASSERT_EQUAL_STRING("2026-08-20T10:00:00Z", + xce_version_field("2026-08-20T10:00:00Z")); +} + +void test_compose_version_both_set(void) +{ + char buf[128]; + int n; + + n = xce_compose_version(buf, sizeof(buf), "xce v0.0.1-", "a1b2c3d", + "2026-08-20T10:00:00Z"); + + TEST_ASSERT_TRUE(n > 0); + TEST_ASSERT_EQUAL_STRING("xce v0.0.1-a1b2c3d built 2026-08-20T10:00:00Z", + buf); +} + +void test_compose_version_both_unset(void) +{ + char buf[128]; + + xce_compose_version(buf, sizeof(buf), "xce v0.0.1-", "", ""); + + /* both fields must render as the literal word "unknown", never a + * blank/garbled gap (e.g. NOT "xce v0.0.1- built "). */ + TEST_ASSERT_EQUAL_STRING("xce v0.0.1-unknown built unknown", buf); +} + +void test_compose_version_null_fields(void) +{ + char buf[128]; + + xce_compose_version(buf, sizeof(buf), "xce v0.0.1-", NULL, NULL); + + TEST_ASSERT_EQUAL_STRING("xce v0.0.1-unknown built unknown", buf); +} + void test_utils(void) { printf("\n>> utils test\n\n"); @@ -150,4 +199,9 @@ void test_utils(void) RUN_TEST(test_extract_bits); RUN_TEST(test_parse_jobid1); RUN_TEST(test_parse_jobid2); + RUN_TEST(test_version_field_unknown_fallback); + RUN_TEST(test_version_field_passthrough); + RUN_TEST(test_compose_version_both_set); + RUN_TEST(test_compose_version_both_unset); + RUN_TEST(test_compose_version_null_fields); } diff --git a/src/lib/utils.c b/src/lib/utils.c index 4b18e3aa..690710e0 100644 --- a/src/lib/utils.c +++ b/src/lib/utils.c @@ -296,6 +296,47 @@ int mylog_setlevel(char* level) { return tlog_setlevel(name_to_level(level)); } void mylog_fini() { tlog_exit(); } +/* build-time-versioning task 3.1: see the SUPERSEDED comment above + * MK_VERSION in utils.h. raw is typically + * STR(VERSION_COMMIT_ID)/STR(VERSION_BUILD_TIME), which is "" (never a + * missing/NULL token) when the corresponding meson option was left unset + * -- NULL is also accepted defensively for any other caller. */ +const char* xce_version_field(const char* raw) +{ + if (raw == NULL || raw[0] == '\0') { + return "unknown"; + } + + return raw; +} + +/* NOTE FOR CALLERS with a file-scope `argp_program_version`-style global + * (controller/main.c, scheduler/src/main.c both currently have one, built + * today via the now-superseded MK_VERSION macro -- see utils.h): this + * function is a regular runtime call, NOT a constant expression, so its + * result cannot initialize a file-scope variable + * (`const char* argp_program_version = xce_compose_version(...);` at file + * scope will not compile). Compute the version string into a + * `static char buf[...]` (sized generously enough for prefix + commit + + * " built " + timestamp, e.g. 128 bytes) inside main() -- or any function + * that runs before argp_parse() is called, since argp reads + * argp_program_version by pointer when --version/-V is processed -- then + * point the (non-const-initialized, runtime-assigned) global at that + * buffer. buf must outlive the call (static or otherwise long-lived + * storage, never a stack buffer that goes out of scope before argp_parse() + * runs). */ +int xce_compose_version(char* buf, size_t bufsz, const char* prefix, + const char* commit, const char* build_time) +{ + if (buf == NULL || bufsz == 0) { + return -1; + } + + return snprintf(buf, bufsz, "%s%s built %s", + (prefix != NULL) ? prefix : "", + xce_version_field(commit), xce_version_field(build_time)); +} + static _Thread_local ectx_t ectx; void set_error(char* file, char* func, int line, int code)