Skip to content
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

Add a name attribute to the fetchers #4988

Merged
merged 5 commits into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libexpr/primops/fetchMercurial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
7 changes: 7 additions & 0 deletions src/libexpr/primops/fetchTree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 6 additions & 1 deletion src/libfetchers/fetchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/libfetchers/fetchers.hh
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ public:
std::string_view file,
std::optional<std::string> commitMsg) const;

std::string getName() const;

StorePath computeStorePath(Store & store) const;

// Convenience functions for common attributes.
Expand Down
8 changes: 4 additions & 4 deletions src/libfetchers/git.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -167,10 +167,10 @@ struct GitInputScheme : InputScheme

std::pair<Tree, Input> fetch(ref<Store> 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);
Expand Down Expand Up @@ -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?
Expand Down
2 changes: 1 addition & 1 deletion src/libfetchers/github.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
8 changes: 4 additions & 4 deletions src/libfetchers/mercurial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -147,10 +147,10 @@ struct MercurialInputScheme : InputScheme

std::pair<Tree, Input> fetch(ref<Store> 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

Expand Down Expand Up @@ -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)),
Expand Down
4 changes: 2 additions & 2 deletions src/libfetchers/tarball.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -226,7 +226,7 @@ struct TarballInputScheme : InputScheme

std::pair<Tree, Input> fetch(ref<Store> 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};
}
};
Expand Down
4 changes: 4 additions & 0 deletions tests/fetchGit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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$ ]]
5 changes: 5 additions & 0 deletions tests/fetchMercurial.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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$ ]]
5 changes: 5 additions & 0 deletions tests/tarball.sh
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ test_tarball() {
(! nix-instantiate --eval -E '<fnord/xyzzy> 1' -I fnord=file://no-such-tarball$ext)

nix-instantiate --eval -E '<fnord/config.nix>' -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
Expand Down