NixOS/nix

flakes: Locking on effective urls breaks fetching github releases (and probably other things)

johnae opened this issue · 7 comments

Describe the bug

Before #4595 was merged we used flakes to lock and fetch github releases. Github uses redirects to so-called signed urls which expire after some time. This means that any flake.lock will, after the merge of #4595, contain unusable urls.

The intention of the merged pull request seems to have been to enable locking of nixos channels which ALSO uses redirects but for a different purpose. I see how that might be useful but I still question the point of enabling the use of nixos channels when flakes are supposed to replace them in the end.

In any case - that merge broke many of of our projects so that we cannot update flake inputs anymore unless we use an older version of Nix.

Steps To Reproduce

  1. Use a Nix version after pull request 4595 was merged
  2. Add an input to a github release, for example:
{
   inputs.fluxcd-x86_64-linux = {
    url = "https://github.com/fluxcd/flux2/releases/download/v0.10.0/flux_0.10.0_linux_amd64.tar.gz";
    flake = false;
  };
}
  1. Lock that input by running nix flake update.
  2. See how the locked url is actually a signed expiring url

Expected behavior

The old behavior of NOT following the redirect.

nix-env --version nix-env (Nix) 2.4pre20210317_8a5203d

As the author of #4595, I was unaware of this situation and I think it is fine to revert it

fogti commented

Would it be possible to introduce a switch (e.g. similiar to .flake = false) which would optionally enable the "effective url locking" behavoir?

I don't think it's worth it, considering that using redirect to work with channels is pretty niche, and it's also surprising.

Any other use case for locking on the redirect destination?

fogti commented

I don't know of any other case... and working with channels in flakes is probably also rather niche...

L-as commented

You could have a channel type instead of having some switch, which would work like tarball except have the functionality of the PR.

I'm leaning on not implementing this feature now and reverting #4595

cc @edolstra, who originally merged #4595. What do you think? Sorry for the noise...

Thanks, I've reverted it.