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
Use Nix-builtin fetchurl where possible #41147
Use Nix-builtin fetchurl where possible #41147
Conversation
It seems @GrahamcOfBorg is failing on a derivation that's already failing on master (ran this locally). |
…iltin-fetchurl
Could we do some sort of feature detection for this? Like checking whether the new fetchurl is available to retain compatibility with old Nix? |
18f4edd
to
2a5e412
Compare
That's a good idea. I've added an additional check for the Nix version. I wasn't able to find a good way to check for the |
That makes sense. I think that's the only proper way to do it. Also:
Can we implement this in Nixpkgs? It should just be some string manipulation and a lookup of mirrors.nix.
I'm not sure I understand this. I think we will also have to rewrite builders that do use NIX_CURL_FLAGS though: https://github.com/NixOS/nixpkgs/search?q=NIX_CURL_FLAGS
I think this is okay for now. nix-build never had good progress support anyway & as long as nix build progress works I'm fine with that. |
I don't understand what private files have to do with fetchurl usage in nixpkgs. What's wrong with the statusquo exactly? |
Note that there is unfortunately no way to actually check if |
I don't think we can handle 404's from outside the derivation, but maybe I'm misunderstanding this part? EDIT: I think we probably need to add support for mirrors inside Nix itself, however we can already take the first site from the mirrors to make most fetches use the builtin method.
Yes, that's true. We also need to add options to builtin:fetchurl to support the use-cases of those builders. However, another part is that you can pass I think that is the right way to go though, because
The documentation mentions to use The most popular way to get a netrc-file used by One problem is that people need to pass it to their command-line tool as well as Another problem is that That is why I think it would be nice to move more towards using the methods in Nix to fetch HTTP(S) resources. That way we can make this process less vague for the user, the documentation more consistent and the builds more secure.
Hmm, good point. I ran into |
But none of the fetchurl calls in nixpkgs itself should require authorization. I get why you might want to use bultins:fetchurl or even builtins.fetchurl (which operates at eval time, not build time) in a private project, but what does it matter to nixpkgs? |
That's true, but A global netrc file would help quite a lot in this regard. It will work for private Nix imports, private cache as well as private src. A lot easier to grasp imo. |
Aren’t there other benefits besides private files? For instance:
|
Yeah I'd be excited to get rid of |
Shall I make removal of fetchurlBoot part of this PR, or would it be better to create a separate PR afterwards? Edit: just remembered, the fetchurlBoot does work with mirrors as it will just take the first entry and use that one. So we cannot replace fetchurlBoot with the implementation in this PR yet. |
What can I do to get this moving forward? |
This is awesome. I'd love to see this merged, if only for the ability to see download progress with Nix 2.0. |
@matthewbauer @shlevy @Ericson2314 @edolstra What can I do to move this forward? |
There are a few things missing in builtin fetchurl:
|
@edolstra Indeed, for cases where mirrors are used, curl is still being used as a fallback: nixpkgs/pkgs/build-support/fetchurl/default.nix Lines 55 to 67 in 2a5e412
Would you say these features need to be implemented in Nix before moving over using fallbacks like this PR does? |
So far I can't see benefits, except slightly more cleaner in some cases:
isn't it already a case? Or would it make
you've shown already it doesn't
because this is a partial Also, if it gets merged, it will deprecate things like So, maybe it would make more sense to replace @bobvanderlinden @matthewbauer what do you think? |
I was thinking that eventually we want all url fetches to be done by Nix itself, instead of the curl cmdline. We still need to support some features of the curl solution, so until those are implemented in Nix, curl will need to be used as fallback. This PR was a first step towards gradually migrating all url fetches to Nix. If 1 curl fetch spoils your build, it is because it is using features not supported by Nix. Explicit usage of builtins would be cool, but a lot of helperfunctions are already referring to the nixpkgs fetchurl. This PR isn't intended to be a really good and clean solution, but just a way to migrate over to full builtins.fetchurl coverage. As for the download progress (correct me if I'm wrong, because I haven't tried this is a while), it reports downloads using Nix fetchurl differently than using curl. Currently fetches mostly reported as builds, whereas fetches for Nix files and the lines are reported as downloads. It would be nice to have all fetches be reported the same. This PR isn't a full solution for that, but it is a step toward that goal. That said, I was only making this PR because of frustration regarding half-functioning netrc file for Nix and thereby the inability to fetch private files needed to set up company environments. This is one of the points blocking potential adoption at my workplace. I cannot 'sell' Nix if it doesn't have a consistent answer for fetching private files. |
It would definitely be better if fetches are done by Nix rather than by builders, since then (one day) we can remove the ability of fixed-output derivations to fetch things from the network and thereby be arbitrarily irreproducible (NixOS/nix#2270). |
I've added a trace to find out how many fetchurl calls are non-standard when building my system:
Pretty much. Please review patch, maybe I've did it wrong:
EDIT: fix bug in script |
alright, I've fixed a bug in patch above, here's new result:
Out of ~1800 fetchurl calls only 337 will be replaced with builtin fetchurl. So, I'd say, this PR is too much ahead of time. I wonder also if it's possible to add netrc for fetchurl via overlay. |
@danbst That's a really good idea! It's unfortunate so little fetches will use Nix this way. Your statistics do show what has the biggest impact to tackle to get Nix fetch dependencies. Apart from those issues, I looked into getting netrc into fetchurl via overlay, but it all felt confusing and dirty to do it that way. I thought I ran into the solution that would force me to allow a sandbox to access my netrc file. As it includes really private information, that wasn't something I'd like to recommend to coworkers. That said, I'm still a bit hesitant to just close this PR, as it does give some more motivation to implement (for instance) mirror support in Nix. |
Closing this one because it probably will need more work on Nix itself because this PR is a viable option. |
@danbst we need to be careful with interpreting those kind of results. |
Motivation for this change
Nixpkgs uses the
fetchurl
function quite a lot. That function uses Curl binary which runs inside a build sandbox. This requires the netrc file required to download private resources to be exposed inside of the sandbox. This was mentioned in #41000.I was interested how viable it would be to use the Nix-builtin fetchurl instead of using the Curl binary.
This PR changes the way
fetchurl
works by usingfetchurl
from Nix by default, but fallback to the Curl variant when arguments are being used that are not supported by the Nix fetchurl.The original
fetchurl/default.nix
was renamed tofetchurl/curl.nix
.fetchurl/default.nix
uses a derivation withbuilder = "builtins:fetchurl"
, similar tofetchurl/boot.nix
.fetchurl/default.nix
has the same signature asfetchurl/curl.nix
, but will check for incompatible arguments to see whether it should fallback tofetchurl/curl.nix
.Consider this PR a work in progress and a way to gauge interest/motivation/problems/compatiblity.
The problems that I anticipate are:
fetchurl
calls fallback tocurl
.NIX_CURL_FLAGS
will be broken for those calls that do use Nix.nix-build
does not show progress of downloads.Things done
nix-build -A nginx
andnix-build -A mysql
.build-use-sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)