Merge legacy fetchGit-builtin with the generic fetchTree-function

The original idea was to implement a git-fetcher in Nix's core that
supports content hashes[1]. In #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] https://github.com/NixOS/nix/pull/3216
[2] https://github.com/NixOS/nix/pull/3549#issuecomment-625194383
[3] https://github.com/NixOS/nix/pull/3459
[4] https://github.com/NixOS/nix/pull/3216#issuecomment-553956703

src/libexpr/primops/fetchGit.cc

@@ -1,91 +0,0 @@
-#include "primops.hh"
-#include "eval-inline.hh"
-#include "store-api.hh"
-#include "hash.hh"
-#include "fetchers.hh"
-#include "url.hh"
-
-namespace nix {
-
-static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Value & v)
-{
-    std::string url;
-    std::optional<std::string> ref;
-    std::optional<Hash> rev;
-    std::string name = "source";
-    bool fetchSubmodules = false;
-    PathSet context;
-
-    state.forceValue(*args[0]);
-
-    if (args[0]->type == tAttrs) {
-
-        state.forceAttrs(*args[0], pos);
-
-        for (auto & attr : *args[0]->attrs) {
-            string n(attr.name);
-            if (n == "url")
-                url = state.coerceToString(*attr.pos, *attr.value, context, false, false);
-            else if (n == "ref")
-                ref = state.forceStringNoCtx(*attr.value, *attr.pos);
-            else if (n == "rev")
-                rev = Hash(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA1);
-            else if (n == "name")
-                name = state.forceStringNoCtx(*attr.value, *attr.pos);
-            else if (n == "submodules")
-                fetchSubmodules = state.forceBool(*attr.value, *attr.pos);
-            else
-                throw EvalError({
-                    .hint = hintfmt("unsupported argument '%s' to 'fetchGit'", attr.name),
-                    .errPos = *attr.pos
-                });
-        }
-
-        if (url.empty())
-            throw EvalError({
-                .hint = hintfmt("'url' argument required"),
-                .errPos = pos
-            });
-
-    } else
-        url = state.coerceToString(pos, *args[0], context, false, false);
-
-    // FIXME: git externals probably can be used to bypass the URI
-    // whitelist. Ah well.
-    state.checkURI(url);
-
-    if (evalSettings.pureEval && !rev)
-        throw Error("in pure evaluation mode, 'fetchGit' requires a Git revision");
-
-    fetchers::Attrs attrs;
-    attrs.insert_or_assign("type", "git");
-    attrs.insert_or_assign("url", url.find("://") != std::string::npos ? url : "file://" + url);
-    if (ref) attrs.insert_or_assign("ref", *ref);
-    if (rev) attrs.insert_or_assign("rev", rev->gitRev());
-    if (fetchSubmodules) attrs.insert_or_assign("submodules", fetchers::Explicit<bool>{true});
-    auto input = fetchers::Input::fromAttrs(std::move(attrs));
-
-    // FIXME: use name?
-    auto [tree, input2] = input.fetch(state.store);
-
-    state.mkAttrs(v, 8);
-    auto storePath = state.store->printStorePath(tree.storePath);
-    mkString(*state.allocAttr(v, state.sOutPath), storePath, PathSet({storePath}));
-    // Backward compatibility: set 'rev' to
-    // 0000000000000000000000000000000000000000 for a dirty tree.
-    auto rev2 = input2.getRev().value_or(Hash(htSHA1));
-    mkString(*state.allocAttr(v, state.symbols.create("rev")), rev2.gitRev());
-    mkString(*state.allocAttr(v, state.symbols.create("shortRev")), rev2.gitShortRev());
-    // Backward compatibility: set 'revCount' to 0 for a dirty tree.
-    mkInt(*state.allocAttr(v, state.symbols.create("revCount")),
-        input2.getRevCount().value_or(0));
-    mkBool(*state.allocAttr(v, state.symbols.create("submodules")), fetchSubmodules);
-    v.attrs->sort();
-
-    if (state.allowedPaths)
-        state.allowedPaths->insert(tree.actualPath);
-}
-
-static RegisterPrimOp r("fetchGit", 1, prim_fetchGit);
-
-}

src/libexpr/primops/fetchTree.cc

@@ -36,6 +36,9 @@ void emitTreeAttrs(
         mkString(*state.allocAttr(v, state.symbols.create("shortRev")), rev->gitShortRev());
     }
 
+    if (input.getType() == "git")
+        mkBool(*state.allocAttr(v, state.symbols.create("submodules")), maybeGetBoolAttr(input.attrs, "submodules").value_or(false));
+
     if (auto revCount = input.getRevCount())
         mkInt(*state.allocAttr(v, state.symbols.create("revCount")), *revCount);
 
@@ -48,10 +51,25 @@ 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)
 {
-    settings.requireExperimentalFeature("flakes");
+    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);
+}
+
+static void fetchTree(
+    EvalState &state,
+    const Pos &pos,
+    Value **args,
+    Value &v,
+    const std::optional<std::string> type
+) {
     fetchers::Input input;
     PathSet context;
 
@@ -64,8 +82,15 @@ 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)
+            if (attr.value->type == tPath || attr.value->type == tString)
-                attrs.emplace(attr.name, attr.value->string.s);
+                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, fetchers::Explicit<bool>{attr.value->boolean});
             else if (attr.value->type == tInt)
@@ -75,6 +100,9 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V
                     attr.name, showType(*attr.value));
         }
 
+        if (type)
+            attrs.emplace("type", type.value());
+
         if (!attrs.count("type"))
             throw Error({
                 .hint = hintfmt("attribute 'type' is missing in call to 'fetchTree'"),
@@ -82,8 +110,18 @@ static void prim_fetchTree(EvalState & state, const Pos & pos, Value * * args, V
             });
 
         input = fetchers::Input::fromAttrs(std::move(attrs));
-    } else
+    } else {
-        input = fetchers::Input::fromURL(state.coerceToString(pos, *args[0], context, false, false));
+        auto url = fixURI(state.coerceToString(pos, *args[0], context, false, false), state);
+
+        if (type == "git") {
+            fetchers::Attrs attrs;
+            attrs.emplace("type", "git");
+            attrs.emplace("url", url);
+            input = fetchers::Input::fromAttrs(std::move(attrs));
+        } else {
+            input = fetchers::Input::fromURL(url);
+        }
+    }
 
     if (!evalSettings.pureEval && !input.isDirect())
         input = lookupInRegistries(state.store, input).first;
@@ -99,6 +137,12 @@ 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)
+{
+    settings.requireExperimentalFeature("flakes");
+    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,
@@ -178,7 +222,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);
 
 }

src/libfetchers/fetchers.cc

@@ -134,7 +134,7 @@ std::pair<Tree, Input> Input::fetch(ref<Store> store) const
 
     if (auto prevNarHash = getNarHash()) {
         if (narHash != *prevNarHash)
-            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, prevNarHash->to_string(SRI, true), narHash->to_string(SRI, true));
     }
 

tests/fetchGit.sh

@@ -32,6 +32,8 @@ rev2=$(git -C $repo rev-parse HEAD)
 # Fetch a worktree
 unset _NIX_FORCE_HTTP
 path0=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$TEST_ROOT/worktree).outPath")
+path0_=$(nix eval --impure --raw --expr "(builtins.fetchTree { type = \"git\"; url = file://$TEST_ROOT/worktree; }).outPath")
+[[ $path0 = $path0_ ]]
 export _NIX_FORCE_HTTP=1
 [[ $(tail -n 1 $path0/hello) = "hello" ]]
 
@@ -87,8 +89,6 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath")
 [ ! -e $path2/.git ]
 [[ $(cat $path2/dir1/foo) = foo ]]
 
-[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]]
-
 # ... unless we're using an explicit ref or rev.
 path3=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; ref = \"master\"; }).outPath")
 [[ $path = $path3 ]]
@@ -102,6 +102,12 @@ git -C $repo commit -m 'Bla3' -a
 path4=$(nix eval --impure --refresh --raw --expr "(builtins.fetchGit file://$repo).outPath")
 [[ $path2 = $path4 ]]
 
+nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; rev = \"$rev2\"; narHash = \"sha256-B5yIPHhEm0eysJKEsO7nqxprh9vcblFxpJG11gXJus1=\"; }).outPath" || status=$?
+[[ "$status" = "102" ]]
+
+path5=$(nix eval --impure --raw --expr "(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
@@ -123,7 +129,7 @@ path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath
 # Using local path with branch other than 'master' should work when clean or dirty
 path3=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath")
 # (check dirty-tree handling was used)
-[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]]
+[[ $(nix eval --impure --expr "(builtins.fetchGit $repo).rev or null") = null ]]
 
 # Committing shouldn't change store path, or switch to using 'master'
 git -C $repo commit -m 'Bla5' -a

tests/tarball.sh

@@ -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  -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  -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  -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 { 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