Merge pull request #269 from DeterminateSystems/skip-revCount

Git fetcher: Don't compute revCount/lastModified if they're already specified

Author: edolstra

src/libfetchers/fetchers.cc

@@ -276,24 +276,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, ref<Store> store) const

src/libfetchers/git.cc

@@ -738,13 +738,17 @@ struct GitInputScheme : InputScheme
 
         auto rev = *input.getRev();
 
-        Attrs infoAttrs({
-            {"rev", rev.gitRev()},
-            {"lastModified", getLastModified(settings, repoInfo, repoDir, rev)},
-        });
-
-        if (!getShallowAttr(input))
-            infoAttrs.insert_or_assign("revCount", getRevCount(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));
+
+        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));
+        }
 
         printTalkative("using revision %s of repo '%s'", rev.gitRev(), repoInfo.locationToArg());
 
@@ -800,9 +804,6 @@ struct GitInputScheme : InputScheme
         }
 
         assert(!origRev || origRev == rev);
-        if (!getShallowAttr(input))
-            input.attrs.insert_or_assign("revCount", getIntAttr(infoAttrs, "revCount"));
-        input.attrs.insert_or_assign("lastModified", getIntAttr(infoAttrs, "lastModified"));
 
         return {accessor, std::move(input)};
     }

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/tarball-flakes.nix

@@ -98,7 +98,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=")
     '';