Skip to content

buildRustPackage has excessive dependency rebuilds in checkPhase for unclear reasons #291222

@thoughtpolice

Description

@thoughtpolice

Problem description

Jujutsu is a Rust project that has a Nix flake in its repository. The flake is meant to provide a convenient nix profile install path, and is built using this Nix expression at its core. This is not a complicated expression; it runs both cargo build and cargo test, as buildRustPackage is meant to do.

At the moment, our CI system just runs nix build . on every commit, because the build naturally runs the tests, too, which is ideal. However, at the moment, it effectively rebuilds the whole project twice, once for test, and once for the release binaries, even with the same profile. The build log is included below. The manual reference says:

When using buildRustPackage, the checkPhase is enabled by default and runs cargo test on the package to build. To make sure that we don't compile the sources twice and to actually test the artifacts that will be used at runtime, the tests will be ran in the release mode by default.
https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/rust.section.md#running-package-tests-running-package-tests

This seems like a strange regression or edge case, of some kind. Or maybe I just don't understand how buildRustPackage works; that's also quite possible.

In short, this pretty much bloats our nix build time by 2x more than necessary. You can see in the full build log that the cargo build --release phase took 2m51s, the cargo test build took 3m7s (because it compiled everything again + a little extra for the tests), and the test suite itself took only a minute or so.

This is a pretty severe impediment to our CI times and it makes Nix our slowest CI build by far. To stem the immediate bleeding, I created a PR that will:

  • Separate out a new Flake check, used by CI, which is
  • An .override of the main expression, setting build buildPhase = "true"; and also,
  • Setting the build profile to test to cut down on build times, on top of all that.

These changes halve the amount of time our Continuous Integration takes, bringing it very close to normal Linux builds. Honestly, just setting the test profile is valuable enough for our CI times that keeping a nix flake check separate from the build is probably worth it.

However, while this patches up the CI build, which now uses flake check, the ordinary nix build still suffers from this flaw in our setup, so users who nix profile install (like me!) will still need to wait an extra 3+ minutes to upgrade their build.

Leading theory

When I was debugging this for 5 minutes this week, I think a part of the problem is that, for some reason, the cargo build step has several environment variable set like HOST_CC and HOST_CXX when the command is run, while these aren't applied to the cargo test command. When I looked at the debugging output, it seemed like Cargo is probably rebuilding stuff for reasons like that, because it has an impact on any foreign libraries with build.rs scripts that might need to interrogate the host compiler (and we have several crates that use C libraries and compile them on the fly, it's super common obviously.)

I haven't investigated this any further, but this is basically my leading theory, right now.

Ideal behavior

I would like our normal jujutsu expression to not rebuild everything twice when nix build is run.

Notes

Metadata

Metadata

Assignees

No one assigned

    Labels

    0.kind: bugSomething is broken6.topic: rustGeneral-purpose programming language emphasizing performance, type safety, and concurrency.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions