Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nix flake check fails on crane due to IFD #612

Closed
korfuri opened this issue May 16, 2024 · 8 comments · Fixed by #641
Closed

nix flake check fails on crane due to IFD #612

korfuri opened this issue May 16, 2024 · 8 comments · Fixed by #641
Labels
bug Something isn't working

Comments

@korfuri
Copy link

korfuri commented May 16, 2024

Describe the bug

crane's flake.nix uses an IFD in the form of he inputFromLock function, with this import. This causes nix flake check to fail in some instances, if the check happens without this derivation having been built.

This causes any flake depending on crane to fail flake checks as well.

In my case I'm hitting this because I want to run nix flake check in the CI of my homelab flake that contains multiple nixosConfigurations. I don't want to build all these configurations when I'm just running CI, that would be a ton of compute for each change, so I'm running with --no-build (I also need this as I have machines with various architectures, but my CI runners can't build for all these archs). I use https://github.com/nix-community/lanzaboote which depends on crane, and so my nix flake checks are failing.

I don't understand the Rust ecosystem well enough, but it seems this pattern is only used to import a specific version of rust-overlay. Couid this instead be done without an IFD, by depending on it as a regular flake input?

Reproduction

nix gc
nix flake check --no-build github:ipetkov/crane
@korfuri korfuri added the bug Something isn't working label May 16, 2024
@ipetkov
Copy link
Owner

ipetkov commented May 18, 2024

Hi @korfuri thanks for the report!

nix flake check --no-build github:ipetkov/crane

This is actually running all of crane's test suite; to my understanding, flake checks are never run for dependencies (or even transitive dependencies), so even if the location you linked is triggering IFD, I'm not entirely certain why that would be triggered in your flake.

Could you provide a(n ideally minimal) reproduction of your flake that I can test with?

@ipetkov ipetkov added the needs reproduction Missing a flake which easily reproduces the problem label May 18, 2024
@korfuri
Copy link
Author

korfuri commented Jun 8, 2024

Apologies for the delayed response. The easiest way I can reproduce is simply:

$ nix flake init -t github:ipetkov/crane#quick-start
do you want to allow configuration setting 'extra-substituters' to be set to 'https://crane.cachix.org' (y/N)? 
do you want to permanently mark this value as untrusted (y/N)? 
warning: ignoring untrusted flake configuration setting 'extra-substituters'.
Pass '--accept-flake-config' to trust it
do you want to allow configuration setting 'extra-trusted-public-keys' to be set to 'crane.cachix.org-1:8Scfpmn9w+hGdXH/Q9tTLiYAE/2dnJYRJP7kl80GuRk=' (y/N)? 
do you want to permanently mark this value as untrusted (y/N)? 
warning: ignoring untrusted flake configuration setting 'extra-trusted-public-keys'.
Pass '--accept-flake-config' to trust it
wrote: /tmp/testflake/Cargo.toml
wrote: /tmp/testflake/flake.nix
wrote: /tmp/testflake/.gitignore
wrote: /tmp/testflake/Cargo.lock
wrote: /tmp/testflake/src/main.rs
wrote: /tmp/testflake/src
wrote: /tmp/testflake/deny.toml
$ nix flake check --no-build .
warning: Git tree '/tmp/testflake' is dirty
warning: creating lock file '/tmp/testflake/flake.lock'
warning: Git tree '/tmp/testflake' is dirty
error:
       … while checking flake output 'packages'

         at /nix/store/na7sykizsgkzh9i3wc8m8pz5xfqib2rv-source/lib.nix:39:17:

           38|               {
           39|                 ${key} = (attrs.${key} or { })
             |                 ^
           40|                   // { ${system} = ret.${key}; };

       … while checking the derivation 'packages.x86_64-linux.default'

         at /nix/store/bgrg09dihvkj0xcdpm07fviws124jnl0-source/flake.nix:115:11:

          114|         packages = {
          115|           default = my-crate;
             |           ^
          116|         } // lib.optionalAttrs (!pkgs.stdenv.isDarwin) {

       (stack trace truncated; use '--show-trace' to show the full trace)

       error: path '/nix/store/hbxnhh353vinlr7p91ajxcf3wjzwzby2-source' is not valid

It seems that literally any use of a crane-built package in a flake will trigger this when flake-checking without building (unless the necessary artifacts were built previously and not gc'd).

@ipetkov
Copy link
Owner

ipetkov commented Jun 9, 2024

Thank you for the instructions, I was able to reproduce the issue after running nix-collect-garbage!


It seems like the problem boils down to using src = craneLib.cleanCargoSource (craneLib.path ./.);. During vendoring we peek at a bunch of different files to automate as much of the configuration as possible, yet it appears that using nix flake check --no-build sadly does not materialize any paths through lib.cleanSourceWith.

Changing that line to just src = craneLib.path ./.; seems to "fix" the situation, at the expense of potential unnecessary rebuilds if irrelevant files change.


I'm going to close this as I don't think there is anything we can do here in the general sense (I'd prefer to keep the examples using cleanCargoSource to nudge users in the generally correct path), but feel free to reopen if you have additional questions!

@ipetkov ipetkov closed this as not planned Won't fix, can't repro, duplicate, stale Jun 9, 2024
@korfuri
Copy link
Author

korfuri commented Jun 9, 2024

Thanks for digging into this! I'm bringing that up with Lanzaboote since in my case they're the user of Crane and the ones bringing the IFD into my configs.

It's unfortunate but after some digging on my own side, it seems completely unavoidable: nixpkgs's cleanFilterSource eventually must call builtins.path to materialize the filtered path in the store. Using builtins.filterSource wouldn't help either, it's an IFD either way.

Maybe documenting this behavior in Crane's API reference would be useful? I guess most people don't really care about IFDs but for the few of us that are impacted this may save us some time. Thanks!

korfuri added a commit to korfuri/lanzaboote that referenced this issue Jun 9, 2024
Before this commit, we use cranelib.cleanCargoSource[1] to filter
files in the ./rust/uefi directory. This prevents some rebuilds when
files that do not contribute to the build are changed (e.g. a
README). However, this causes lanzaboote's flake.nix to contain an
IFD[2]. This seems to be unavoidable when using cleanCargoSource[3].

Why this is useful anyway:

Since `nix flake check` must check all outputs of the given flake,
it's not possible to split a flake check across multiple builder
machines. This means that `nix flake check` without `--no-build` can
only work if the builder machine has support to build for all
architectures present in that flake's `nixosConfigurations`
output. This is achievable with binfmt emulation, but excruciatingly
slow, especially for use-cases like "run nix flake check in CI". In
practice this means that a flake containing nixosConfiguration outputs
with different `system` attributes can generally only be `nix flake
check`'d with `--no-build`. See [4] for a long discussion on this.

Unfortunately, this affects flakes using Lanzaboote transitively. So
any flake containing Lanzaboote-enabled nixosConfigurations (like
mine[5]) are not be able to use `nix flake check --no-build`.

This commit corrects this by removing the IFD. This can be tested
with:

$ nix store gc   # to ensure IFD paths are not cached
$ nix flake check --no-build github:nix-community/lanzaboote  # fails
$ nix store gc
$ nix flake check --no-build github:korfuri/lanzaboote  # succeeds

[1] https://crane.dev/API.html#cranelibfiltercargosources
[2] https://nix.dev/manual/nix/2.22/language/import-from-derivation
[3] ipetkov/crane#612
[4] NixOS/nix#4265
[5] https://gitlab.com/korfuri/infra/-/blob/77635232eb65e877d8b79eb98f6264381231c20c/flake.nix
@ipetkov
Copy link
Owner

ipetkov commented Jun 10, 2024

Reopening since I remembered that cleanSourceWith makes the original source available (via origSrc) meaning it might be possible to peek at/read the original files without the filter being applied

@ipetkov
Copy link
Owner

ipetkov commented Jun 11, 2024

Still able to repro the issue, I think this check needs to be done more lazily:

if builtins.pathExists cargoToml

@ipetkov
Copy link
Owner

ipetkov commented Jun 11, 2024

Tried testing this again today and wasn't able to reproduce so it might have been a hiccup with my testing setup (constantly having to gc is not very fun).

Going to close this out for now but we can investigate further as needed

@ipetkov ipetkov closed this as completed Jun 11, 2024
@korfuri
Copy link
Author

korfuri commented Jun 12, 2024

I can confirm that for Lanzaboote it seems to work: nix store gc && nix flake check --no-build github:korfuri/lanzaboote/update-flake passes (with an updated flake input to crane's latest commit), where nix store gc && nix flake check --no-build github:nix-community/lanzaboote was failing due to IFD. Thanks a lot for figuring this out!

Note that nix flake check github:ipetkov/crane --no-build still fails due to IFD, but that's only a problem for you if you decide it is, it shouldn't affect users (at least users that use Crane in the same way as Lanzaboote does).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants