|
This reverts commit 35eec921af1043fc6322edc0ad88c872d41623b8.
Reason for revert: Regressed nix-eval-jobs, and it appears to be this change is buggy/missing a case. It just needs another pass.
Code causing the problem in n-e-j, when invoked with `nix-eval-jobs --flake '.#hydraJobs'`:
```
n-e-j/tests/assets ยป ../../build/src/nix-eval-jobs --meta --workers 1 --flake .#hydraJobs
warning: unknown setting 'trusted-users'
warning: `--gc-roots-dir' not specified
error: unsupported Git input attribute 'dir'
error: worker error: error: unsupported Git input attribute 'dir'
```
```
nix::Value *vRoot = [&]() {
if (args.flake) {
auto [flakeRef, fragment, outputSpec] =
nix::parseFlakeRefWithFragmentAndExtendedOutputsSpec(
args.releaseExpr, nix::absPath("."));
nix::InstallableFlake flake{
{}, state, std::move(flakeRef), fragment, outputSpec,
{}, {}, args.lockFlags};
return flake.toValue(*state).first;
} else {
return releaseExprTopLevelValue(*state, autoArgs, args);
}
}();
```
Inspecting the program behaviour reveals that `dir` was in fact set in the URL going into the fetcher. This is in turn because unlike in the case changed in this commit, it was not erased before handing it to libfetchers, which is probably just a mistake.
```
(rr) up
3 0x00007ffff60262ae in nix::fetchers::Input::fromURL (url=..., requireTree=requireTree@entry=true) at src/libfetchers/fetchers.cc:39
warning: Source file is more recent than executable.
39 auto res = inputScheme->inputFromURL(url, requireTree);
(rr) p url
$1 = (const nix::ParsedURL &) @0x7fffdc874190: {url = "git+file:///home/jade/lix/nix-eval-jobs",
base = "git+file:///home/jade/lix/nix-eval-jobs", scheme = "git+file", authority = std::optional<std::string> = {[contained value] = ""},
path = "/home/jade/lix/nix-eval-jobs", query = std::map with 1 element = {["dir"] = "tests/assets"}, fragment = ""}
(rr) up
4 0x00007ffff789d904 in nix::parseFlakeRefWithFragment (url=".#hydraJobs", baseDir=std::optional<std::string> = {...},
allowMissing=allowMissing@entry=false, isFlake=isFlake@entry=true) at src/libexpr/flake/flakeref.cc:179
warning: Source file is more recent than executable.
179 FlakeRef(Input::fromURL(parsedURL, isFlake), getOr(parsedURL.query, "dir", "")),
(rr) p parsedURL
$2 = {url = "git+file:///home/jade/lix/nix-eval-jobs", base = "git+file:///home/jade/lix/nix-eval-jobs", scheme = "git+file",
authority = std::optional<std::string> = {[contained value] = ""}, path = "/home/jade/lix/nix-eval-jobs", query = std::map with 1 element = {
["dir"] = "tests/assets"}, fragment = ""}
(rr) list
174
175 if (pathExists(flakeRoot + "/.git/shallow"))
176 parsedURL.query.insert_or_assign("shallow", "1");
177
178 return std::make_pair(
179 FlakeRef(Input::fromURL(parsedURL, isFlake), getOr(parsedURL.query, "dir", "")),
180 fragment);
181 }
```
Change-Id: Ib55a882eaeb3e59228857761dc1e3b2e366b0f5e
|
|
The original idea was to fix lix#174, but for a user friendly solution,
I figured that we'd need more consistency:
* Invalid query params will cause an error, just like invalid
attributes. This has the following two consequences:
* The `?dir=`-param from flakes will be removed before the URL to be
fetched is passed to libfetchers.
* The tarball fetcher doesn't allow URLs with custom query params
anymore. I think this was questionable anyways given that an
arbitrary set of query params was silently removed from the URL you
wanted to fetch. The correct way is to use an attribute-set
with a key `url` that contains the tarball URL to fetch.
* Same for the git & mercurial fetchers: in that case it doesn't even
matter though: both fetchers added unused query params to the URL
that's passed from the input scheme to the fetcher (`url2` in the code).
It turns out that this was never used since the query parameters were
erased again in `getActualUrl`.
* Validation happens for both attributes and URLs. Previously, a lot of
fetchers validated e.g. refs/revs only when specified in a URL and
the validity of attribute names only in `inputFromAttrs`.
Now, all the validation is done in `inputFromAttrs` and `inputFromURL`
constructs attributes that will be passed to `inputFromAttrs`.
* Accept all attributes as URL query parameters. That also includes
lesser used ones such as `narHash`.
And "output" attributes like `lastModified`: these could be declared
already when declaring inputs as attribute rather than URL. Now the
behavior is at least consistent.
Personally, I think we should differentiate in the future between
"fetched input" (basically the attr-set that ends up in the lock-file)
and "unfetched input" earlier: both inputFrom{Attrs,URL} entrypoints
are probably OK for unfetched inputs, but for locked/fetched inputs
a custom entrypoint should be used. Then, the current entrypoints
wouldn't have to allow these attributes anymore.
Change-Id: I1be1992249f7af8287cfc37891ab505ddaa2e8cd
|
