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: c2a77fe | 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".
| base: ubuntu-24.04 # always x86_64-linux-gnu | ||
| - os: linux | ||
| cpu: aarch64 | ||
| base: ubuntu-24.04-arm # always aarch64-linux-gnu |
There was a problem hiding this comment.
In the Linux matrix entries this runs the full cargo build --workspace --exclude builder on standard ubuntu-24.04/ubuntu-24.04-arm runners. The existing test workflow explicitly routes Ubuntu full-workspace builds to the APM larger runner pool because the default GitHub-hosted Ubuntu disk fills during that same workspace build (.github/workflows/test.yml:74-76), and nix develop will also populate /nix/store, so this new check is likely to fail before exercising the flake.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Seems like it's working ok?
There was a problem hiding this comment.
You're probably ok, since you're compiling a single build. The test workflow is doing multiple, and also doing extra stuff like compiling test fixtures for crashtracker tests, generating code coverage reports, etc.
You could temporarily output df here to see how much headroom you have and potentially use the free-disk-space action like we do here (You probably don't have to be as aggressive as the test workflow is).
|
|
||
| buildInputs = [ | ||
| rust # rustc + cargo + rustfmt + clippy, pinned via toolchain file | ||
| pkgs.rust-cbindgen |
There was a problem hiding this comment.
Should cbindgen be pinned as well?
There was a problem hiding this comment.
I'm not sure, but it's probably not critical for a first PR, since the headers aren't checked in libdatadog manually but are generated automatically when building the FFI (thus using a pinned version). Having cbindgen is nice for local development if you are curious about the generated C interface but it's indicative I believe (plus the fact that I expect cbindgen output to be stable?).
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".
| @@ -0,0 +1,53 @@ | |||
| { | |||
There was a problem hiding this comment.
AGENTS.md states that all source files must carry the Apache 2.0 license header, but this new Nix source file starts directly with the expression body. Since .nix files can include comments, add the standard Datadog copyright and SPDX header here so the newly added development-shell source follows the repository's licensing convention.
Useful? React with 👍 / 👎.
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 |
| 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.
|
|
||
| buildInputs = [ | ||
| rust # rustc + cargo + rustfmt + clippy, pinned via toolchain file | ||
| pkgs.rust-cbindgen |
There was a problem hiding this comment.
I'm not sure, but it's probably not critical for a first PR, since the headers aren't checked in libdatadog manually but are generated automatically when building the FFI (thus using a pinned version). Having cbindgen is nice for local development if you are curious about the generated C interface but it's indicative I believe (plus the fact that I expect cbindgen output to be stable?).
|
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?
| 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?).
| 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?
| @@ -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.
4 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.