build.rs does not track native/metal headers; stale object file can be linked after a header change #15

New Issue

hugo · 2026-07-25T09:42:00Z

hugo commented

2026-07-25 09:42:00 +00:00

Symptom

build.rs declares a rebuild dependency on native/metal/ds4_metal.m only. Editing any of the headers it compiles against does not trigger a rebuild, so cargo happily links a stale object file against changed declarations. The result is a silently mismatched ABI — the worst class of FFI bug, because it produces wrong numbers or a crash with no compiler diagnostic pointing at the cause.

This matters more than usual here because the native sources are a vendored mirror of ../ds4, re-synced by hand.

Evidence

build.rs:8-15:

let metal = Path::new("native/metal/ds4_metal.m");
println!("cargo:rerun-if-changed={}", metal.display());
cc::Build::new()
    .include("native/metal")
    .file(metal)
    .flag("-fobjc-arc")
    .opt_level(3)
    .compile("ds4_metal");

native/metal/ contains three headers that are on the include path and none of them are tracked: ds4.h, ds4_gpu.h, ds4_ssd.h.

ds4_gpu.h is the one that matters — it declares every function in the unsafe extern "C" block at src/engine/metal/gpu.rs:8-773. Change a parameter there without a rebuild and the Rust and ObjC sides disagree about the call frame.

The .metal kernel sources under metal/ are not affected: they are loaded at runtime via the env vars set in src/engine/metal.rs:51-73, not compiled by build.rs.

How ds4 handles this

../ds4/Makefile lists header prerequisites explicitly for every object, including the Metal one:

Makefile:243 — ds4_metal.o: ds4_metal.m ds4_gpu.h $(METAL_SRCS)
and the same discipline throughout, e.g. Makefile:168 ds4.o: ds4.c ds4.h ds4_ssd.h ds4_distributed.h ds4_gpu.h, Makefile:189 ds4_server.o: ds4_server.c ds4.h ds4_ssd.h ds4_distributed.h ds4_help.h ds4_kvstore.h rax.h

Note ds4 tracks $(METAL_SRCS) too, because its build embeds the kernels; DS4Server ships them as bundle resources instead, so only the headers are missing here.

Vendoring context that raises the stakes — the native layer is a near-verbatim copy of ds4's:

native/metal/ds4_gpu.h is byte-identical to ../ds4/ds4_gpu.h.
native/metal/ds4_metal.m differs from ../ds4/ds4_metal.m in exactly 3 lines, all comments ("Objective-C Metal glue for the Rust engine" vs "the C engine").
All 19 metal/*.metal kernels are byte-identical to ../ds4/metal/*.metal.

So the expected workflow is "pull upstream ds4 changes into native/metal/" — which is precisely the workflow that will drop a new ds4_gpu.h into the tree and get a stale libds4_metal.a linked against it.

Suggested fix

One line, plus optional tightening:

println!("cargo:rerun-if-changed=native/metal");

Directory-level rerun-if-changed covers the .m and all three headers, and survives new files being added. (Cargo recurses into directories for this directive.) Keep or drop the existing per-file line as you prefer — the directory one subsumes it.

While in here, consider adding a short comment recording that native/metal/ is a vendored mirror of ../ds4 and which files are expected to stay byte-identical upstream. That is the missing half of the sync workflow.

Acceptance criteria

touch native/metal/ds4_gpu.h && cargo build recompiles ds4_metal.m. (Verify with cargo build -v or by checking the object file's mtime.)
Same for ds4.h and ds4_ssd.h.
make bundle still succeeds.

## Symptom `build.rs` declares a rebuild dependency on `native/metal/ds4_metal.m` only. Editing any of the headers it compiles against does not trigger a rebuild, so cargo happily links a stale object file against changed declarations. The result is a silently mismatched ABI — the worst class of FFI bug, because it produces wrong numbers or a crash with no compiler diagnostic pointing at the cause. This matters more than usual here because the native sources are a **vendored mirror** of `../ds4`, re-synced by hand. ## Evidence `build.rs:8-15`: ```rust let metal = Path::new("native/metal/ds4_metal.m"); println!("cargo:rerun-if-changed={}", metal.display()); cc::Build::new() .include("native/metal") .file(metal) .flag("-fobjc-arc") .opt_level(3) .compile("ds4_metal"); ``` `native/metal/` contains three headers that are on the include path and none of them are tracked: `ds4.h`, `ds4_gpu.h`, `ds4_ssd.h`. `ds4_gpu.h` is the one that matters — it declares every function in the `unsafe extern "C"` block at `src/engine/metal/gpu.rs:8-773`. Change a parameter there without a rebuild and the Rust and ObjC sides disagree about the call frame. The `.metal` kernel sources under `metal/` are *not* affected: they are loaded at runtime via the env vars set in `src/engine/metal.rs:51-73`, not compiled by `build.rs`. ## How ds4 handles this `../ds4/Makefile` lists header prerequisites explicitly for every object, including the Metal one: - `Makefile:243` — `ds4_metal.o: ds4_metal.m ds4_gpu.h $(METAL_SRCS)` - and the same discipline throughout, e.g. `Makefile:168` `ds4.o: ds4.c ds4.h ds4_ssd.h ds4_distributed.h ds4_gpu.h`, `Makefile:189` `ds4_server.o: ds4_server.c ds4.h ds4_ssd.h ds4_distributed.h ds4_help.h ds4_kvstore.h rax.h` Note ds4 tracks `$(METAL_SRCS)` too, because its build embeds the kernels; DS4Server ships them as bundle resources instead, so only the headers are missing here. **Vendoring context that raises the stakes** — the native layer is a near-verbatim copy of ds4's: - `native/metal/ds4_gpu.h` is **byte-identical** to `../ds4/ds4_gpu.h`. - `native/metal/ds4_metal.m` differs from `../ds4/ds4_metal.m` in exactly 3 lines, all comments ("Objective-C Metal glue for the **Rust** engine" vs "the **C** engine"). - All 19 `metal/*.metal` kernels are byte-identical to `../ds4/metal/*.metal`. So the expected workflow is "pull upstream ds4 changes into `native/metal/`" — which is precisely the workflow that will drop a new `ds4_gpu.h` into the tree and get a stale `libds4_metal.a` linked against it. ## Suggested fix One line, plus optional tightening: ```rust println!("cargo:rerun-if-changed=native/metal"); ``` Directory-level `rerun-if-changed` covers the `.m` and all three headers, and survives new files being added. (Cargo recurses into directories for this directive.) Keep or drop the existing per-file line as you prefer — the directory one subsumes it. While in here, consider adding a short comment recording that `native/metal/` is a vendored mirror of `../ds4` and which files are expected to stay byte-identical upstream. That is the missing half of the sync workflow. ## Acceptance criteria - `touch native/metal/ds4_gpu.h && cargo build` recompiles `ds4_metal.m`. (Verify with `cargo build -v` or by checking the object file's mtime.) - Same for `ds4.h` and `ds4_ssd.h`. - `make bundle` still succeeds.

hugo added the bug label 2026-07-25 09:42:00 +00:00

hugo commented

2026-07-25 11:06:59 +00:00

Fixed in 7686c22. build.rs now tracks the entire native/metal directory, covering ds4_metal.m, all three current headers, and future vendored additions. Verified by touching ds4_gpu.h, ds4.h, and ds4_ssd.h individually; each caused Cargo to recompile the native target. Also passed cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.

Fixed in 7686c22. build.rs now tracks the entire native/metal directory, covering ds4_metal.m, all three current headers, and future vendored additions. Verified by touching ds4_gpu.h, ds4.h, and ds4_ssd.h individually; each caused Cargo to recompile the native target. Also passed cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.

hugo closed this issue

2026-07-25 11:07:03 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: hugo/DS4Server#15