Skip to content

Fix all the nix - #4187

Draft
jeff-hykin wants to merge 110 commits into
mainfrom
jeff/fix/native_build_cargo_path
Draft

jeff-hykin wants to merge 110 commits into
mainfrom
jeff/fix/native_build_cargo_path

Conversation

@jeff-hykin

@jeff-hykin jeff-hykin commented Sep 16, 2026

Copy link
Copy Markdown
Member
  • CI should be 30% faster
  • standardize native modules
  • crate2nix
  • fix nix store problems

…missing

Non-interactive shells (ssh commands, systemd units) don't source the rustup
PATH entry, so `cargo build --release` build_commands died with exit 127 on
robots even though cargo was installed.
rustup's PATH entry is absent in non-interactive shells (ssh commands, systemd
units), so the plain `cargo build --release` build_commands died with exit 127
on robots. Run them through `nix develop` instead, like the realsense module
already does, and put cargo/rustc in the flake so the shell is self-contained.
main's native-module publish gate AST-scans each module's config and refuses an
f-string default, so the DIMOS_PROJECT_ROOT interpolation has to go. A cwd-relative
nix path keeps the symlink behavior the resolve() was there for.
A flake ref copies its whole source tree into the nix store, and a fresh
copy each time that tree changes. Aimed at the repo root that is the build
dirs, .venv and .git, or the tracked LFS blobs -- tens of gigabytes per
build. nix/rust/ is two files, so the snapshot is stable and free.

Also run cargo fmt and cargo clippy inside that shell, and stop the clippy
hook from re-entering the repo-root flake: the root has its own flake.nix,
so the per-crate branch was matching it and shelling into the whole tree.
`nix develop ... -c cargo build` left the result in a per-machine target
dir and needed a working cargo cache. A `nix build` of the whole cargo
workspace is one hermetic derivation that Cachix can substitute, and its
src is assembled from path literals so a doc or python edit neither
rebuilds it nor invalidates the publish marker.

Also give the clippy/fmt shell librealsense on Linux, so the realsense
crate can join the workspace without breaking macOS.
CI builds the PR merge commit, so the dependabot cargo bump on main
(de2b0fc) changed the lock the vendor derivation sees. Take the value
that commit's lock actually vendors to.
The root flake hardcoded every crate directory in the workspace, so
adding a module meant editing a file far from it. Each module now owns a
deps.nix naming the trees it contributes, the cargo packages that are
module executables, and any system libraries it links; the flake only
composes them.

The inputs-hash scanner follows imported .nix files for the same reason
it follows path: inputs -- otherwise a crate reachable only through a
deps.nix would not key the Cachix publish marker.
Adding a rust native module no longer means editing the repo-root flake,
which was the last place the module list was duplicated.

The search rule is deliberately the dumbest one available -- every file
named deps.nix, anywhere in the tree -- because the Cachix publish gate
in bin/build-native-modules has to arrive at the same set without
evaluating nix, and it does that with a glob. Anything cleverer would be
two implementations that drift.

Searching makes the flake name ./. , which the publish gate would
otherwise hash as the whole repository, rekeying the marker on every
commit. readDir reads directory names rather than file contents, so the
deps.nix files and the crates they name are the only content that
actually reaches the derivation, and both are already collected. The
exemption is therefore limited to that one expression: any other ./. in
the root flake is now a hard error.
cargoHash is one fixed-output hash over every vendored crate, so any
Cargo.lock change invalidates it -- including a dependabot bump landing on
main, which CI surfaces through the merge commit and so fails pull
requests that never touched rust. Recovering means a human reading a hash
out of a vendor-staging log and pasting it back.

cargoLock uses the per-crate checksums already in the lock, so there is no
aggregate hash to maintain. Git dependencies still need an entry because
the lock records no checksum for them, but that hash is keyed to a pinned
rev rather than to the lock as a whole.

An earlier revision of this branch claimed nixpkgs could not fetch crates
this way because crates.io rejects the curl user agent. That was measured
with a manual curl against a different URL and never tested against the
actual build, which fetches without trouble.
The 403 does not reproduce locally -- the crate tarballs are
fixed-output, so they substitute from cache.nixos.org and the fetch
never runs. Only CI, on a cache miss, actually hits crates.io. Note
that, and the reason the CDN cannot be substituted in, so the next
attempt does not have to rediscover it from a red build.
#3741 changed Cargo.lock on main. CI builds the merge commit, so the
vendor hash went stale on a branch that never touched rust -- the
failure mode the comment above the hash describes.
Cargo.nix gives every crate its own derivation, so editing one module
does not rebuild the others and each crate caches on its own. Measured:
a cold ray_tracing build is 11m43s, and mls_planner right after is 21.8s.

It also removes the aggregate cargoHash, which had to be re-pasted by
hand whenever Cargo.lock moved -- including from main, which breaks
branches that never touched rust, because CI builds the merge commit.
The replacement failure mode is a stale generated file, which the new
cargo-nix-current job prints as a diff instead of a number to re-paste.

Regenerate with bin/regen-cargo-nix. The generated pair is excluded from
the byte-rewriting pre-commit hooks so it stays reproducible, and
Cargo.nix is exempted from the large-files check -- LFS would be wrong
for a file CI has to diff as text.
Adding a module is now a deps.nix and nothing else, so the examples can be
real build targets instead of a workspace member nothing referenced. They
build in 42s here because every crate under them is already a store path.

Doubles as the regression test for that claim: if adding a module ever starts
requiring a flake.nix edit again, this file is the one that stops building.
Its keys are scoped to a git dependency's branch and carry no rev, so a
committed copy silently goes stale the second that branch moves, pinning a
hash that no longer matches the revision Cargo.lock resolves to. That is the
exact failure this whole change set exists to remove.

It is also redundant: it is only crate2nix's prefetch cache, and the hashes
are already inlined in Cargo.nix. Regenerating with the file absent produces
a byte-identical Cargo.nix, so nothing in the repo depends on it.

Dropping it also retires two problems rather than papering over them: it no
longer needs a pre-commit exclusion (pretty-format-json was rewriting it on
every commit, which had already introduced a trailing newline the generator
never emits), and it no longer needs an editorconfig exception for that
missing final newline.
crate2nix shells out to `cargo metadata`, so the generated file depends on
whichever cargo is on PATH. The generator itself was pinned by flake.lock but
cargo was not: locally that was a rustup 1.98.0, while the toolchain this repo
pins for its clippy and fmt hooks is 1.94.0, and CI would have used whatever
the runner image ships.

The two happen to agree today -- checked by regenerating under 1.94.0 and
getting a byte-identical Cargo.nix -- so this fixes a latent problem rather
than a live one. Worth doing anyway: the failure it prevents is a diff that
only appears on the machine with the odd cargo, which is the hardest kind to
read when cargo-nix-current is what reports it.
I had commented it as crate2nix's per-crate override hook. It is not: the
`crateConfig` argument Cargo.nix defaults to this path is declared and then
never read anywhere in the generated file. Overrides added here would have
silently done nothing, which is a worse trap than having no file at all.

The file still has to exist -- `bin/build-native-modules --inputs-hash` walks
nix path literals textually, cannot evaluate the `builtins.pathExists` guard
around this one, and exits 1 with "reference './crate-config.nix' does not
exist". Verified by moving the file away. Kept an empty file rather than
teaching that parser about conditional paths, since a parser that learns to
skip guarded references can also skip one that matters.
…_cargo_path_wt

# Conflicts:
#	.github/workflows/ci.yml
Three things the merge with main surfaced.

livox and virtual_mid360 were still on a bare `cargo build --release`, which
assumes a rust toolchain on PATH. The R1 Pro has none — nix is installed there,
cargo is not — so those modules could never have built on the robot. Each gets a
deps.nix naming its cargo package, which is all the root flake needs.

turbojpeg-sys vendors libjpeg-turbo and drives cmake from its own build script,
so it failed outright in the sandbox with "is `cmake` not installed?". The hook
for that is the defaultCrateOverrides argument to Cargo.nix, exactly as
crate-config.nix's comment says; it was never actually set. dontUseCmakeConfigure
goes with it, or nixpkgs' cmake setup hook tries to configure a crate root that
has no CMakeLists.txt.

_collect_input_paths followed a literal naming a .nix *file* but never one naming
a directory that is itself a flake. Cargo.nix names every workspace member's
directory as a crate src, and main has since added a member that carries its own
flake — dimos/experimental/memory/rust, whose fileset reaches back out to the
repo-root Cargo.lock. So that directory entered the input set while the paths its
flake pulls in did not, which is the gap
test_flake_refs_resolve_and_are_covered exists to catch. Following it can only
widen the input set, which is the safe direction for a cache-publish marker.

Claude-Session: https://claude.ai/code/session_01ExBQYh7BJX5bnGHTKpNVDy
…_cargo_path

# Conflicts:
#	.pre-commit-config.yaml
Exports lib.<system>.buildNativeModule, which assembles a sandbox holding a
module at its repo-relative path plus the shared crates at theirs -- so a
module's existing `path = "../../native/rust/dimos-module"` dependency
resolves unchanged -- and runs crate2nix over it. crate2nix emits one
derivation per crate, so the shared crates and every third-party dependency
are built once and substituted into every other module. buildRustPackage
cannot do that: it vendors and compiles the whole graph privately per
package, which is why the repo had one all-modules-at-once derivation.

The two crates become their own workspace roots so the layout cargo sees
locally is the layout it sees inside the derivation.
The C++ module flakes reached it as a `${../../../../../../native/cpp}`
path literal. Under the `path:.` build refs the modules are moving to, such
a literal escapes the module's store path and cannot resolve at all; before
that it resolved by copying the entire repository.
Every native module now carries its own Cargo.toml workspace root and its
own Cargo.lock, and the repo-root Cargo.toml/Cargo.lock are gone. Each new
root restates the old root's [profile.*], since cargo only honours a profile
at a workspace root.

.cargo/config.toml gains `[build] target-dir = "target"`. Config paths
resolve against the config file's own directory, so the modules -- which no
longer share a workspace -- still share one repo-root target/, and a module
directory stays free of build output. That matters because the build refs
are becoming `path:.`, which copies a directory wholesale: a module-local
target/ would be copied back into the store on every rebuild.
pointlio's rust module and virtual_mid360 reached sideways into
dimos/hardware/sensors/lidar/livox/rust for it. A native module may only
reach the shared native/ crates, and livox is a specific sensor rather than
part of that generic API.

Only wire.rs, pipeline.rs and pcap.rs are copied -- 1546 of livox's 2820
lines, and all either consumer actually used. live.rs, module.rs and main.rs
are the Mid-360 driver and stay put. The three files are copied byte for
byte so a protocol fix can be moved across with cp, and each consumer
declares them #[allow(dead_code)] because neither uses the whole surface.
…ories

pointlio/cpp and fastlio2/cpp took the Livox SDK from the livox module's
flake (`path:../../livox/cpp`) and its headers from `../../common`, and all
three C++ flakes named native/cpp with a bare path literal. The SDK
derivation, its darwin patch and the common headers are now copied into each
module, and native/cpp arrives as a flake input.

Keep the copied livox-sdk2 derivation in step with the original in
dimos/hardware/sensors/lidar/livox/cpp/flake.nix.
Profiling the last cachix-build changed what is worth optimising: it copied 2461
store paths and built 17. These jobs substitute far more than they compile, so
`release = false` and a second build worker -- both aimed at compile time -- were
aimed at the wrong thing. Correct, and not where the minutes are.

nix defaults to 25 parallel HTTP connections. crate2nix's one-derivation-per-crate
graph is precisely what makes the fetch thousands of small paths, so the sharing
that makes the builds cheap is what makes the download expensive. 50 now, in all
seven nix job configs.

Also relocks pointlio/cpp, habitat/nix and examples/native-modules/cpp: native/cpp
changed when cleanModuleSource left it, and only fastlio2 had been relocked. That
was the arm job's failure in the last run -- the guard working, not a flake.
The standalone-flake rewrite dropped --shell along with the native/rust flake,
then re-added that flake as a devShell in the same commit. So the option came
out for no remaining reason, and lint's pre-commit step went 0.2m (main) to
1.4m: eleven nix evals and twenty-two nix develops for a tool that only parses.

Locally 11.8s -> 5.9s, and realsense and dim_slam are formatted on darwin now
instead of skipped for want of a devShell rustfmt never needed.
jobs:
lint:
timeout-minutes: 1
timeout-minutes: 1.5

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

40sec of ruff/uv and ~7sec of nix.

The .5 is effectively padding to make sure this isn't flakey

Comment thread .github/workflows/ci.yml Outdated
@jeff-hykin

Copy link
Copy Markdown
Member Author

Todo:

  • make CI rust use nix
  • figure out something for maturin / pyo3 needing UV (not nix) python at compile time

- name: uv lock
run: uv lock

- name: Did native/ change?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is for bumping flake locks whenever the root (./native/rust or ./native/cpp) stuff changes

Comment thread .github/workflows/ci.yml
- name: Build native modules
env:
BUILD_WORKERS: "1"
BUILD_WORKERS: "4"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

free performance. No Idea why it was set to 1

Comment thread .github/workflows/ci.yml
BUILD_WORKERS: "1"
BUILD_WORKERS: "4"
run: python3 bin/build-native-modules
- name: Lint native modules

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linting is now a nix thing

Comment thread .github/workflows/ci.yml
- name: Install Nix
env:
INPUT_EXTRA_NIX_CONFIG: |
http-connections = 50

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make cache loading faster

Comment thread .github/workflows/ci.yml
timeout-minutes: 30
runs-on: ubuntu-latest
env:
CARGO_INCREMENTAL: "0"

@jeff-hykin jeff-hykin Sep 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

time save: prevents creating an always-gets-busted cache

Comment thread .github/workflows/ci.yml
-not -path '*/result/*' \
-printf '%h\n' \
modules=$(git ls-files '*/flake.nix' \
| sed 's|/flake.nix$||' \

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

native/cpp now has a flake.nix, need this find to not find that one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip Skip creating a backport to any release branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant