Fix test

This fixes the issue where updating a Git input does a non-shallow
fetch, and then a subsequent eval does a shallow refetch because
the revCount is already known. Now the subsequent eval will use the
repo used in the first fetch.

src/libexpr/primops/fetchTree.cc

@@ -151,11 +151,6 @@ static void fetchTree(
             attrs.emplace("exportIgnore", Explicit<bool>{true});
         }
 
-        // fetchTree should fetch git repos with shallow = true by default
-        if (type == "git" && !params.isFetchGit && !attrs.contains("shallow")) {
-            attrs.emplace("shallow", Explicit<bool>{true});
-        }
-
         if (!params.allowNameArgument)
             if (auto nameIter = attrs.find("name"); nameIter != attrs.end())
                 state.error<EvalError>("argument 'name' isn’t supported in call to '%s'", fetcher)

src/libfetchers/fetchers.cc

@@ -269,24 +269,10 @@ void Input::checkLocks(Input specified, Input & result)
         }
     }
 
-    if (auto prevLastModified = specified.getLastModified()) {
-        if (result.getLastModified() != prevLastModified)
-            throw Error(
-                "'lastModified' attribute mismatch in input '%s', expected %d, got %d",
-                result.to_string(),
-                *prevLastModified,
-                result.getLastModified().value_or(-1));
-    }
-
     if (auto prevRev = specified.getRev()) {
         if (result.getRev() != prevRev)
             throw Error("'rev' attribute mismatch in input '%s', expected %s", result.to_string(), prevRev->gitRev());
     }
-
-    if (auto prevRevCount = specified.getRevCount()) {
-        if (result.getRevCount() != prevRevCount)
-            throw Error("'revCount' attribute mismatch in input '%s', expected %d", result.to_string(), *prevRevCount);
-    }
 }
 
 std::pair<ref<SourceAccessor>, Input> Input::getAccessor(const Settings & settings, Store & store) const

src/libfetchers/git.cc

@@ -778,6 +778,16 @@ struct GitInputScheme : InputScheme
         }
     }
 
+    /**
+     * Decide whether we can do a shallow clone, which is faster. This is possible if the user explicitly specified
+     * `shallow = true`, or if we already have a `revCount`.
+     */
+    bool canDoShallow(const Input & input) const
+    {
+        bool shallow = getShallowAttr(input);
+        return shallow || input.getRevCount().has_value();
+    }
+
     std::pair<ref<SourceAccessor>, Input>
     getAccessorFromCommit(const Settings & settings, Store & store, RepoInfo & repoInfo, Input && input) const
     {
@@ -786,7 +796,7 @@ struct GitInputScheme : InputScheme
         auto origRev = input.getRev();
 
         auto originalRef = input.getRef();
-        bool shallow = getShallowAttr(input);
+        bool shallow = canDoShallow(input);
         auto ref = originalRef ? *originalRef : getDefaultRef(repoInfo, shallow);
         input.attrs.insert_or_assign("ref", ref);
 
@@ -797,11 +807,27 @@ struct GitInputScheme : InputScheme
             if (!input.getRev())
                 input.attrs.insert_or_assign("rev", GitRepo::openRepo(repoDir)->resolveRef(ref).gitRev());
         } else {
+            auto rev = input.getRev();
             auto repoUrl = std::get<ParsedURL>(repoInfo.location);
             std::filesystem::path cacheDir = getCachePath(repoUrl.to_string(), shallow);
             repoDir = cacheDir;
             repoInfo.gitDir = ".";
 
+            /* If shallow = false, but we have a non-shallow repo that already contains the desired rev, then use that
+             * repo instead. */
+            std::filesystem::path cacheDirNonShallow = getCachePath(repoUrl.to_string(), false);
+            if (rev && shallow && pathExists(cacheDirNonShallow)) {
+                auto nonShallowRepo = GitRepo::openRepo(cacheDirNonShallow, true, true);
+                if (nonShallowRepo->hasObject(*rev)) {
+                    debug(
+                        "using non-shallow cached repo for '%s' since it contains rev '%s'",
+                        repoUrl.to_string(),
+                        rev->gitRev());
+                    repoDir = cacheDirNonShallow;
+                    goto have_rev;
+                }
+            }
+
             std::filesystem::create_directories(cacheDir.parent_path());
             PathLocks cacheDirLock({cacheDir.string()});
 
@@ -817,7 +843,7 @@ struct GitInputScheme : InputScheme
 
             /* If a rev was specified, we need to fetch if it's not in the
                repo. */
-            if (auto rev = input.getRev()) {
+            if (rev) {
                 doFetch = !repo->hasObject(*rev);
             } else {
                 if (getAllRefsAttr(input)) {
@@ -831,7 +857,6 @@ struct GitInputScheme : InputScheme
             }
 
             if (doFetch) {
-                bool shallow = getShallowAttr(input);
                 try {
                     auto fetchRef = getAllRefsAttr(input)             ? "refs/*:refs/*"
                                     : input.getRev()                  ? input.getRev()->gitRev()
@@ -859,7 +884,7 @@ struct GitInputScheme : InputScheme
                     warn("could not update cached head '%s' for '%s'", ref, repoInfo.locationToArg());
             }
 
-            if (auto rev = input.getRev()) {
+            if (rev) {
                 if (!repo->hasObject(*rev))
                     throw Error(
                         "Cannot find Git revision '%s' in ref '%s' of repository '%s'! "
@@ -876,23 +901,30 @@ struct GitInputScheme : InputScheme
             // the remainder
         }
 
+    have_rev:
         auto repo = GitRepo::openRepo(repoDir);
 
-        auto isShallow = repo->isShallow();
-
-        if (isShallow && !getShallowAttr(input))
-            throw Error(
-                "'%s' is a shallow Git repository, but shallow repositories are only allowed when `shallow = true;` is specified",
-                repoInfo.locationToArg());
-
         // FIXME: check whether rev is an ancestor of ref?
 
         auto rev = *input.getRev();
 
-        input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev));
+        /* Skip lastModified computation if it's already supplied by the caller.
+           We don't care if they specify an incorrect value; it doesn't
+           matter for security, unlike narHash. */
+        if (!input.attrs.contains("lastModified"))
+            input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev));
+
+        /* Like lastModified, skip revCount if supplied by the caller. */
+        if (!shallow && !input.attrs.contains("revCount")) {
+            auto isShallow = repo->isShallow();
+
+            if (isShallow && !shallow)
+                throw Error(
+                    "'%s' is a shallow Git repository, but shallow repositories are only allowed when `shallow = true;` is specified",
+                    repoInfo.locationToArg());
 
-        if (!getShallowAttr(input))
             input.attrs.insert_or_assign("revCount", getRevCount(settings, repoInfo, repoDir, rev));
+        }
 
         printTalkative("using revision %s of repo '%s'", rev.gitRev(), repoInfo.locationToArg());
 

tests/functional/flakes/common.sh

@@ -32,6 +32,8 @@ writeSimpleFlake() {
     baseName = builtins.baseNameOf ./.;
 
     root = ./.;
+
+    number = 123;
   };
 }
 EOF

tests/functional/flakes/meson.build

@@ -34,6 +34,7 @@ suites += {
     'source-paths.sh',
     'old-lockfiles.sh',
     'trace-ifd.sh',
+    'shallow.sh',
   ],
   'workdir' : meson.current_source_dir(),
 }

tests/functional/flakes/shallow.sh

@@ -0,0 +1,35 @@
+#!/usr/bin/env bash
+
+export _NIX_FORCE_HTTP=1
+
+source ./common.sh
+
+requireGit
+TODO_NixOS
+
+createFlake1
+
+repoDir="$TEST_ROOT/repo"
+mkdir -p "$repoDir"
+echo "# foo" >> "$flake1Dir/flake.nix"
+git -C "$flake1Dir" commit -a -m bla
+
+cat > "$repoDir"/flake.nix <<EOF
+{
+  inputs.dep = {
+    type = "git";
+    url = "file://$flake1Dir";
+  };
+  outputs = inputs: rec {
+    revs = assert inputs.dep.number == 123; inputs.dep.revCount;
+  };
+}
+EOF
+
+# This will do a non-shallow fetch.
+[[ $(nix eval "path:$repoDir#revs") = 2 ]]
+
+# This should re-use the existing non-shallow clone.
+clearStore
+mv "$flake1Dir" "$flake1Dir.moved"
+[[ $(nix eval "path:$repoDir#revs") = 2 ]]

tests/nixos/content-encoding.nix

@@ -131,6 +131,7 @@ in
     start_all()
 
     machine.wait_for_unit("nginx.service")
+    machine.wait_for_open_port(80)
 
     # Original test: zstd archive with gzip content-encoding
     # Make sure that the file is properly compressed as the test would be meaningless otherwise

tests/nixos/fetch-git/test-cases/fetchTree-shallow/default.nix

@@ -1,5 +1,5 @@
 {
-  description = "fetchTree fetches git repos shallowly by default";
+  description = "fetchTree fetches git repos shallowly if possible";
   script = ''
     # purge nix git cache to make sure we start with a clean slate
     client.succeed("rm -rf ~/.cache/nix")
@@ -28,6 +28,7 @@
         type = "git";
         url = "{repo.remote}";
         rev = "{commit2_rev}";
+        revCount = 1234;
       }}
     """
 

tests/nixos/tarball-flakes.nix

@@ -99,7 +99,6 @@ in
 
       # Check that fetching fails if we provide incorrect attributes.
       machine.fail("nix flake metadata --json http://localhost/tags/latest.tar.gz?rev=493300eb13ae6fb387fbd47bf54a85915acc31c0")
-      machine.fail("nix flake metadata --json http://localhost/tags/latest.tar.gz?revCount=789")
       machine.fail("nix flake metadata --json http://localhost/tags/latest.tar.gz?narHash=sha256-tbudgBSg+bHWHiHnlteNzN8TUvI80ygS9IULh4rklEw=")
     '';