ipetkov / crane

A Nix library for building cargo projects. Never build twice thanks to incremental artifact caching.
https://crane.dev
MIT License
962 stars 92 forks source link

`nix flake check` fails on crane due to IFD #612

Closed korfuri closed 5 months ago

korfuri commented 6 months ago

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
ipetkov commented 6 months ago

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?

korfuri commented 5 months ago

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 commented 5 months ago

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!

korfuri commented 5 months ago

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!

ipetkov commented 5 months ago

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 commented 5 months ago

Still able to repro the issue, I think this check needs to be done more lazily: https://github.com/ipetkov/crane/blob/a3f0c63eed74a516298932b9b1627dd80b9c3892/lib/crateNameFromCargoToml.nix#L23

ipetkov commented 5 months ago

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

korfuri commented 5 months ago

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).