chore(nix): add development shell and CI#2128
Conversation
Provide a Nix flake devshell so contributors can get a reproducible environment (rustc, cargo, rustfmt, clippy, cbindgen, cmake, autotools) without manual setup. The Rust toolchain is read from the existing rust-toolchain.toml via the rust-overlay input, so the devshell tracks the same channel as CI and rustup.
Mirror the CODEOWNERS convention already used in dd-trace-rb and libdatadog-rb so the Nix guild reviews changes to the development shell alongside the existing tooling owners.
Add a GitHub Actions workflow that enters the Nix development shell and builds the workspace on Linux (x86_64, aarch64) and macOS (arm64), mirroring the Nix CI in dd-trace-rb and libdatadog-rb. This guards the devshell against bit-rot so contributors relying on it keep a working, toolchain-pinned environment. Co-own the workflow with the Nix guild.
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: cfb9257 | Docs | Datadog PR Page | Give us feedback! |
The stdenv cc-wrapper injects -D_FORTIFY_SOURCE, which glibc rejects when compiling without optimization. spawn_worker's build script compiles its trampoline.c at -O0 with -Werror, turning the fortify #warning into a hard error and breaking a workspace build inside the devshell. Disable fortify hardening so these C build steps succeed.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efca305b12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efca305b12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
| platform: | ||
| - os: darwin | ||
| cpu: arm64 | ||
| base: macos-15 # always arm64-darwin |
There was a problem hiding this comment.
Is macos strictly needed? I ask because macos runners are scarce and ocuppying one for this might hurt us a bit in terms of merge times
There was a problem hiding this comment.
Yes, it guarantees that it consistently works across OSes for any dev that uses Nix on a macOS machine
Should be mostly mitigated by conditional running mentioned above.
There was a problem hiding this comment.
You could also use a concurrency group (example from our test workflow)and cancel in progress if you don't need commit level granularity.
Not necessarily a hill I want to die on, but I think using a concurrency group is a good compromise between getting signal that something broke and not potentially backing up the MergeQueue waiting for a macOS runner for a workflow that doesn't indicate an actual code issue.
The devshell is a rarely-changing safeguard, so gating every pull request on it is unnecessary. Drop the blanket pull_request trigger and keep the workflow on main and mq-working-branch-* pushes; devshell breakage will still surface in the main pipeline.
Keep the devshell guard responsive to changes that actually affect it: run on pull requests that modify the Nix files (paths mirror the Nix CODEOWNERS entries), rust-toolchain.toml (read by the flake), or this workflow.
Add a non-default `.#nightly` devshell built from ./nightly-toolchain.toml (via rust-overlay's fromRustupToolchainFile), for the workflows that need a nightly compiler. Factor the shared shell definition into mkDevShell so default and nightly only differ by toolchain.
| fuzz/ @DataDog/chaos-platform | ||
|
|
||
| # Nix | ||
| *.nix @DataDog/nix-guild @DataDog/apm-common-components-core |
There was a problem hiding this comment.
It's a small but growing group. Join us at #nix!
| hardeningDisable = [ "fortify" "fortify3" ]; | ||
|
|
||
| buildInputs = [ | ||
| rust # rustc + cargo + rustfmt + clippy, pinned via toolchain file |
There was a problem hiding this comment.
One slightly annoying thing here is that we need to cargo fmt with nightly. I wonder if we should provide a nightly toolchain pinned as well, or even maybe propose a "libdd-fmt" alias that uses the right nightly toolchain under the hood. It's a detail though, and could be worked out in a follow up PR.
There was a problem hiding this comment.
Coincidentally I just added a nightly toolchain as .#nightly.
Maybe we can even work out something that would have a closure over the nightly toolchain as an internal dependency for a nightly-based cargo fmt - and only that - to be exposed. That's exactly the kind of thing Nix can solve.
Indeed probably for a later PR.
There was a problem hiding this comment.
To clarify usage as of today:
nix develop .#nightlyto enter a shell with the nightly toolchainnix develop .#nightly -c cargo fmtto run its nightlycargoright away
|
Do you know what are those Test Nix / test report jobs, or where they are coming from? They have no logs and a Windows one doesn't make sense 
|
Interesting. I'm not sure, let me look that up. |
ekump
left a comment
ekump
left a comment
There was a problem hiding this comment.
Left a few nit comments, but I'm not a nix guy so not sure how relevant they are.
LGTM after we add a little bit of documentation and add a concurrency group for the workflow.
| # hardening in the shell so those builds succeed. | ||
| hardeningDisable = [ "fortify" "fortify3" ]; | ||
|
|
||
| buildInputs = [ |
There was a problem hiding this comment.
nit: Should this be nativeBuildInputs since they are build-time host tools?
There was a problem hiding this comment.
True-ish: it's only really relevant for cross-compilation so it doesn't really matter for devshells as they are always running on the host platform, although there's some subtlety like xdg paths and such.
I'll amend for the sake of good practice.
| flake-utils.url = "github:numtide/flake-utils"; | ||
|
|
||
| # backwards compatibility with nix-build and nix-shell | ||
| flake-compat.url = "https://flakehub.com/f/edolstra/flake-compat/1.tar.gz"; |
There was a problem hiding this comment.
nit: Is this necessary? shell.nix or default.nix aren't included in this repo (maybe they will be in the future?).
There was a problem hiding this comment.
Ha, I forgot to add the shims, will add.
| platform: | ||
| - os: darwin | ||
| cpu: arm64 | ||
| base: macos-15 # always arm64-darwin |
There was a problem hiding this comment.
You could also use a concurrency group (example from our test workflow)and cancel in progress if you don't need commit level granularity.
Not necessarily a hill I want to die on, but I think using a concurrency group is a good compromise between getting signal that something broke and not potentially backing up the MergeQueue waiting for a macOS runner for a workflow that doesn't indicate an actual code issue.
|
|
||
| permissions: | ||
| contents: read | ||
| id-token: write |
There was a problem hiding this comment.
Is this necessary? could you get away with contents: read?
There was a problem hiding this comment.
I think it would be fine without it indeed.
| @@ -0,0 +1,69 @@ | |||
| name: Test Nix | |||
There was a problem hiding this comment.
Can we add documentation (maybe here and / or inline in this file) with instructions on what to do if there is a failure? I suspect it'll only come up on MSRV bumps.
There was a problem hiding this comment.
I'm not too sure what the expectation of the doc would be: some basic usage examples to onboard the Nix-unaware / a LLM agent maybe?
I did not want to be too invasive as a first PR but seems like you folks are quite on board with this so I feel like the doc you pointed to would be a good place. Will add.
lloeki
left a comment
lloeki
left a comment
There was a problem hiding this comment.
Thanks for the astute reviews! Will address.
| hardeningDisable = [ "fortify" "fortify3" ]; | ||
|
|
||
| buildInputs = [ | ||
| rust # rustc + cargo + rustfmt + clippy, pinned via toolchain file |
There was a problem hiding this comment.
To clarify usage as of today:
nix develop .#nightlyto enter a shell with the nightly toolchainnix develop .#nightly -c cargo fmtto run its nightlycargoright away
| # hardening in the shell so those builds succeed. | ||
| hardeningDisable = [ "fortify" "fortify3" ]; | ||
|
|
||
| buildInputs = [ |
There was a problem hiding this comment.
True-ish: it's only really relevant for cross-compilation so it doesn't really matter for devshells as they are always running on the host platform, although there's some subtlety like xdg paths and such.
I'll amend for the sake of good practice.
| flake-utils.url = "github:numtide/flake-utils"; | ||
|
|
||
| # backwards compatibility with nix-build and nix-shell | ||
| flake-compat.url = "https://flakehub.com/f/edolstra/flake-compat/1.tar.gz"; |
There was a problem hiding this comment.
Ha, I forgot to add the shims, will add.
|
|
||
| permissions: | ||
| contents: read | ||
| id-token: write |
There was a problem hiding this comment.
I think it would be fine without it indeed.
| @@ -0,0 +1,69 @@ | |||
| name: Test Nix | |||
There was a problem hiding this comment.
I'm not too sure what the expectation of the doc would be: some basic usage examples to onboard the Nix-unaware / a LLM agent maybe?
I did not want to be too invasive as a first PR but seems like you folks are quite on board with this so I feel like the doc you pointed to would be a good place. Will add.
| fuzz/ @DataDog/chaos-platform | ||
|
|
||
| # Nix | ||
| *.nix @DataDog/nix-guild @DataDog/apm-common-components-core |
There was a problem hiding this comment.
It's a small but growing group. Join us at #nix!
|
Ok so I looked up the
|
| push: | ||
| branches: | ||
| - main | ||
| - mq-working-branch-* |
There was a problem hiding this comment.
| push: | |
| branches: | |
| - main | |
| - mq-working-branch-* | |
| # Nightly safeguard: catches devshell drift from changes that don't match the | |
| # paths above (e.g. a dependency or build-script change that needs new native | |
| # tooling). Scheduled runs only fire from the default branch, against its HEAD. | |
| schedule: | |
| - cron: "0 4 * * *" # daily at 04:00 UTC |
There was a problem hiding this comment.
Instead of triggering it in every PR I think it would be better to have a nightly schedule. That way we can save some capacity for the actual pipelines.
There was a problem hiding this comment.
But wouldn't you want to check tings on PRs that change Nix files at least?
There was a problem hiding this comment.
Definitely.
As is it's not going to trigger in every PR anyway, only those that change nix files or that have an impact on nix (the toolchain files).
The schedule can be nice though as a sanity check for when those don't change often.
|
|
||
| { | ||
| inputs = { | ||
| nixpkgs.url = "github:nixos/nixpkgs/release-25.11"; |
There was a problem hiding this comment.
the 26.05 channel is the latest stable, why not use it instead ?
And what about using unstable instead ?
There was a problem hiding this comment.
Unstable is nice to have the very latest version, but I think you won't hit the binary cache, which might mean a lot of full local rebuilds from source. The latest stable makes sense though 👍
There was a problem hiding this comment.
Does unstable really makes a difference with binary cache ? I never hit that issue personally.
There was a problem hiding this comment.
Unstable is actually cached, but it moves fast. I vaguely remember situations such as user A updates the flake.lock, 1 month passes, new user B checks the project out from scratch and inherits the same flake.lock, but stuff isn't in the unstable cache anymore. If you are the one updating you shouldn't have issues ofc, because when you check out the package, they are the most recent available and thus in cache. I think. I am not sure. Both latest stable and unstable are probably reasonable.
Another reason could be if you run NixOS, you usually target a specific release, and you probably don't want to duplicate many versions of core packages (the stable and the unstable one).
|
Hey I wanted to suggest many small changes (mostly styling/preferences), I created a stacked PR instead because it got too large |
5 participants
What does this PR do?
Adds a Nix flake development shell to the repository, with a CI workflow that exercises it.
flake.nix/flake.lock: a devshell providing the Rust toolchain (rustc, cargo, rustfmt, clippy), cbindgen, cmake and autotools. The Rust toolchain is pinned via theoxalica/rust-overlayinput reading the existingrust-toolchain.toml, so the devshell tracks the same channel as CI and rustup..github/workflows/nix.yml: enters the devshell and builds the workspace on Linux (x86_64, aarch64) and macOS (arm64), mirroring the Nix CI already present in dd-trace-rb and libdatadog-rb.CODEOWNERS: co-owns the Nix files and workflow with the Nix guild.Motivation
Contributors using Nix previously kept these files locally and unshared, with the Rust version floating with the nixpkgs channel (currently 1.91.1) rather than matching the toolchain libdatadog is built with. Committing a pinned devshell gives a reproducible, low-setup environment, and the CI guard keeps it from bit-rotting.
Additional Notes
The pin derives from
rust-toolchain.toml(single source of truth), so a future MSRV bump updates the devshell automatically.AI was used to accelerate implementation; all code was reviewed and understood.
How to test the change?
nix develop --command cargo build --workspace --exclude builder, and confirmnix develop --command rustc --versionreports the channel fromrust-toolchain.toml. CI runs the same build across the three platforms.