fetchGit: Ignore tarballTtl if rev is set and not in the repo.

Fixes #1697.

3 files changed, 43 insertions, 24 deletions

diff --git a/src/libexpr/primops/fetchGit.cc b/src/libexpr/primops/fetchGit.cc
index fd3e84c292c3..e92e0638031f 100644
--- a/src/libexpr/primops/fetchGit.cc
+++ b/src/libexpr/primops/fetchGit.cc
@@ -89,32 +89,43 @@ GitInfo exportGit(ref<Store> store, const std::string & uri,
 
     Path localRefFile = cacheDir + "/refs/heads/" + localRef;
 
-    /* If the local ref is older than ‘tarball-ttl’ seconds, do a git
-       fetch to update the local ref to the remote ref. */
+    bool doFetch;
     time_t now = time(0);
-    struct stat st;
-    if (stat(localRefFile.c_str(), &st) != 0 ||
-        st.st_mtime <= now - settings.tarballTtl)
-    {
-        if (rev == "" ||
-            chomp(runProgram(
-                RunOptions("git", { "-C", cacheDir, "cat-file", "-t", rev })
-                .killStderr(true)).second) != "commit")
-        {
-            Activity act(*logger, lvlTalkative, actUnknown, fmt("fetching Git repository '%s'", uri));
-
-            // FIXME: git stderr messes up our progress indicator, so
-            // we're using --quiet for now. Should process its stderr.
-            runProgram("git", true, { "-C", cacheDir, "fetch", "--quiet", "--force", "--", uri, *ref + ":" + localRef });
-
-            struct timeval times[2];
-            times[0].tv_sec = now;
-            times[0].tv_usec = 0;
-            times[1].tv_sec = now;
-            times[1].tv_usec = 0;
-
-            utimes(localRefFile.c_str(), times);
+    /* If a rev was specified, we need to fetch if it's not in the
+       repo. */
+    if (rev != "") {
+        try {
+            runProgram("git", true, { "-C", cacheDir, "cat-file", "-e", rev });
+            doFetch = false;
+        } catch (ExecError & e) {
+            if (WIFEXITED(e.status)) {
+                doFetch = true;
+            } else {
+                throw;
+            }
         }
+    } else {
+        /* If the local ref is older than ‘tarball-ttl’ seconds, do a
+           git fetch to update the local ref to the remote ref. */
+        struct stat st;
+        doFetch = stat(localRefFile.c_str(), &st) != 0 ||
+            st.st_mtime <= now - settings.tarballTtl;
+    }
+    if (doFetch)
+    {
+        Activity act(*logger, lvlTalkative, actUnknown, fmt("fetching Git repository '%s'", uri));
+
+        // FIXME: git stderr messes up our progress indicator, so
+        // we're using --quiet for now. Should process its stderr.
+        runProgram("git", true, { "-C", cacheDir, "fetch", "--quiet", "--force", "--", uri, *ref + ":" + localRef });
+
+        struct timeval times[2];
+        times[0].tv_sec = now;
+        times[0].tv_usec = 0;
+        times[1].tv_sec = now;
+        times[1].tv_usec = 0;
+
+        utimes(localRefFile.c_str(), times);
     }
 
     // FIXME: check whether rev is an ancestor of ref.
diff --git a/tests/common.sh.in b/tests/common.sh.in
index ca6df25362dc..09f2949141a4 100644
--- a/tests/common.sh.in
+++ b/tests/common.sh.in
@@ -21,6 +21,7 @@ export NIX_REMOTE=$NIX_REMOTE_
 unset NIX_PATH
 export TEST_HOME=$TEST_ROOT/test-home
 export HOME=$TEST_HOME
+unset XDG_CACHE_HOME
 mkdir -p $TEST_HOME
 
 export PATH=@bindir@:$PATH
diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh
index 7b13b587defb..09e4f742668e 100644
--- a/tests/fetchGit.sh
+++ b/tests/fetchGit.sh
@@ -86,3 +86,10 @@ git -C $repo commit -m 'Bla3' -a
 
 path4=$(nix eval --tarball-ttl 0 --raw "(builtins.fetchGit file://$repo).outPath")
 [[ $path2 = $path4 ]]
+
+# tarball-ttl should be ignored if we specify a rev
+echo delft > $repo/hello
+git -C $repo add hello
+git -C $repo commit -m 'Bla4'
+rev3=$(git -C $repo rev-parse HEAD)
+nix eval --tarball-ttl 3600 "(builtins.fetchGit { url = $repo; rev = \"$rev3\"; })" >/dev/null