From 83615fcf8fe6095b60e33097e2d15fb1a3f56298 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 5 Jul 2021 13:09:46 +0200 Subject: [PATCH 1/5] Allow `fetchGit` to take a `name` argument Fix #3388 --- src/libfetchers/fetchers.cc | 7 ++++++- src/libfetchers/fetchers.hh | 2 ++ src/libfetchers/git.cc | 8 ++++---- tests/fetchGit.sh | 4 ++++ 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 916e0a8e8..e158d914b 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -200,12 +200,17 @@ void Input::markChangedFile( return scheme->markChangedFile(*this, file, commitMsg); } +std::string Input::getName() const +{ + return maybeGetStrAttr(attrs, "name").value_or("source"); +} + StorePath Input::computeStorePath(Store & store) const { auto narHash = getNarHash(); if (!narHash) throw Error("cannot compute store path for mutable input '%s'", to_string()); - return store.makeFixedOutputPath(FileIngestionMethod::Recursive, *narHash, "source"); + return store.makeFixedOutputPath(FileIngestionMethod::Recursive, *narHash, getName()); } std::string Input::getType() const diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index a72cfafa4..c839cf23b 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -81,6 +81,8 @@ public: std::string_view file, std::optional commitMsg) const; + std::string getName() const; + StorePath computeStorePath(Store & store) const; // Convenience functions for common attributes. diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index d8e0dbe0a..bc1930138 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -60,7 +60,7 @@ struct GitInputScheme : InputScheme if (maybeGetStrAttr(attrs, "type") != "git") return {}; for (auto & [name, value] : attrs) - if (name != "type" && name != "url" && name != "ref" && name != "rev" && name != "shallow" && name != "submodules" && name != "lastModified" && name != "revCount" && name != "narHash" && name != "allRefs") + if (name != "type" && name != "url" && name != "ref" && name != "rev" && name != "shallow" && name != "submodules" && name != "lastModified" && name != "revCount" && name != "narHash" && name != "allRefs" && name != "name") throw Error("unsupported Git input attribute '%s'", name); parseURL(getStrAttr(attrs, "url")); @@ -167,10 +167,10 @@ struct GitInputScheme : InputScheme std::pair fetch(ref store, const Input & _input) override { - auto name = "source"; - Input input(_input); + std::string name = input.getName(); + bool shallow = maybeGetBoolAttr(input.attrs, "shallow").value_or(false); bool submodules = maybeGetBoolAttr(input.attrs, "submodules").value_or(false); bool allRefs = maybeGetBoolAttr(input.attrs, "allRefs").value_or(false); @@ -270,7 +270,7 @@ struct GitInputScheme : InputScheme return files.count(file); }; - auto storePath = store->addToStore("source", actualUrl, FileIngestionMethod::Recursive, htSHA256, filter); + auto storePath = store->addToStore(input.getName(), actualUrl, FileIngestionMethod::Recursive, htSHA256, filter); // FIXME: maybe we should use the timestamp of the last // modified dirty file? diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index 88744ee7f..89294d8d2 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -189,3 +189,7 @@ path8=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$rep rev4=$(git -C $repo rev-parse HEAD) rev4_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"HEAD\"; }).rev") [[ $rev4 = $rev4_nix ]] + +# The name argument should be handled +path9=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"HEAD\"; name = \"foo\"; }).outPath") +[[ $path9 =~ -foo$ ]] From a487a652ed2f662782529d8f2bea4cf90a05e0c9 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 6 Jul 2021 08:42:47 +0200 Subject: [PATCH 2/5] allow fetchTarball to take a `name` argument --- src/libfetchers/tarball.cc | 4 ++-- tests/tarball.sh | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 257465bae..031ccc5f7 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -196,7 +196,7 @@ struct TarballInputScheme : InputScheme if (maybeGetStrAttr(attrs, "type") != "tarball") return {}; for (auto & [name, value] : attrs) - if (name != "type" && name != "url" && /* name != "hash" && */ name != "narHash") + if (name != "type" && name != "url" && /* name != "hash" && */ name != "narHash" && name != "name") throw Error("unsupported tarball input attribute '%s'", name); Input input; @@ -226,7 +226,7 @@ struct TarballInputScheme : InputScheme std::pair fetch(ref store, const Input & input) override { - auto tree = downloadTarball(store, getStrAttr(input.attrs, "url"), "source", false).first; + auto tree = downloadTarball(store, getStrAttr(input.attrs, "url"), input.getName(), false).first; return {std::move(tree), input}; } }; diff --git a/tests/tarball.sh b/tests/tarball.sh index d53ec8cd9..279ef5672 100644 --- a/tests/tarball.sh +++ b/tests/tarball.sh @@ -30,6 +30,7 @@ test_tarball() { nix-build -o $TEST_ROOT/result -E "import (fetchTree file://$tarball)" nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; })" nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })" + nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; name = \"foo\"; })" nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"sha256-xdKv2pq/IiwLSnBBJXW8hNowI4MrdZfW+SYqDQs7Tzc=\"; })" 2>&1 | grep 'NAR hash mismatch in input' nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" >&2 From e4b082a52b4cd983b50037783a647cf39cd18bdf Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 6 Jul 2021 08:43:06 +0200 Subject: [PATCH 3/5] allow fetchMercurial to take a `name` argument --- src/libexpr/primops/fetchMercurial.cc | 1 + src/libfetchers/mercurial.cc | 8 ++++---- tests/fetchMercurial.sh | 5 +++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index 4830ebec3..3f88ccb91 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -62,6 +62,7 @@ static void prim_fetchMercurial(EvalState & state, const Pos & pos, Value * * ar fetchers::Attrs attrs; attrs.insert_or_assign("type", "hg"); attrs.insert_or_assign("url", url.find("://") != std::string::npos ? url : "file://" + url); + attrs.insert_or_assign("name", name); if (ref) attrs.insert_or_assign("ref", *ref); if (rev) attrs.insert_or_assign("rev", rev->gitRev()); auto input = fetchers::Input::fromAttrs(std::move(attrs)); diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 0eb401e10..efb4ee8db 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -74,7 +74,7 @@ struct MercurialInputScheme : InputScheme if (maybeGetStrAttr(attrs, "type") != "hg") return {}; for (auto & [name, value] : attrs) - if (name != "type" && name != "url" && name != "ref" && name != "rev" && name != "revCount" && name != "narHash") + if (name != "type" && name != "url" && name != "ref" && name != "rev" && name != "revCount" && name != "narHash" && name != "name") throw Error("unsupported Mercurial input attribute '%s'", name); parseURL(getStrAttr(attrs, "url")); @@ -147,10 +147,10 @@ struct MercurialInputScheme : InputScheme std::pair fetch(ref store, const Input & _input) override { - auto name = "source"; - Input input(_input); + auto name = input.getName(); + auto [isLocal, actualUrl_] = getActualUrl(input); auto actualUrl = actualUrl_; // work around clang bug @@ -193,7 +193,7 @@ struct MercurialInputScheme : InputScheme return files.count(file); }; - auto storePath = store->addToStore("source", actualUrl, FileIngestionMethod::Recursive, htSHA256, filter); + auto storePath = store->addToStore(input.getName(), actualUrl, FileIngestionMethod::Recursive, htSHA256, filter); return { Tree(store->toRealPath(storePath), std::move(storePath)), diff --git a/tests/fetchMercurial.sh b/tests/fetchMercurial.sh index d8a4e09d2..726840664 100644 --- a/tests/fetchMercurial.sh +++ b/tests/fetchMercurial.sh @@ -94,3 +94,8 @@ hg commit --cwd $repo -m 'Bla3' path4=$(nix eval --impure --refresh --raw --expr "(builtins.fetchMercurial file://$repo).outPath") [[ $path2 = $path4 ]] + +echo paris > $repo/hello +# Passing a `name` argument should be reflected in the output path +path5=$(nix eval -vvvvv --impure --refresh --raw --expr "(builtins.fetchMercurial { url = \"file://$repo\"; name = \"foo\"; } ).outPath") +[[ $path5 =~ -foo$ ]] From 2c8240677edb94f17cc0bcd681e442c46b987013 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 6 Jul 2021 08:43:19 +0200 Subject: [PATCH 4/5] allow fetchFromGitHub to take a `name` argument --- src/libfetchers/github.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 8352ef02d..298c05f9a 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -207,7 +207,7 @@ struct GitArchiveInputScheme : InputScheme auto url = getDownloadUrl(input); - auto [tree, lastModified] = downloadTarball(store, url.url, "source", true, url.headers); + auto [tree, lastModified] = downloadTarball(store, url.url, input.getName(), true, url.headers); input.attrs.insert_or_assign("lastModified", uint64_t(lastModified)); From 7e5c79a2d210e7b986803d5fe5c7ebb504b3e74e Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 8 Jul 2021 13:29:58 +0200 Subject: [PATCH 5/5] Forbid the `name` attribute for fetchTree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need to support it for the “old” fetch* functions for backwards compatibility, but we don’t need it for fetchTree (as it’s a new function). Given that changing the `name` messes-up the content hashing, we can just forbid passing a custom `name` argument to it --- src/libexpr/primops/fetchTree.cc | 7 +++++++ tests/tarball.sh | 6 +++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index b8b99d4fa..472efe0ee 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -119,6 +119,13 @@ static void fetchTree( .errPos = pos }); + if (auto nameIter = attrs.find("name"); nameIter != attrs.end()) + throw Error({ + .msg = hintfmt("attribute 'name' isn’t supported in call to 'fetchTree'"), + .errPos = pos + }); + + input = fetchers::Input::fromAttrs(std::move(attrs)); } else { auto url = fixURI(state.coerceToString(pos, *args[0], context, false, false), state); diff --git a/tests/tarball.sh b/tests/tarball.sh index 279ef5672..1301922a5 100644 --- a/tests/tarball.sh +++ b/tests/tarball.sh @@ -30,7 +30,6 @@ test_tarball() { nix-build -o $TEST_ROOT/result -E "import (fetchTree file://$tarball)" nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; })" nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })" - nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; name = \"foo\"; })" nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"sha256-xdKv2pq/IiwLSnBBJXW8hNowI4MrdZfW+SYqDQs7Tzc=\"; })" 2>&1 | grep 'NAR hash mismatch in input' nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" >&2 @@ -41,6 +40,11 @@ test_tarball() { (! nix-instantiate --eval -E ' 1' -I fnord=file://no-such-tarball$ext) nix-instantiate --eval -E '' -I fnord=file://no-such-tarball$ext -I fnord=. + + # Ensure that the `name` attribute isn’t accepted as that would mess + # with the content-addressing + (! nix-instantiate --eval -E "fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; name = \"foo\"; }") + } test_tarball '' cat