Rewrite builtin:fetchurl output paths #1805
Conversation
This was failing when it needed redirected outputs, such as when calling a build with --check on a fixed-output derivation that used the builtin fetchurl. Fixes NixOS#1803
| builtinFetchurl(*drv, netrcData); | ||
| if (drv->builder == "builtin:fetchurl") { | ||
| /* fetchurl might need to rewrite its outputs, so let's pass in a closure to rewrite anythings */ | ||
| auto rewriter = [&](const std::string & in) { return rewriteStrings(in, inputRewrites); }; |
There was a problem hiding this comment.
The string rewriting support is all in build.cc so I didn't want to move the typedef for InputRewrites to somewhere common, when all I really needed was a string -> string function in builtinFetchurl.
|
Argh, I swear this was working and now it isn't. Edit: I think it was; |
| @@ -4,6 +4,6 @@ | |||
|
|
|||
| namespace nix { | |||
|
|
|||
| void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData); | |||
| void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData, std::function<std::string(const std::string &)> rewriteStrings); | |||
There was a problem hiding this comment.
I think it would be cleaner to rewrite strings in 'drv' prior to calling the builtin. That way all (future) builtin derivations benefit.
There was a problem hiding this comment.
@edolstra agree, and I wanted to do that but I didn't feel comfortable changing the working of the main codepath, since all the state machine and branching stuff in this file makes it very hard to understand (or feel confident that you really grok it).
There was a problem hiding this comment.
My inclination would be to remove env altogether (it's only used in a couple of places and is just another value that can get out of sync), and add the default environment values (as well as rewritten paths) to drv->env.
|
@edolstra any chance we can just put this in and then clean up the rewriting logic in a subsequent commit? This is just broken right now and I don't really know how to fix it more cleanly. |
|
Thanks! 😄 |
This was failing when it needed redirected outputs, such as when calling a build with --check on a fixed-output derivation that used the builtin fetchurl.
Fixes #1803
cc @edolstra @shlevy @domenkozar