chore: switch to crate2nix by Lymah123 · Pull Request #2719 · molybdenumsoftware/statix

Lymah123 · 2026-07-26T05:25:00Z

Switches the statix package build from rustPlatform.buildRustPackage to crate2nix, which builds each crate as a separate Nix derivation enabling more granular caching.

Changes:

Adds github:nix-community/crate2nix as a flake = false input
Threads inputs through the overlay so crate2nixSrc is available to callPackage
Rewrites packages/statix.nix to use tools.generatedCargoNix (IFD). Cargo.nix is generated at build time rather than checked in
Removes allow-import-from-derivation = false from nixConfig since IFD is now required

The statix-vim package is unaffected.

Lymah123 · 2026-07-26T05:39:20Z

Hi @mightyiam , this PR is ready for review.

mightyiam

What about the experimental crate2nix json api?

Lymah123 · 2026-07-27T01:34:21Z

What about the experimental crate2nix json api?

Will investigate the experimental JSON output separately and report back in the discussion.

mightyiam

Any news on the JSON API?

Lymah123 · 2026-08-04T02:38:36Z

Any news on the JSON API?

Still pending, will try it after resolving the current comments. If it simplifies the approach, happy to switch to it entirely.

mightyiam

It seems that this change results in our dev shell including some rust crate derivations. Can that be avoided?

It seems that this change results in tests not being run during the build. Rust tests | crate2nix.

Lymah123 · 2026-08-06T00:01:19Z

It seems that this change results in our dev shell including some rust crate derivations. Can that be avoided?

inputsFrom = [ pkgs.statix ] previously brought in rustc, cargo etc. from rustPlatform.buildRustPackage. With crate2nix it pulls in individual compiled crate derivations instead, which aren't useful in a dev shell. Will replace it with explicit pkgs.cargo, pkgs.rustc etc.

Lymah123 · 2026-08-06T00:04:10Z

It seems that this change results in tests not being run during the build. Rust tests | crate2nix.

Confirmed, workspaceMembers.statix.build only builds, tests need .override { runTests = true; }. Will add a separate checks.statix-tests using that.

Lymah123 · 2026-08-06T19:58:22Z

Investigated the test failure. The existing integration tests in bin/tests/ call Command::new("cargo").arg("run") inside _utils::test_cli to invoke statix, they shell out to cargo run rather than calling a pre-built binary. This is incompatible with the Nix sandbox where cargo isn't on PATH and the workspace isn't present at test time.

runTests = true only works for tests that compile into the test binary directly. To run these integration tests in a Nix derivation, they'd need to be rewritten to use the built binary path (like walk.rs does with env!("CARGO_BIN_EXE_statix")). That's a separate, non-trivial change.

For now I've removed checks."statix-tests" from this PR, it belongs in a follow-up that also fixes the test infrastructure.

mightyiam · 2026-08-07T06:49:35Z

For now I've removed checks."statix-tests" from this PR, it belongs in a follow-up that also fixes the test infrastructure.

Does that feel reasonable to you?

Lymah123 · 2026-08-07T11:49:33Z

For now I've removed checks."statix-tests" from this PR, it belongs in a follow-up that also fixes the test infrastructure.

Does that feel reasonable to you?

Yes, that feels reasonable. The test infrastructure issue is real and worth fixing properly in a follow-up, the existing integration tests call cargo run which can't work in the Nix sandbox. When we get to that follow-up, we'd need to rewrite _utils::test_cli to use env!("CARGO_BIN_EXE_statix") like walk.rs does, and then runTests = true should work cleanly.

mightyiam · 2026-08-07T12:25:30Z

The change you are currently suggesting disables the running of all of the program's cargo tests during nix flake check and therefore from CI checks. Am I right?

Lymah123 · 2026-08-07T12:48:37Z

The change you are currently suggesting disables the running of all of the program's cargo tests during nix flake check and therefore from CI checks. Am I right?

You're right, that is a regression. rustPlatform.buildRustPackage ran cargo test by default, crate2nix doesn't. The test infrastructure issue (tests calling cargo run rather than a built binary) needs to be fixed as part of this PR, not deferred, since shipping without it would mean CI has no Rust test coverage at all.

Two options I can see:

Fix _utils::test_cli to use env!("CARGO_BIN_EXE_statix") and use runTests = true, the correct long-term fix but more work
Revert to rustPlatform.buildRustPackage for now and open a separate PR to switch to crate2nix once the test infrastructure is ready

Which would you prefer?

mightyiam · 2026-08-07T15:04:09Z

Sorry, I'm not sure what you mean by "once the test infrastructure is ready". What infrastructure is that, please? What is not ready? Something about cargo not being in PATH? cargo can be added to PATH wherever necessary.

Lymah123 · 2026-08-07T16:47:38Z

Sorry, I'm not sure what you mean by "once the test infrastructure is ready". What infrastructure is that, please? What is not ready? Something about cargo not being in PATH? cargo can be added to PATH wherever necessary.

You're right, I overcomplicated it. The tests call cargo run, adding cargo (and rustc) to the test derivation's PATH via testPreRun or nativeBuildInputs should be enough. Will try that.

Lymah123 · 2026-08-08T11:37:47Z

Adding cargo to PATH got 32/56 tests passing. The remaining 24 fail because cargo run tries to recompile statix from source and needs network access (crates.io) which isn't available in the sandbox. A fake cargo wrapper that intercepts cargo run -- and delegates to the pre-built pkgs.statix binary fixes this, would that approach be acceptable, or would you prefer a different direction?

mightyiam · 2026-08-13T15:03:22Z

@@ -1,44 +1,31 @@
 use std::{io::Write, process::Command, process::Stdio};
-


Is this intentional?

No, no intentional. Will restore it.

mightyiam · 2026-08-13T23:04:51Z

-    let mut child = Command::new("cargo")
-        .arg("run")
-        .arg("--")
+    let mut child = Command::new(env!("CARGO_BIN_EXE_statix"))


Can this be made a precursor PR?

Yes, will extract the _utils.rs change into a precursor PR, then rebase this one on top of it.

mightyiam · 2026-08-13T23:05:54Z

+        clippy = pkgs.rustPlatform.buildRustPackage {
+          pname = "statix-clippy";
+          version = "0.6.0-git";
+          inherit src;
+          cargoLock.lockFile = root + "/Cargo.lock";
+          RUSTFLAGS = "-D warnings";
+          nativeBuildInputs = [ pkgs.clippy ];
+          buildPhase = "cargo clippy --all-targets --all-features";
+          installPhase = "touch $out";
+          doCheck = false;
+        };


Can clippy be based on crate2nix as well?

Yes, will switch clippy to use pkgs.statix-workspace, the workspace already has all crates built, so clippy can run against those derivations without needing a separate buildRustPackage

Crate2nix doesn't expose a clippy mechanism through its build attributes, it builds crates via rustc derivations with no clippy hook. The rustPlatform.buildRustPackage approach is the right tool for running clippy; I've updated it to use the shared pkgs.statix-src from packages/src.nix so there's no duplication with the crate2nix builds.

Doesn't this section demonstrate a useClippy = true? Isn't that what we are looking for?

mightyiam · 2026-08-13T23:07:51Z

+  cargoNix = crate2nixTools.generatedCargoNix {
+    name = "statix";
+    src = lib.fileset.toSource {
+      root = ../.;
+      fileset = lib.fileset.unions [
+        (lib.fileset.fileFilter (
+          file:
+          lib.any lib.id [
+            (file.name == "Cargo.toml")
+            (file.hasExt "rs")
+            (file.hasExt "snap")
+          ]
+        ) ../.)
+        ../Cargo.lock
+        ../insta.yaml
+      ];
+    };


It seems that this expression exists twice. Is that intentional?

Yes, not intentional. Will deduplicate, the fileset can be defined once and referenced in both, probably by moving it into the overlay or having statix-workspace re-export it for statix.nix to use.

mightyiam · 2026-08-18T14:34:46Z

+        clippy = pkgs.rustPlatform.buildRustPackage {
+          pname = "statix-clippy";
+          version = "0.6.0-git";
+          inherit src;
+          cargoLock.lockFile = root + "/Cargo.lock";
+          RUSTFLAGS = "-D warnings";
+          nativeBuildInputs = [ pkgs.clippy ];
+          buildPhase = "cargo clippy --all-targets --all-features";
+          installPhase = "touch $out";
+          doCheck = false;
+        };


Doesn't this section demonstrate a useClippy = true? Isn't that what we are looking for?

mightyiam · 2026-08-18T14:36:18Z

-  meta = {
-    mainProgram = "statix";
-    description = "Lints and suggestions for the Nix programming language";
-    homepage = "https://github.com/molybdenumsoftware/statix";
-    license = lib.licenses.mit;


Do we lose these or are they automatically populated from Cargo.toml?

They're lost, crate2nix only auto-populates mainProgram. description, homepage, and license need to be added back via overrideAttrs. Will fix.

Lymah123 · 2026-08-19T05:51:03Z

Doesn't this section demonstrate a useClippy = true? Isn't that what we are looking for?

You're right, build-rust-crate exposes useClippy which switches the compiler to clippy-driver. We can pass it via crateOverrides. Will replace the rustPlatform.buildRustPackage approach with statixBuild.override { crateOverrides = pkgs.defaultCrateOverrides // { statix = _: { useClippy = true; RUSTFLAGS = "-D warnings"; }; }; }.

mightyiam

Please review thoroughly before submitting.

mightyiam · 2026-08-20T15:22:25Z

+          testPreRun = ''
+            export INSTA_SNAPSHOT_DIR=${root}/bin/tests/snapshots
+          '';


Why is this needed, please?

The integration tests use insta::assert_snapshot! which looks for snapshot files relative to the crate source location. In the Nix build sandbox, the working directory isn't the source tree, so insta can't find the snapshots without being explicitly pointed at them via INSTA_SNAPSHOT_DIR.

If

The integration tests use insta::assert_snapshot! which looks for snapshot files relative to the crate source location

it sounds like if that is the case, there should be no problem finding them. Are you sure of that explanation?

mightyiam · 2026-08-20T15:22:51Z

+{ lib }:
+lib.fileset.toSource {
+  root = ../.;
+  fileset = lib.fileset.unions [
+    (lib.fileset.fileFilter (
+      file:
+      lib.any lib.id [
+        (file.name == "Cargo.toml")
+        (file.hasExt "rs")
+        (file.hasExt "snap")
+      ]
+    ) ../.)
+    ../Cargo.lock
+    ../insta.yaml
+  ];
+}


Perhaps this should be inlined?

Happy to inline it , though that means accepting the duplication between the two files rather than avoiding it. Is that the preference, or should one file reference the other's src? Asking because the original concern was about the expression appearing twice.

Happy to inline it , though that means accepting the duplication between the two files rather than avoiding it. Is that the preference, or should one file reference the other's src? Asking because the original concern was about the expression appearing twice.

What about this? @mightyiam ?

I think as soon as the duplicate code is removed, this should be inlined into a single remaining position.

mightyiam · 2026-08-20T15:24:04Z

+  cargoNix = crate2nixTools.generatedCargoNix {
+    name = "statix";
+    src = statix-src;


Is this a duplicate?

You're right, generatedCargoNix is called identically in both files, generating the same derivation twice. The fix is to expose cargoNix itself via the overlay and have both files use it. Will restructure.

Lymah123 · 2026-08-23T12:34:35Z

@mightyiam , I am waiting for your answers, it changes the structure significantly.

mightyiam · 2026-08-23T14:08:36Z

I thought I was waiting for you. You wrote this:
#2719 (comment)

Lymah123 · 2026-08-23T14:12:44Z

I thought I was waiting for you. You wrote this: #2719 (comment)

I am waiting for your answers on my responses under your latest feedbacks

mightyiam · 2026-08-23T14:15:55Z

As is, I see duplicate code. cargoNix is declared in two different files.

mightyiam · 2026-08-23T14:33:27Z

@@ -1,11 +1,14 @@
+{ ... }:


@Lymah123 please

mightyiam · 2026-08-23T14:36:31Z

+          testPreRun = ''
+            export INSTA_SNAPSHOT_DIR=${root}/bin/tests/snapshots
+          '';


If

The integration tests use insta::assert_snapshot! which looks for snapshot files relative to the crate source location

it sounds like if that is the case, there should be no problem finding them. Are you sure of that explanation?

mightyiam · 2026-08-23T14:37:43Z

+        clippy = statixBuild.override {
+          crateOverrides = pkgs.defaultCrateOverrides // {
+            statix = _: {
+              useClippy = true;
+              RUSTFLAGS = "-D warnings";
+            };
+          };
+        };


Instead of having an additional check, can we have only one check, and that is simply the package, itself, in which clippy is used?

Makes sense, will move useClippy = true and RUSTFLAGS = "-D warnings" into the package build via crateOverrides in packages/statix.nix, so checks.statix = pkgs.statix covers clippy automatically. No separate check needed.

Lymah123 · 2026-08-23T15:18:00Z

If

The integration tests use insta::assert_snapshot! which looks for snapshot files relative to the crate source location

it sounds like if that is the case, there should be no problem finding them. Are you sure of that explanation

You're right, tested without it and the tests pass. Insta finds the snapshots via CARGO_MANIFEST_DIR which crate2nix sets correctly. Removing INSTA_SNAPSHOT_DIR

mightyiam · 2026-08-23T23:18:37Z

@@ -1,11 +1,13 @@
-{
+_: {


Please review your changes as best as you can before expecting anyone else to review them. Such trivial mistakes can be avoided.

Understood. I should have caught that before pushing. I'll be more careful.

This unrelated and undesireable change is still present.

mightyiam · 2026-08-24T14:07:01Z

-{
+_: {
  perSystem =
    { pkgs, ... }:
+    let
+      statixBuild = pkgs.statix-workspace.workspaceMembers.statix.build;
+    in
    {
      treefmt.settings.global.excludes = [ "bin/tests/data/*.nix" ];
-      checks.build = pkgs.statix;
+      checks = {
+        inherit (pkgs) statix;
+        statix-tests = statixBuild.override {
+          runTests = true;
+        };
+      };
    };
 }


Does this file need to change? Perhaps if the runTests=true is in the one single derivation, this file can remain the same?

If runTests = true is in the package itself alongside useClippy = true, then checks.build = pkgs.statix covers everything and flake-parts/statix.nix doesn't need to change. Will move runTests = true into packages/statix.nix and revert flake-parts/statix.nix to its pre-PR state.

mightyiam

Do you think that a pull request should be freshly rebased at the time review is requested?

mightyiam · 2026-08-29T05:02:00Z

@@ -1,11 +1,13 @@
-{
+_: {


This unrelated and undesireable change is still present.

mightyiam · 2026-08-29T05:06:56Z

+    crateOverrides = defaultCrateOverrides // {
+      statix = _: {
+        useClippy = true;
+        RUSTFLAGS = "-D warnings";


Should this be achieved using capLints instead?

I am not aware capLints as an idiomatic option. I will investigate and switch to it.

mightyiam · 2026-08-29T05:08:33Z

+  statix-cargo-nix = prev.callPackage ./packages/cargo-nix.nix { };
  statix = prev.callPackage ./packages/statix.nix { };
+  statix-workspace = prev.callPackage ./packages/statix-workspace.nix { };
  statix-vim = prev.callPackage ./packages/statix-vim.nix { };


Does exposing these two new intermediate packages provide any value to developers or users? Should they exist as local binding in packages/statix.nix instead?

Neither of these has value outside of being inputs to building statix itself. So, I think they should exist as local binding in packages/statix.nix.
I agreed with you because, these are implementation details that shouldn't leak into the public overlay.

Lymah123 · 2026-08-29T07:08:42Z

Do you think that a pull request should be freshly rebased at the time review is requested?

Yes. It is a good practice. I will take note of that going forward

Lymah123 · 2026-08-29T07:09:59Z

#2719 (comment)

I removed it, I am wondering how it shows again.

mightyiam · 2026-09-04T12:20:24Z

Is this waiting on me?

Lymah123 marked this pull request as ready for review July 26, 2026 05:38

mightyiam reviewed Jul 26, 2026

View reviewed changes

Comment thread overlay.nix Outdated

Lymah123 mentioned this pull request Aug 1, 2026

chore: use nixpkgs NixOS module in perSystem #2726

Merged

Lymah123 force-pushed the chore/crane-to-crate2nix branch from db377d9 to 0917330 Compare August 2, 2026 14:22

Lymah123 requested a review from mightyiam August 2, 2026 15:32

mightyiam reviewed Aug 4, 2026

View reviewed changes

Comment thread flake-parts/nixpkgs.nix Outdated

Comment thread packages/statix.nix Outdated

Comment thread overlay.nix Outdated

Lymah123 force-pushed the chore/crane-to-crate2nix branch from 0917330 to 1aab0dc Compare August 4, 2026 20:03

Lymah123 requested a review from mightyiam August 4, 2026 20:10

mightyiam reviewed Aug 5, 2026

View reviewed changes

Comment thread packages/statix.nix Outdated

Comment thread packages/statix.nix Outdated

Comment thread packages/statix.nix Outdated

Lymah123 force-pushed the chore/crane-to-crate2nix branch from 1aab0dc to 9cc159c Compare August 5, 2026 06:10

Lymah123 requested a review from mightyiam August 5, 2026 06:27

mightyiam reviewed Aug 5, 2026

View reviewed changes

Lymah123 force-pushed the chore/crane-to-crate2nix branch from 9cc159c to 08467a6 Compare August 6, 2026 20:02

Lymah123 requested a review from mightyiam August 6, 2026 20:02

Lymah123 force-pushed the chore/crane-to-crate2nix branch 3 times, most recently from 99f6ee9 to 391729a Compare August 9, 2026 06:31

mightyiam reviewed Aug 12, 2026

View reviewed changes

Comment thread flake-parts/rust.nix

Comment thread flake-parts/statix.nix Outdated

Lymah123 force-pushed the chore/crane-to-crate2nix branch from 570cf68 to 354eb67 Compare August 13, 2026 11:45

Lymah123 requested a review from mightyiam August 13, 2026 12:34

mightyiam reviewed Aug 13, 2026

View reviewed changes

Lymah123 force-pushed the chore/crane-to-crate2nix branch from 354eb67 to 76de74b Compare August 18, 2026 00:49

Lymah123 requested a review from mightyiam August 18, 2026 00:56

mightyiam reviewed Aug 18, 2026

View reviewed changes

Lymah123 force-pushed the chore/crane-to-crate2nix branch from 76de74b to f0e5ff2 Compare August 20, 2026 02:50

Lymah123 requested a review from mightyiam August 20, 2026 03:10

mightyiam reviewed Aug 20, 2026

View reviewed changes

Lymah123 force-pushed the chore/crane-to-crate2nix branch from f0e5ff2 to 2ba8532 Compare August 23, 2026 14:27

mightyiam reviewed Aug 23, 2026

View reviewed changes

Lymah123 force-pushed the chore/crane-to-crate2nix branch from 2ba8532 to 69b9bcf Compare August 23, 2026 14:44

Lymah123 requested a review from mightyiam August 23, 2026 14:50

Lymah123 force-pushed the chore/crane-to-crate2nix branch 2 times, most recently from b7e7db6 to 35e640b Compare August 24, 2026 03:30

mightyiam reviewed Aug 24, 2026

View reviewed changes

chore: switch to crate2nix

1d1b084

Lymah123 force-pushed the chore/crane-to-crate2nix branch from 35e640b to 1d1b084 Compare August 29, 2026 03:26

Lymah123 requested a review from mightyiam August 29, 2026 03:39

mightyiam reviewed Aug 29, 2026

View reviewed changes

		@@ -1,44 +1,31 @@
		use std::{io::Write, process::Command, process::Stdio};

Uh oh!

Conversation

Lymah123 commented Jul 26, 2026

Uh oh!

Lymah123 commented Jul 26, 2026

Uh oh!

mightyiam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Lymah123 commented Jul 27, 2026

Uh oh!

mightyiam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Lymah123 commented Aug 4, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mightyiam left a comment

Choose a reason for hiding this comment

Uh oh!

Lymah123 commented Aug 6, 2026

Uh oh!

Lymah123 commented Aug 6, 2026

Uh oh!

Lymah123 commented Aug 6, 2026

Uh oh!

mightyiam commented Aug 7, 2026

Uh oh!

Lymah123 commented Aug 7, 2026

Uh oh!

mightyiam commented Aug 7, 2026

Uh oh!

Lymah123 commented Aug 7, 2026

Uh oh!

mightyiam commented Aug 7, 2026

Uh oh!

Lymah123 commented Aug 7, 2026

Uh oh!

Lymah123 commented Aug 8, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lymah123 Aug 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Lymah123 commented Aug 19, 2026

Uh oh!

mightyiam left a comment

Lymah123 Aug 15, 2026 •

edited

Loading