Skip to content

Commit

Permalink
Merge legacy fetchGit-builtin with the generic fetchTree-function
Browse files Browse the repository at this point in the history
The original idea was to implement a git-fetcher in Nix's core that
supports content hashes[1]. In NixOS#3549[2] it has been suggested to
actually use `fetchTree` for this since it's a fairly generic wrapper
over the new fetcher-API[3] and already supports content-hashes.

This patch implements a new git-fetcher based on `fetchTree` by
incorporating the following changes:

* Removed the original `fetchGit`-implementation and replaced it with an
  alias on the `fetchTree` implementation.

* Ensured that the `git`-fetcher from `libfetchers` always computes a
  content-hash and returns an "empty" revision on dirty trees (the
  latter one is needed to retain backwards-compatibility).

* The hash-mismatch error in the fetcher-API exits with code 102 as it
  usually happens whenever a hash-mismatch is detected by Nix.

* Removed the `flakes`-feature-flag: I didn't see a reason why this API
  is so tightly coupled to the flakes-API and at least `fetchGit` should
  remain usable without any feature-flags.

* It's only possible to specify a `narHash` for a `git`-tree if either a
  `ref` or a `rev` is given[4].

* It's now possible to specify an URL without a protocol. If it's missing,
  `file://` is automatically added as it was the case in the original
  `fetchGit`-implementation.

[1] NixOS#3216
[2] NixOS#3549 (comment)
[3] NixOS#3459
[4] NixOS#3216 (comment)
  • Loading branch information
Ma27 committed May 8, 2020
1 parent d3d8186 commit cd91739
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 98 deletions.
85 changes: 0 additions & 85 deletions src/libexpr/primops/fetchGit.cc

This file was deleted.

79 changes: 71 additions & 8 deletions src/libexpr/primops/fetchTree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ void emitTreeAttrs(
mkString(*state.allocAttr(v, state.symbols.create("narHash")),
tree.info.narHash.to_string(SRI));

if (input->type() == "git")
mkBool(*state.allocAttr(v, state.symbols.create("submodules")), input->fetchSubmodules());

if (input->getRev()) {
mkString(*state.allocAttr(v, state.symbols.create("rev")), input->getRev()->gitRev());
mkString(*state.allocAttr(v, state.symbols.create("shortRev")), input->getRev()->gitShortRev());
Expand All @@ -40,10 +43,40 @@ void emitTreeAttrs(
v.attrs->sort();
}

static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, Value & v)
std::string fixURI(std::string uri, EvalState &state)
{
state.checkURI(uri);
return uri.find("://") != std::string::npos ? uri : "file://" + uri;
}

void addURI(EvalState &state, fetchers::Attrs &attrs, Symbol name, std::string v)
{
string n(name);
attrs.emplace(name, n == "url" ? fixURI(v, state) : v);
}

void validateFetcherAttrs(fetchers::Attrs attrs, const Pos &pos)
{
settings.requireExperimentalFeature("flakes");
if (!attrs.count("type"))
throw Error("attribute 'type' is missing in call to 'fetchTree', at %s", pos);

if (getStrAttr(attrs, "type") == "git") {
auto narHash = maybeGetStrAttr(attrs, "narHash");
auto ref = maybeGetStrAttr(attrs, "ref");
auto rev = maybeGetStrAttr(attrs, "rev");

if (narHash && !ref && !rev)
throw EvalError(format("attribute 'narHash' can only be used for 'git' if either 'ref' or 'rev' is set, at %1%!") % pos);
}
}

static void fetchTree(
EvalState &state,
const Pos &pos,
Value **args,
Value &v,
const std::optional<std::string> type
) {
std::shared_ptr<const fetchers::Input> input;
PathSet context;

Expand All @@ -56,21 +89,40 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V

for (auto & attr : *args[0]->attrs) {
state.forceValue(*attr.value);
if (attr.value->type == tString)
attrs.emplace(attr.name, attr.value->string.s);
if (attr.value->type == tPath)
addURI(
state,
attrs,
attr.name,
state.coerceToString(*attr.pos, *attr.value, context, false, false)
);
else if (attr.value->type == tString)
addURI(state, attrs, attr.name, attr.value->string.s);
else if (attr.value->type == tBool)
attrs.emplace(attr.name, attr.value->boolean);
else
throw TypeError("fetchTree argument '%s' is %s while a string or Boolean is expected",
attr.name, showType(*attr.value));
}

if (!attrs.count("type"))
throw Error("attribute 'type' is missing in call to 'fetchTree', at %s", pos);
if (type)
attrs.emplace("type", type.value());

validateFetcherAttrs(attrs, pos);

input = fetchers::inputFromAttrs(attrs);
} else
input = fetchers::inputFromURL(state.coerceToString(pos, *args[0], context, false, false));
} else {
auto url = fixURI(state.coerceToString(pos, *args[0], context, false, false), state);

if (type && type.value() == "git") {
fetchers::Attrs attrs;
attrs.emplace("type", "git");
attrs.emplace("url", url);
input = fetchers::inputFromAttrs(attrs);
} else {
input = fetchers::inputFromURL(url);
}
}

if (evalSettings.pureEval && !input->isImmutable())
throw Error("in pure evaluation mode, 'fetchTree' requires an immutable input");
Expand All @@ -84,6 +136,11 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V
emitTreeAttrs(state, tree, input2, v);
}

static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, Value & v)
{
fetchTree(state, pos, args, v, std::nullopt);
}

static RegisterPrimOp r("fetchTree", 1, prim_fetchTree);

static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v,
Expand Down Expand Up @@ -159,7 +216,13 @@ static void prim_fetchTarball(EvalState & state, const Pos & pos, Value * * args
fetch(state, pos, args, v, "fetchTarball", true, "source");
}

static void prim_fetchGit(EvalState &state, const Pos &pos, Value **args, Value &v)
{
fetchTree(state, pos, args, v, "git");
}

static RegisterPrimOp r2("__fetchurl", 1, prim_fetchurl);
static RegisterPrimOp r3("fetchTarball", 1, prim_fetchTarball);
static RegisterPrimOp r4("fetchGit", 1, prim_fetchGit);

}
2 changes: 1 addition & 1 deletion src/libfetchers/fetchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ std::pair<Tree, std::shared_ptr<const Input>> Input::fetchTree(ref<Store> store)
assert(input->narHash == tree.info.narHash);

if (narHash && narHash != input->narHash)
throw Error("NAR hash mismatch in input '%s' (%s), expected '%s', got '%s'",
throw Error((unsigned int) 102, "NAR hash mismatch in input '%s' (%s), expected '%s', got '%s'",
to_string(), tree.actualPath, narHash->to_string(SRI), input->narHash->to_string(SRI));

return {std::move(tree), input};
Expand Down
2 changes: 2 additions & 0 deletions src/libfetchers/fetchers.hh
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ struct Input : std::enable_shared_from_this<Input>

virtual std::optional<Hash> getRev() const { return {}; }

virtual bool fetchSubmodules() const { return false; }

virtual ParsedURL toURL() const = 0;

std::string to_string() const
Expand Down
7 changes: 7 additions & 0 deletions src/libfetchers/git.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ struct GitInput : Input

std::optional<Hash> getRev() const override { return rev; }

bool fetchSubmodules() const override { return submodules; }

ParsedURL toURL() const override
{
ParsedURL url2(url);
Expand Down Expand Up @@ -115,6 +117,7 @@ struct GitInput : Input
{
assert(input->rev);
assert(!rev || rev == input->rev);
input->narHash = store->queryPathInfo(storePath)->narHash;
return {
Tree {
.actualPath = store->toRealPath(storePath),
Expand Down Expand Up @@ -207,6 +210,10 @@ struct GitInput : Input
}
};

// Backward compatibility: set 'rev' to
// 0000000000000000000000000000000000000000 for a dirty tree.
input->rev = Hash(htSHA1);

return {std::move(tree), input};
}
}
Expand Down
10 changes: 10 additions & 0 deletions tests/fetchGit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ rev2=$(git -C $repo rev-parse HEAD)
# Fetch a worktree
unset _NIX_FORCE_HTTP
path0=$(nix eval --raw "(builtins.fetchGit file://$TEST_ROOT/worktree).outPath")
path0_=$(nix eval --raw "(builtins.fetchTree { type = \"git\"; url = file://$TEST_ROOT/worktree; }).outPath")
[[ $path0 = $path0_ ]]
export _NIX_FORCE_HTTP=1
[[ $(tail -n 1 $path0/hello) = "hello" ]]

Expand Down Expand Up @@ -102,6 +104,14 @@ git -C $repo commit -m 'Bla3' -a
path4=$(nix eval --tarball-ttl 0 --raw "(builtins.fetchGit file://$repo).outPath")
[[ $path2 = $path4 ]]

nix eval --raw "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-B5yIPHhEm0eysJKEsO7nqxprh9vcblFxpJG11gXJus1=\"; }).outPath" || status=$?
[[ "$status" = "102" ]]

nix eval --raw "(builtins.fetchGit { url = $repo; narHash = \"sha256-B5yIPHhEm0eysJKEsO7nqxprh9vcblFxpJG11gXJus1=\"; }).outPath" 2>&1 | grep "attribute 'narHash' can only be used for 'git' if either 'ref' or 'rev' is set"

path5=$(nix eval --raw "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-Hr8g6AqANb3xqX28eu1XnjK/3ab8Gv6TJSnkb1LezG9=\"; }).outPath")
[[ $path = $path5 ]]

# tarball-ttl should be ignored if we specify a rev
echo delft > $repo/hello
git -C $repo add hello
Expand Down
11 changes: 7 additions & 4 deletions tests/tarball.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,13 @@ test_tarball() {

nix-build -o $TEST_ROOT/result -E "import (fetchTarball file://$tarball)"

nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree file://$tarball)"
nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; })"
nix-build --experimental-features flakes -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })"
nix-build --experimental-features flakes -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-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 = \"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
nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" 2>&1 | grep 'true'

nix-instantiate --eval -E '1 + 2' -I fnord=file://no-such-tarball.tar$ext
nix-instantiate --eval -E 'with <fnord/xyzzy>; 1 + 2' -I fnord=file://no-such-tarball$ext
Expand Down

0 comments on commit cd91739

Please sign in to comment.