edolstra / flake-compat

MIT License
251 stars 71 forks source link

Importing a remote package using flake-compat is now broken #60

Closed basile-henry closed 1 year ago

basile-henry commented 1 year ago

I am not entirely sure if this is an issue with hydra's use of flake-compat or a flake-compat issue, but it seems to me like the following way of importing a nix package that uses flake-compat should remain valid. At least I expect I am not the only one to import nix packages this way so I expect this to break other peoples builds as well.

I have a dependency on NixOS/hydra (using niv) which under the hood uses flake-compat: https://github.com/NixOS/hydra/blob/c1a5ff3959f8d3d0f13295d29a02084e14dff735/default.nix#L4-L6

In order to simplify the reproduction of the issue, we can get niv out of the way and simply use fetchTarball.

This used to work:

nix-build -E 'import (fetchTarball https://github.com/NixOS/hydra/archive/master.tar.gz)'

But it now breaks:

$ nix-build -E 'import (fetchTarball https://github.com/NixOS/hydra/archive/master.tar.gz)'
this derivation will be built:
  /nix/store/z0dgrm7wykms253zcnzjq4skc97b08aa-hydra-0.1.19700101.DIRTY.drv
building '/nix/store/z0dgrm7wykms253zcnzjq4skc97b08aa-hydra-0.1.19700101.DIRTY.drv'...
unpacking sources
unpacking source archive /nix/store/xwvjm5dxj60h6js47la34jv6xshr4rh1-source
do not know how to unpack source archive /nix/store/xwvjm5dxj60h6js47la34jv6xshr4rh1-source
error: builder for '/nix/store/z0dgrm7wykms253zcnzjq4skc97b08aa-hydra-0.1.19700101.DIRTY.drv' failed with exit code 1;
       last 3 log lines:
       > unpacking sources
       > unpacking source archive /nix/store/xwvjm5dxj60h6js47la34jv6xshr4rh1-source
       > do not know how to unpack source archive /nix/store/xwvjm5dxj60h6js47la34jv6xshr4rh1-source
       For full logs, run 'nix log /nix/store/z0dgrm7wykms253zcnzjq4skc97b08aa-hydra-0.1.19700101.DIRTY.drv'.

I believe the commit that introduces the issue is https://github.com/edolstra/flake-compat/commit/bc5e257a8d0c4df04652ecff9053d05b0dc9484e

I have forked NixOS/hydra and modified the fetchTarball of flake-compat to a specific commit to showcase when it used to work and what it looks like now/on that first broken commit:

Works:

nix-build -E 'import (fetchTarball https://github.com/basile-henry/hydra/archive/last-working-flake-compat.tar.gz)'

Broken:

nix-build -E 'import (fetchTarball https://github.com/basile-henry/hydra/archive/first-broken-flake-compat.tar.gz)'
basile-henry commented 1 year ago

As a temporary fix for my dependency on hydra, I can inline their default.nix (I'm guessing this avoids the ./. in there which could be ambiguous):

nix-build -E '(import (fetchTarball https://github.com/edolstra/flake-compat/archive/master.tar.gz) { src = fetchTarball https://github.com/NixOS/hydra/archive/master.tar.gz; }).defaultNix'

(this works with niv to fetch the source as well)

edolstra commented 1 year ago

Weird. The cause appears to be that

outPath = sourceInfo + ((if subdir == "" then "" else "/") + subdir);

seems to lose the dependency (string content) on sourceInfo.outPath (e.g. /nix/store/xwvjm5dxj60h6js47la34jv6xshr4rh1-source above), so the generated derivation source not have its src attribute in the inputSrcs. Hence why the build does succeed if you pass --no-sandbox. This looks like a Nix bug because it shouldn't lose track of a dependency.

Changing that line to

outPath = sourceInfo.outPath + ((if subdir == "" then "" else "/") + subdir);

makes it work, although that results in double copying (e.g. /nix/store/rs0gf70y4h7c8ffmv3cfrhmsvzqd02wf-xvyczslhxf5zm12df5s8csijkbdrs7i0-source). Even better is

outPath = (builtins.storePath sourceInfo) + ((if subdir == "" then "" else "/") + subdir);

since that avoids the double copy.

basile-henry commented 1 year ago

Great! :smile: Thanks for the quick fix and explanation.

I did not even think that it could be a nix/sandbox thing :sweat_smile:

lovesegfault commented 1 year ago

I think this fix broke other things, since my CI is failing after picking-up this change: https://github.com/lovesegfault/nix-config/pull/3014

edolstra commented 1 year ago

@lovesegfault I've tweaked my fix a bit, does it work now?

lovesegfault commented 1 year ago

@edolstra Yup! CI passed and auto-merged. Thanks for the swift fix!