Merge pull request #6174 from hercules-ci/fetchTree-reuse-local-paths

fetchTree: Do not re-fetch paths already present + refactor

src/libexpr/eval.cc

@@ -517,6 +517,14 @@ void EvalState::allowPath(const StorePath & storePath)
         allowedPaths->insert(store->toRealPath(storePath));
 }
 
+void EvalState::allowAndSetStorePathString(const StorePath &storePath, Value &v)
+{
+    allowPath(storePath);
+
+    auto path = store->printStorePath(storePath);
+    v.mkString(path, PathSet({path}));
+}
+
 Path EvalState::checkSourcePath(const Path & path_)
 {
     if (!allowedPaths) return path_;

src/libexpr/eval.hh

@@ -161,6 +161,9 @@ public:
        the real store path if `store` is a chroot store. */
     void allowPath(const StorePath & storePath);
 
+    /* Allow access to a store path and return it as a string. */
+    void allowAndSetStorePathString(const StorePath & storePath, Value &v);
+
     /* Check whether access to a path is allowed and throw an error if
        not. Otherwise return the canonicalised path. */
     Path checkSourcePath(const Path & path);

src/libexpr/primops.cc

@@ -1919,20 +1919,15 @@ static void addPath(
         if (expectedHash)
             expectedStorePath = state.store->makeFixedOutputPath(method, *expectedHash, name);
 
-        Path dstPath;
         if (!expectedHash || !state.store->isValidPath(*expectedStorePath)) {
-            dstPath = state.store->printStorePath(settings.readOnlyMode
+            StorePath dstPath = settings.readOnlyMode
                 ? state.store->computeStorePathForPath(name, path, method, htSHA256, filter).first
-                : state.store->addToStore(name, path, method, htSHA256, filter, state.repair, refs));
+                : state.store->addToStore(name, path, method, htSHA256, filter, state.repair, refs);
-            if (expectedHash && expectedStorePath != state.store->parseStorePath(dstPath))
+            if (expectedHash && expectedStorePath != dstPath)
                 throw Error("store path mismatch in (possibly filtered) path added from '%s'", path);
+            state.allowAndSetStorePathString(dstPath, v);
         } else
-            dstPath = state.store->printStorePath(*expectedStorePath);
+            state.allowAndSetStorePathString(*expectedStorePath, v);
-
-        v.mkString(dstPath, {dstPath});
-
-        state.allowPath(dstPath);
-
     } catch (Error & e) {
         e.addTrace(pos, "while adding path '%s'", path);
         throw;

src/libexpr/primops/fetchTree.cc

@@ -203,6 +203,8 @@ static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v,
                 url = state.forceStringNoCtx(*attr.value, *attr.pos);
             else if (n == "sha256")
                 expectedHash = newHashAllowEmpty(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA256);
+            else if (n == "narHash")
+                expectedHash = newHashAllowEmpty(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA256);
             else if (n == "name")
                 name = state.forceStringNoCtx(*attr.value, *attr.pos);
             else
@@ -230,6 +232,20 @@ static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v,
     if (evalSettings.pureEval && !expectedHash)
         throw Error("in pure evaluation mode, '%s' requires a 'sha256' argument", who);
 
+    // early exit if pinned and already in the store
+    if (expectedHash && expectedHash->type == htSHA256) {
+        auto expectedPath =
+            unpack
+            ? state.store->makeFixedOutputPath(FileIngestionMethod::Recursive, *expectedHash, name, {})
+            : state.store->makeFixedOutputPath(FileIngestionMethod::Flat, *expectedHash, name, {});
+
+        auto validPaths = state.store->queryValidPaths({expectedPath}, NoSubstitute);
+        if (!validPaths.empty()) {
+            state.allowAndSetStorePathString(expectedPath, v);
+            return;
+        }
+    }
+
     auto storePath =
         unpack
         ? fetchers::downloadTarball(state.store, *url, name, (bool) expectedHash).first.storePath
@@ -244,10 +260,7 @@ static void fetch(EvalState & state, const Pos & pos, Value * * args, Value & v,
                 *url, expectedHash->to_string(Base32, true), hash.to_string(Base32, true));
     }
 
-    state.allowPath(storePath);
+    state.allowAndSetStorePathString(storePath, v);
-
-    auto path = state.store->printStorePath(storePath);
-    v.mkString(path, PathSet({path}));
 }
 
 static void prim_fetchurl(EvalState & state, const Pos & pos, Value * * args, Value & v)

tests/fetchurl.sh

@@ -9,6 +9,10 @@ outPath=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file:
 
 cmp $outPath fetchurl.sh
 
+# Do not re-fetch paths already present.
+outPath2=$(nix-build -vvvvv --expr 'import <nix/fetchurl.nix>' --argstr url file:///does-not-exist/must-remain-unused/fetchurl.sh --argstr sha256 $hash --no-out-link)
+test "$outPath" == "$outPath2"
+
 # Now using a base-64 hash.
 clearStore
 

tests/tarball.sh

@@ -26,10 +26,14 @@ test_tarball() {
     nix-build -o $TEST_ROOT/result '<foo>' -I foo=file://$tarball
 
     nix-build -o $TEST_ROOT/result -E "import (fetchTarball file://$tarball)"
+    # Do not re-fetch paths already present
+    nix-build  -o $TEST_ROOT/result -E "import (fetchTarball { url = file:///does-not-exist/must-remain-unused/$tarball; sha256 = \"$hash\"; })"
 
     nix-build  -o $TEST_ROOT/result -E "import (fetchTree file://$tarball)"
     nix-build  -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; narHash = \"$hash\"; })"
+    # Do not re-fetch paths already present
+    nix-build  -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file:///does-not-exist/must-remain-unused/$tarball; narHash = \"$hash\"; })"
     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