feat(build-time-versioning): version macro/helper functions with unknown fallback

Adds xce_version_field()/xce_compose_version() (utils.h/utils.c) to
compose a version string carrying commit id + build time, rendering
"unknown" for either field when its underlying meson-supplied define is
empty -- ready for controller/scheduler/worker to consume.

MK_VERSION/STR (the pre-existing, compile-time-only macro pair) can't
express this empty-to-"unknown" substitution at the preprocessor level,
so this is a runtime mechanism instead. Round 1 review REJECTED an
earlier version of this task for silently orphaning MK_VERSION/STR
(the task explicitly asked to "extend" them) and for leaving an
undocumented architectural trap: argp_program_version is a file-scope
const char* initialized at compile time, which cannot call a runtime
function -- consuming code must instead compute into a static buffer
inside main() and assign the pointer before argp_parse() runs. Round 2
adds an explicit SUPERSEDED doc comment on MK_VERSION/STR plus NOTE FOR
CALLERS guidance covering both points, and was APPROVED.

Also expands task 3.2's boundary to include controller/testsuite/test.c
(a third MK_VERSION/STR call site discovered during review) so no
caller of the superseded macro survives implementation.

5 new unit tests in test_utils.c (RED->GREEN, TDD).

Task 3.1/13 of build-time-versioning.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Rongsong Shen 2026-08-20 10:16:17 +00:00
parent d08fda76a5
commit 48a86cdab6
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
4 changed files with 159 additions and 7 deletions

View file

@ -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

View file

@ -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);

View file

@ -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);
}

View file

@ -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)