fetchToStore() cache: Use content hashes instead of store paths

We can always compute the store path from the content hash, but not
vice versa. Storing the content hash allows `hashPath()` to be
replaced by `fetchToStore(...FetchMode::DryRun...)`, which gets us
caching in lazy-trees mode.

src/libexpr/paths.cc

@@ -84,8 +84,7 @@ StorePath EvalState::mountInput(
             if (store->isValidPath(storePath))
                 _narHash = store->queryPathInfo(storePath)->narHash;
             else
-                // FIXME: use fetchToStore to make it cache this
+                _narHash = fetchToStore2(*store, accessor, FetchMode::DryRun, input.getName()).second;
-                _narHash = accessor->hashPath(CanonPath::root);
         }
         return _narHash;
     };

src/libfetchers/fetch-to-store.cc

@@ -3,19 +3,16 @@
 
 namespace nix {
 
-fetchers::Cache::Key makeFetchToStoreCacheKey(
+fetchers::Cache::Key makeSourcePathToHashCacheKey(
-    const std::string &name,
+    const std::string & fingerprint,
-    const std::string &fingerprint,
     ContentAddressMethod method,
-    const std::string &path)
+    const std::string & path)
 {
-    return fetchers::Cache::Key{"fetchToStore", {
+    return fetchers::Cache::Key{"sourcePathToHash", {
-        {"name", name},
         {"fingerprint", fingerprint},
         {"method", std::string{method.render()}},
         {"path", path}
     }};
-
 }
 
 StorePath fetchToStore(
@@ -27,9 +24,18 @@ StorePath fetchToStore(
     PathFilter * filter,
     RepairFlag repair)
 {
-    // FIXME: add an optimisation for the case where the accessor is
+    return fetchToStore2(store, path, mode, name, method, filter, repair).first;
-    // a `PosixSourceAccessor` pointing to a store path.
+}
 
+std::pair<StorePath, Hash> fetchToStore2(
+    Store & store,
+    const SourcePath & path,
+    FetchMode mode,
+    std::string_view name,
+    ContentAddressMethod method,
+    PathFilter * filter,
+    RepairFlag repair)
+{
     std::optional<fetchers::Cache::Key> cacheKey;
 
     auto [subpath, fingerprint] =
@@ -38,32 +44,47 @@ StorePath fetchToStore(
         : path.accessor->getFingerprint(path.path);
 
     if (fingerprint) {
-        cacheKey = makeFetchToStoreCacheKey(std::string{name}, *fingerprint, method, subpath.abs());
+        cacheKey = makeSourcePathToHashCacheKey(*fingerprint, method, subpath.abs());
-        if (auto res = fetchers::getCache()->lookupStorePath(*cacheKey, store, mode == FetchMode::DryRun)) {
+        if (auto res = fetchers::getCache()->lookup(*cacheKey)) {
-            debug("store path cache hit for '%s'", path);
+            debug("source path hash cache hit for '%s'", path);
-            return res->storePath;
+            auto hash = Hash::parseSRI(fetchers::getStrAttr(*res, "hash"));
+            auto storePath = store.makeFixedOutputPathFromCA(name,
+                ContentAddressWithReferences::fromParts(method, hash, {}));
+            if (store.isValidPath(storePath)) {
+                debug("source path '%s' has valid store path '%s'", path, store.printStorePath(storePath));
+                return {storePath, hash};
+            }
+            debug("source path '%s' not in store", path);
         }
     } else
-        debug("source path '%s' is uncacheable (%d, %d)", path, filter, (bool) fingerprint);
+        // FIXME: could still provide in-memory caching keyed on `SourcePath`.
+        debug("source path '%s' is uncacheable (%d, %d)", path, (bool) filter, (bool) fingerprint);
 
     Activity act(*logger, lvlChatty, actUnknown,
         fmt(mode == FetchMode::DryRun ? "hashing '%s'" : "copying '%s' to the store", path));
 
     auto filter2 = filter ? *filter : defaultPathFilter;
 
-    auto storePath =
+    if (mode == FetchMode::DryRun) {
-        mode == FetchMode::DryRun
+        auto [storePath, hash] = store.computeStorePath(
-        ? store.computeStorePath(
+            name, path, method, HashAlgorithm::SHA256, {}, filter2);
-            name, path, method, HashAlgorithm::SHA256, {}, filter2).first
+        debug("hashed '%s' to '%s'", path, store.printStorePath(storePath));
-        : store.addToStore(
-            name, path, method, HashAlgorithm::SHA256, {}, filter2, repair);
-
-    debug(mode == FetchMode::DryRun ? "hashed '%s'" : "copied '%s' to '%s'", path, store.printStorePath(storePath));
-
         if (cacheKey)
-        fetchers::getCache()->upsert(*cacheKey, store, {}, storePath);
+            fetchers::getCache()->upsert(*cacheKey, {{"hash", hash.to_string(HashFormat::SRI, true)}});
-
+        return {storePath, hash};
-    return storePath;
+    } else {
+        auto storePath = store.addToStore(
+            name, path, method, HashAlgorithm::SHA256, {}, filter2, repair);
+        debug("copied '%s' to '%s'", path, store.printStorePath(storePath));
+        // FIXME: this is the wrong hash when method !=
+        // ContentAddressMethod::Raw::NixArchive. Doesn't matter at
+        // the moment since the only place where that's the case
+        // doesn't use the hash.
+        auto hash = store.queryPathInfo(storePath)->narHash;
+        if (cacheKey)
+            fetchers::getCache()->upsert(*cacheKey, {{"hash", hash.to_string(HashFormat::SRI, true)}});
+        return {storePath, hash};
+    }
 }
 
 }

src/libfetchers/fetchers.cc

@@ -352,7 +352,7 @@ std::pair<ref<SourceAccessor>, Input> Input::getAccessorUnchecked(ref<Store> sto
 
     auto [accessor, result] = scheme->getAccessor(store, *this);
 
-    assert(!accessor->fingerprint);
+    if (!accessor->fingerprint)
         accessor->fingerprint = result.getFingerprint(store);
 
     return {accessor, std::move(result)};

src/libfetchers/include/nix/fetchers/fetch-to-store.hh

@@ -23,7 +23,16 @@ StorePath fetchToStore(
     PathFilter * filter = nullptr,
     RepairFlag repair = NoRepair);
 
-fetchers::Cache::Key makeFetchToStoreCacheKey(
+std::pair<StorePath, Hash> fetchToStore2(
-    const std::string & name, const std::string & fingerprint, ContentAddressMethod method, const std::string & path);
+    Store & store,
+    const SourcePath & path,
+    FetchMode mode,
+    std::string_view name = "source",
+    ContentAddressMethod method = ContentAddressMethod::Raw::NixArchive,
+    PathFilter * filter = nullptr,
+    RepairFlag repair = NoRepair);
+
+fetchers::Cache::Key
+makeSourcePathToHashCacheKey(const std::string & fingerprint, ContentAddressMethod method, const std::string & path);
 
 }

src/libfetchers/path.cc

@@ -144,37 +144,22 @@ struct PathInputScheme : InputScheme
             storePath = store->addToStoreFromDump(*src, "source");
         }
 
-        // To avoid copying the path again to the /nix/store, we need to add a cache entry.
+        auto accessor = makeStorePathAccessor(store, *storePath);
-        ContentAddressMethod method = ContentAddressMethod::Raw::NixArchive;
+
-        auto fp = getFingerprint(store, input);
+        // To prevent `fetchToStore()` copying the path again to Nix
-        if (fp) {
+        // store, pre-create an entry in the fetcher cache.
-            auto cacheKey = makeFetchToStoreCacheKey(input.getName(), *fp, method, "/");
+        auto info = store->queryPathInfo(*storePath);
-            fetchers::getCache()->upsert(cacheKey, *store, {}, *storePath);
+        accessor->fingerprint = fmt("path:%s", store->queryPathInfo(*storePath)->narHash.to_string(HashFormat::SRI, true));
-        }
+        fetchers::getCache()->upsert(
+            makeSourcePathToHashCacheKey(*accessor->fingerprint, ContentAddressMethod::Raw::NixArchive, "/"),
+            {{"hash", info->narHash.to_string(HashFormat::SRI, true)}});
 
         /* Trust the lastModified value supplied by the user, if
            any. It's not a "secure" attribute so we don't care. */
         if (!input.getLastModified())
             input.attrs.insert_or_assign("lastModified", uint64_t(mtime));
 
-        return {makeStorePathAccessor(store, *storePath), std::move(input)};
+        return {accessor, std::move(input)};
-    }
-
-    std::optional<std::string> getFingerprint(ref<Store> store, const Input & input) const override
-    {
-        if (isRelative(input))
-            return std::nullopt;
-
-        /* If this path is in the Nix store, use the hash of the
-           store object and the subpath. */
-        auto path = getAbsPath(input);
-        try {
-            auto [storePath, subPath] = store->toStorePath(path.string());
-            auto info = store->queryPathInfo(storePath);
-            return fmt("path:%s:%s", info->narHash.to_string(HashFormat::Base16, false), subPath);
-        } catch (Error &) {
-            return std::nullopt;
-        }
     }
 };
 

tests/functional/flakes/flakes.sh

@@ -115,7 +115,7 @@ nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir?ref=HEAD#default"
 # Check that the fetcher cache works.
 if [[ $(nix config show lazy-trees) = false ]]; then
     nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir?ref=HEAD#default" -vvvvv 2>&1 | grepQuietInverse "source path.*is uncacheable"
-    nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir?ref=HEAD#default" -vvvvv 2>&1 | grepQuiet "store path cache hit"
+    nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir?ref=HEAD#default" -vvvvv 2>&1 | grepQuiet "source path hash cache hit"
 fi
 
 # Check that relative paths are allowed for git flakes.