Merge pull request #115 from DeterminateSystems/fetchToStore-cache-hashes

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

.github/workflows/build.yml

@@ -87,7 +87,7 @@ jobs:
       - uses: DeterminateSystems/flakehub-cache-action@main
       - run: |
           cmd() {
-            nix build -L --keep-going --timeout 300 \
+            nix build -L --keep-going --timeout 600 \
               $(nix flake show --json \
                 | jq -r '
                   .hydraJobs.tests

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,57 @@ 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);
+            auto hash = Hash::parseSRI(fetchers::getStrAttr(*res, "hash"));
-            return res->storePath;
+            auto storePath = store.makeFixedOutputPathFromCA(name,
+                ContentAddressWithReferences::fromParts(method, hash, {}));
+            if (mode == FetchMode::DryRun || store.isValidPath(storePath)) {
+                debug("source path '%s' cache hit in '%s' (hash '%s')", path, store.printStorePath(storePath), hash.to_string(HashFormat::SRI, true));
+                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 =
+    auto [storePath, hash] =
         mode == FetchMode::DryRun
-        ? store.computeStorePath(
+        ? ({
-            name, path, method, HashAlgorithm::SHA256, {}, filter2).first
+            auto [storePath, hash] = store.computeStorePath(
-        : store.addToStore(
+                name, path, method, HashAlgorithm::SHA256, {}, filter2);
-            name, path, method, HashAlgorithm::SHA256, {}, filter2, repair);
+            debug("hashed '%s' to '%s' (hash '%s')", path, store.printStorePath(storePath), hash.to_string(HashFormat::SRI, true));
-
+            std::make_pair(storePath, hash);
-    debug(mode == FetchMode::DryRun ? "hashed '%s'" : "copied '%s' to '%s'", path, store.printStorePath(storePath));
+        })
+        : ({
+            // FIXME: ideally addToStore() would return the hash
+            // right away (like computeStorePath()).
+            auto storePath = store.addToStore(
+                name, path, method, HashAlgorithm::SHA256, {}, filter2, repair);
+            auto info = store.queryPathInfo(storePath);
+            assert(info->references.empty());
+            auto hash =
+                method == ContentAddressMethod::Raw::NixArchive
+                ? info->narHash
+                : ({
+                    if (!info->ca || info->ca->method != method)
+                        throw Error("path '%s' lacks a CA field", store.printStorePath(storePath));
+                    info->ca->hash;
+                });
+            debug("copied '%s' to '%s' (hash '%s')", path, store.printStorePath(storePath), hash.to_string(HashFormat::SRI, true));
+            std::make_pair(storePath, hash);
+        });
 
     if (cacheKey)
-        fetchers::getCache()->upsert(*cacheKey, store, {}, storePath);
+        fetchers::getCache()->upsert(*cacheKey, {{"hash", hash.to_string(HashFormat::SRI, true)}});
 
-    return storePath;
+    return {storePath, hash};
 }
 
 }

src/libfetchers/fetchers.cc

@@ -352,8 +352,8 @@ 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);
+        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;
-        }
     }
 };
 

src/libflake-tests/meson.build

@@ -59,6 +59,7 @@ test(
   this_exe,
   env : {
     '_NIX_TEST_UNIT_DATA': meson.current_source_dir() / 'data',
+    'HOME': meson.current_build_dir() / 'test-home',
   },
   protocol : 'gtest',
 )

src/libflake-tests/package.nix

@@ -56,17 +56,13 @@ mkMesonExecutable (finalAttrs: {
           {
             meta.broken = !stdenv.hostPlatform.emulatorAvailable buildPackages;
           }
-          (
+          (''
-            lib.optionalString stdenv.hostPlatform.isWindows ''
+            export _NIX_TEST_UNIT_DATA=${resolvePath ./data}
-              export HOME="$PWD/home-dir"
+            export HOME="$TMPDIR/home"
-              mkdir -p "$HOME"
+            mkdir -p "$HOME"
-            ''
+            ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage}
-            + ''
+            touch $out
-              export _NIX_TEST_UNIT_DATA=${resolvePath ./data}
+          '');
-              ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage}
-              touch $out
-            ''
-          );
     };
   };
 

src/libstore-tests/meson.build

@@ -100,6 +100,7 @@ test(
   this_exe,
   env : {
     '_NIX_TEST_UNIT_DATA': meson.current_source_dir() / 'data',
+    'HOME': meson.current_build_dir() / 'test-home',
   },
   protocol : 'gtest',
 )

src/libstore-tests/nix_api_store.cc

@@ -28,10 +28,6 @@ TEST_F(nix_api_store_test, nix_store_get_uri)
 
 TEST_F(nix_api_util_context, nix_store_get_storedir_default)
 {
-    if (nix::getEnv("HOME").value_or("") == "/homeless-shelter") {
-        // skipping test in sandbox because nix_store_open tries to create /nix/var/nix/profiles
-        GTEST_SKIP();
-    }
     nix_libstore_init(ctx);
     Store * store = nix_store_open(ctx, nullptr, nullptr);
     assert_ctx_ok();
@@ -136,10 +132,6 @@ TEST_F(nix_api_store_test, nix_store_real_path)
 
 TEST_F(nix_api_util_context, nix_store_real_path_relocated)
 {
-    if (nix::getEnv("HOME").value_or("") == "/homeless-shelter") {
-        // Can't open default store from within sandbox
-        GTEST_SKIP();
-    }
     auto tmp = nix::createTempDir();
     std::string storeRoot = tmp + "/store";
     std::string stateDir = tmp + "/state";
@@ -179,13 +171,7 @@ TEST_F(nix_api_util_context, nix_store_real_path_relocated)
 
 TEST_F(nix_api_util_context, nix_store_real_path_binary_cache)
 {
-    if (nix::getEnv("HOME").value_or("") == "/homeless-shelter") {
+    Store * store = nix_store_open(ctx, nix::fmt("file://%s/binary-cache", nix::createTempDir()).c_str(), nullptr);
-        // TODO: override NIX_CACHE_HOME?
-        // skipping test in sandbox because narinfo cache can't be written
-        GTEST_SKIP();
-    }
-
-    Store * store = nix_store_open(ctx, "https://cache.nixos.org", nullptr);
     assert_ctx_ok();
     ASSERT_NE(store, nullptr);
 

src/libstore-tests/package.nix

@@ -73,17 +73,13 @@ mkMesonExecutable (finalAttrs: {
           {
             meta.broken = !stdenv.hostPlatform.emulatorAvailable buildPackages;
           }
-          (
+          (''
-            lib.optionalString stdenv.hostPlatform.isWindows ''
+            export _NIX_TEST_UNIT_DATA=${data + "/src/libstore-tests/data"}
-              export HOME="$PWD/home-dir"
+            export HOME="$TMPDIR/home"
-              mkdir -p "$HOME"
+            mkdir -p "$HOME"
-            ''
+            ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage}
-            + ''
+            touch $out
-              export _NIX_TEST_UNIT_DATA=${data + "/src/libstore-tests/data"}
+          '');
-              ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage}
-              touch $out
-            ''
-          );
     };
   };
 

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.*cache hit"
 fi
 
 # Check that relative paths are allowed for git flakes.