Merge pull request #270 from DeterminateSystems/shallow-optimization

Avoid unnecessary Git refetches

Author: edolstra

src/libexpr/primops/fetchTree.cc

@@ -150,11 +150,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/git.cc

@@ -627,6 +627,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, ref<Store> store, RepoInfo & repoInfo, Input && input) const
     {
@@ -635,7 +645,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);
 
@@ -646,11 +656,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()});
 
@@ -666,7 +692,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)) {
@@ -680,7 +706,6 @@ struct GitInputScheme : InputScheme
             }
 
             if (doFetch) {
-                bool shallow = getShallowAttr(input);
                 try {
                     auto fetchRef = getAllRefsAttr(input)             ? "refs/*:refs/*"
                                     : input.getRev()                  ? input.getRev()->gitRev()
@@ -708,7 +733,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'! "
@@ -725,15 +750,9 @@ 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();
@@ -744,10 +763,16 @@ struct GitInputScheme : InputScheme
         if (!input.attrs.contains("lastModified"))
             input.attrs.insert_or_assign("lastModified", getLastModified(settings, repoInfo, repoDir, rev));
 
-        if (!getShallowAttr(input)) {
-            /* Like lastModified, skip revCount if supplied by the caller. */
-            if (!input.attrs.contains("revCount"))
-                input.attrs.insert_or_assign("revCount", getRevCount(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());
+
+            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

@@ -36,6 +36,7 @@ suites += {
     'trace-ifd.sh',
     'build-time-flake-inputs.sh',
     'substitution.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/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;
       }}
     """