Skip to content

nix flake check: Skip substitutable derivations#13998

Merged
Mic92 merged 1 commit intoNixOS:masterfrom
Mic92:fast-flake-check
Sep 15, 2025
Merged

nix flake check: Skip substitutable derivations#13998
Mic92 merged 1 commit intoNixOS:masterfrom
Mic92:fast-flake-check

Conversation

@Mic92
Copy link
Copy Markdown
Member

@Mic92 Mic92 commented Sep 15, 2025

This is a rebased version of #13574 Check the previous discussion there.

Since nix flake check doesn't produce a result symlink, it doesn't actually need to build/substitute derivations that are already known to have succeeded, i.e. that are substitutable.

This can speed up CI jobs in cases where the derivations have already been built by other jobs. For instance, a command like

nix flake check github:NixOS/hydra/aa62c7f7db31753f0cde690f8654dd1907fc0ce2

should no longer build anything because the outputs are already in cache.nixos.org.

Based-on: DeterminateSystems#134
Based-on: https://gerrit.lix.systems/c/lix/+/3841

Motivation

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@Mic92 Mic92 requested a review from edolstra as a code owner September 15, 2025 16:24
@github-actions github-actions bot added documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority labels Sep 15, 2025
@Mic92
Copy link
Copy Markdown
Member Author

Mic92 commented Sep 15, 2025

Looking the binary cache download statistics for the github ASN, I would like to see this in - it saves money and time for everyone. It has been now tested for a while (and is not a very complex change) in the other two implementations.

@Ericson2314
Copy link
Copy Markdown
Member

Per #13574 (comment) and the following, if we can have some sort of "todo remove" in the code, then I am good with this.

@Mic92 Mic92 force-pushed the fast-flake-check branch 2 times, most recently from 13f9fa8 to 5febd81 Compare September 15, 2025 17:39
@Mic92 Mic92 enabled auto-merge September 15, 2025 17:39
Since `nix flake check` doesn't produce a `result` symlink, it doesn't
actually need to build/substitute derivations that are already known
to have succeeded, i.e. that are substitutable.

This can speed up CI jobs in cases where the derivations have already
been built by other jobs. For instance, a command like

  nix flake check github:NixOS/hydra/aa62c7f7db31753f0cde690f8654dd1907fc0ce2

should no longer build anything because the outputs are already in
cache.nixos.org.

Based-on: DeterminateSystems#134
Based-on: https://gerrit.lix.systems/c/lix/+/3841
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
@Mic92 Mic92 merged commit 5e17a3f into NixOS:master Sep 15, 2025
14 checks passed
@Mic92 Mic92 deleted the fast-flake-check branch September 16, 2025 06:40
// not actually produce the outputs.
auto missing = store->queryMissing(drvPaths);
// Only occurs if `drvPaths` contains a `DerivedPath::Opaque`, which should never happen
assert(missing.unknown.empty());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This assert breaks nix flake check in certain cases, not sure why...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Mic92 Can you flesh out the thinking here about why it shouldn't ever happen / what it implies if it does?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ref: #14482

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.

Missed that line when reviewing the original pull request.

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

Labels

documentation new-cli Relating to the "nix" command with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants