MOD-18221 - use make to decide wether the binaries are up to date without using any toolchain - #1637

Cargo config omitted from rebuild deps

Medium Severity

RUST_SOURCES lists manifests, lockfile, toolchain, *.rs, and *.pest, but not .cargo/config.toml. That file sets rustflags (library search path and musl crt-static). After this change make build skips cargo when those flags change, so the module can keep a stale link.

 

^{Reviewed by Cursor Bugbot for commit 2d7181c. Configure here.}

[CONFIRMED] .cargo/config.toml (which sets real rustflags, e.g. -L deps/readies/wd40/linux-x64) is invisible to this staleness check — only *.rs/*.pest/Cargo.toml/Cargo.lock/rust-toolchain.toml are tracked.

Failure scenario: edit .cargo/config.toml (e.g. the linker -L path or the musl crt-static flag) and run make build. $(TARGET) is still newer than every tracked prerequisite, so make reports "up to date" and never invokes cargo — the stale binary (linked with the old flags) is kept and installed silently. Before this PR, cargo ran unconditionally every time and always re-read this file.

[CONFIRMED] The first-run backdate trusts the current invocation's flags without checking that the pre-existing $(TARGET) was actually built with them.

Failure scenario: a tree was last built with SAN=address (or DEBUG=1) under the pre-PR Makefile, leaving a binary in $(BINDIR) with no stamp file yet. After adopting this PR, the first plain make build (release) computes a "release" stamp, sees no old stamp, sees $(TARGET) exists and NIGHTLY!=1, and backdates the new stamp to match $(TARGET)'s mtime — so make treats the old ASAN/debug binary as fresh and never invokes cargo, silently shipping the wrong-flavored binary as the release artifact.

[CONFIRMED (empirically verified)] Each flag is interpolated inside single-quoted printf arguments with no escaping, so a value containing a single quote breaks the shell command or silently corrupts the recorded stamp.

Verified: make build RUST_FLAGS="-C link-arg=it's-bad" produces /bin/sh: -c: unexpected EOF while looking for matching '\'' and the .rejson-build-flags recipe fails with a shell syntax error. A value with an even number of embedded quotes doesn't error but silently strips the quote characters from the recorded stamp, corrupting the staleness signature without warning.

[CONFIRMED] clean removes $(TARGET) and the flags stamp but not $(TARGET).debug, produced by ./sbin/extract_symbols_safe.sh for non-DEBUG, non-macOS builds (see line 234).

Failure scenario: make build (release) produces rejson.so + rejson.so.debug. make clean && make build DEBUG=1 removes rejson.so and the stamp but leaves the stale rejson.so.debug from the release build sitting next to the fresh debug .so — any packaging/upload step that globs for *.debug alongside $(TARGET) picks up mismatched symbols.

[PLAUSIBLE] The stamp only records the Make variable $(RUST_FLAGS), never the process's actual RUSTFLAGS env var; when $(RUST_FLAGS) is empty the recipe never exports/overrides RUSTFLAGS at all (line 223), so an ambient exported value flows through to cargo uncontrolled and untracked.

Failure scenario: CI exports RUSTFLAGS="-C target-cpu=native" and runs make build (non-DEBUG/non-SAN, so $(RUST_FLAGS) stays empty) — builds fine. It later switches to RUSTFLAGS="-D warnings" with no source edits and reruns make build: the stamp and RUST_SOURCES are byte-identical, so make treats $(TARGET) as fresh and never invokes cargo, silently keeping the binary built with the old flags.

[PLAUSIBLE] $@.tmp is a fixed filename with no PID/mktemp suffix; two concurrent make invocations sharing the same $(BINDIR) can interleave writes to it.

Failure scenario: two make build-family invocations against the same $(BINDIR) run concurrently (parallel CI shards, or a coverage sub-make racing a bench/build in the same tree); process A writes .rejson-build-flags.tmp, process B overwrites it with different flag content before A's cmp/mv completes, so the stamp ends up reflecting neither invocation's actual flags — a subsequent staleness check can wrongly conclude "fresh" for a flag combination that was never actually built.

[CONFIRMED (empirically verified)] RUST_SOURCE_GOALS only recognizes the literal goal names (build/all/default/bench/benchmark/coverage); invoking make with $(TARGET)'s own file path as the goal (now valid, since it's a real file target) doesn't match any of them, so RUST_SOURCES is never computed for that invocation.

Verified with make -p -n: make build gives the $(TARGET) rule ~28 prerequisites (all tracked .rs/.pest/Cargo.toml files); make bin/<variant>/rejson.so (the same target, invoked by its file path) gives it exactly one prerequisite — the flags stamp. A source-only edit followed by that invocation leaves the target considered up to date and cargo never runs.

[PLAUSIBLE] The rustc --print cfg shell-out for RUST_TARGET is gated only by ifeq($(NIGHTLY),1), unlike RUST_SOURCES which is additionally gated by the RUST_SOURCE_GOALS goal list — so it still runs for goals that don't need it.

Failure scenario: on a machine with no rust toolchain on PATH (the exact scenario this PR targets), running make clean NIGHTLY=1 (or make pack NIGHTLY=1, make help NIGHTLY=1) still fails at Makefile-parse time with rustc: command not found, even though clean/pack/help never need RUST_TARGET.

[PLAUSIBLE] build can now fully skip needing cargo/rustc when up to date, but clean (non-ALL=1 path) still unconditionally runs cargo clean after removing $(TARGET)/$(RUST_BUILD_FLAGS).

Failure scenario: in the "root, no toolchain" persona described in this PR's own comment (line 185), make clean still hard-fails on cargo: command not found even though the two lines that matter (rm -f $(TARGET) $(RUST_BUILD_FLAGS)) already succeeded — the recipe aborts non-zero on the trailing cargo clean.

[CONFIRMED (efficiency)] coverage is in RUST_SOURCE_GOALS, so the outer make coverage invocation runs this find even though coverage:'s own recipe (elsewhere in the Makefile) never consumes RUST_SOURCES/$(TARGET) — it only forks $(MAKE) build COV=1, which independently recomputes the same list.

Every make coverage CI run pays for the tree-walk twice (once in the outer process, once in the inner build sub-make) for no benefit — wasted work that scales with repo size, though currently only dozens of files so the absolute cost is small.

[CONFIRMED (simplification)] RUST_SOURCE_GOALS lists the literal word default alongside a separate $(if $(MAKECMDGOALS),,default) sentinel on line 192 that handles bare make — the two look like a duplicate/leftover, but default is actually a real, matchable goal name (default: build in readies' mk/rules), and removing it from the list was verified (via a throwaway test Makefile) to break tracking specifically for make default.

A future contributor could easily read the $(if ...) sentinel, assume the default list member is now redundant dead code, and delete it as cleanup — silently breaking make default's source tracking with no error. Worth a comment explaining both default occurrences are independently load-bearing.

[PLAUSIBLE (altitude)] Rather than solving "root/sudo has no cargo on PATH" by splitting a normal-user build from a separate root-only install step that never touches cargo, this teaches the same build target to approximate cargo's own per-crate fingerprinting at the Make level via a hand-picked file glob plus a hand-rolled flags stamp.

Every future cargo rebuild trigger this stamp doesn't enumerate (a new profile setting, a build.rs, a new env!-read var, a .cargo/config.toml key) is invisible to this approximation and will only get more wrong over time as cargo's own fingerprinting surface grows without a corresponding update here — see the concrete .cargo/config.toml and ambient-RUSTFLAGS comments on this PR.

[CONFIRMED (simplification, minor)] cmp -s $@.tmp $@ already evaluates false when $@ doesn't exist, so the [ $$had_stamp = 1 ] && guard on the first branch is redundant; had_stamp is only actually needed for the backdate decision in the else branch.

Not a bug, just a readability cost — a reader has to verify the redundant had_stamp=1 guard on the cmp branch is safe (the file-existence check inside cmp already handles that case) before trusting that dropping it and keeping had_stamp only for the touch -r decision is equivalent.

[PLAUSIBLE] stderr from find is discarded (2>/dev/null), so if json_path/ or redis_json/ is momentarily missing/unreadable (partial checkout, submodule not yet materialized), RUST_SOURCES silently shrinks instead of the build erroring loudly.

Failure scenario: a CI/packaging flow that lays down the Makefile before the source tree is fully synced runs make build once; RUST_SOURCES is computed near-empty for that invocation (only Cargo.toml/Cargo.lock/rust-toolchain.toml remain). A long-lived $(MAKE) process that cached this impoverished variable can then miss later edits to the now-present .rs files.