Merge pull request #14050 from NixOS/fix-fetch-to-store-caching

Fix fetchToStore caching

Author: edolstra

src/libexpr/eval.cc

@@ -236,24 +236,17 @@ EvalState::EvalState(
           {CanonPath(store->storeDir), store->getFSAccessor(settings.pureEval)},
       }))
     , rootFS([&] {
-        auto accessor = [&]() -> decltype(rootFS) {
-            /* In pure eval mode, we provide a filesystem that only
-               contains the Nix store. */
-            if (settings.pureEval)
-                return storeFS;
-
-            /* If we have a chroot store and pure eval is not enabled,
-               use a union accessor to make the chroot store available
-               at its logical location while still having the underlying
-               directory available. This is necessary for instance if
-               we're evaluating a file from the physical /nix/store
-               while using a chroot store. */
-            auto realStoreDir = dirOf(store->toRealPath(StorePath::dummy));
-            if (store->storeDir != realStoreDir)
-                return makeUnionSourceAccessor({getFSSourceAccessor(), storeFS});
-
-            return getFSSourceAccessor();
-        }();
+        /* In pure eval mode, we provide a filesystem that only
+           contains the Nix store.
+
+           Otherwise, use a union accessor to make the augmented store
+           available at its logical location while still having the
+           underlying directory available. This is necessary for
+           instance if we're evaluating a file from the physical
+           /nix/store while using a chroot store, and also for lazy
+           mounted fetchTree. */
+        auto accessor = settings.pureEval ? storeFS.cast<SourceAccessor>()
+                                          : makeUnionSourceAccessor({getFSSourceAccessor(), storeFS});
 
         /* Apply access control if needed. */
         if (settings.restrictEval || settings.pureEval)
@@ -3133,6 +3126,11 @@ SourcePath EvalState::findFile(const LookupPath & lookupPath, const std::string_
         auto res = (r / CanonPath(suffix)).resolveSymlinks();
         if (res.pathExists())
             return res;
+
+        // Backward compatibility hack: throw an exception if access
+        // to this path is not allowed.
+        if (auto accessor = res.accessor.dynamic_pointer_cast<FilteringSourceAccessor>())
+            accessor->checkAccess(res.path);
     }
 
     if (hasPrefix(path, "nix/"))
@@ -3199,6 +3197,11 @@ std::optional<SourcePath> EvalState::resolveLookupPathPath(const LookupPath::Pat
         if (path.resolveSymlinks().pathExists())
             return finish(std::move(path));
         else {
+            // Backward compatibility hack: throw an exception if access
+            // to this path is not allowed.
+            if (auto accessor = path.accessor.dynamic_pointer_cast<FilteringSourceAccessor>())
+                accessor->checkAccess(path.path);
+
             logWarning({.msg = HintFmt("Nix search path entry '%1%' does not exist, ignoring", value)});
         }
     }

src/libexpr/include/nix/expr/eval.hh

@@ -42,6 +42,7 @@ class Store;
 namespace fetchers {
 struct Settings;
 struct InputCache;
+struct Input;
 } // namespace fetchers
 struct EvalSettings;
 class EvalState;
@@ -575,6 +576,11 @@ public:
 
     void checkURI(const std::string & uri);
 
+    /**
+     * Mount an input on the Nix store.
+     */
+    StorePath mountInput(fetchers::Input & input, const fetchers::Input & originalInput, ref<SourceAccessor> accessor);
+
     /**
      * Parse a Nix expression from the specified file.
      */

src/libexpr/paths.cc

@@ -1,5 +1,7 @@
 #include "nix/store/store-api.hh"
 #include "nix/expr/eval.hh"
+#include "nix/util/mounted-source-accessor.hh"
+#include "nix/fetchers/fetch-to-store.hh"
 
 namespace nix {
 
@@ -18,4 +20,27 @@ SourcePath EvalState::storePath(const StorePath & path)
     return {rootFS, CanonPath{store->printStorePath(path)}};
 }
 
+StorePath
+EvalState::mountInput(fetchers::Input & input, const fetchers::Input & originalInput, ref<SourceAccessor> accessor)
+{
+    auto storePath = fetchToStore(fetchSettings, *store, accessor, FetchMode::Copy, input.getName());
+
+    allowPath(storePath); // FIXME: should just whitelist the entire virtual store
+
+    storeFS->mount(CanonPath(store->printStorePath(storePath)), accessor);
+
+    auto narHash = store->queryPathInfo(storePath)->narHash;
+    input.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));
+
+    if (originalInput.getNarHash() && narHash != *originalInput.getNarHash())
+        throw Error(
+            (unsigned int) 102,
+            "NAR hash mismatch in input '%s', expected '%s' but got '%s'",
+            originalInput.to_string(),
+            narHash.to_string(HashFormat::SRI, true),
+            originalInput.getNarHash()->to_string(HashFormat::SRI, true));
+
+    return storePath;
+}
+
 } // namespace nix

src/libexpr/primops/fetchTree.cc

@@ -10,6 +10,7 @@
 #include "nix/util/url.hh"
 #include "nix/expr/value-to-json.hh"
 #include "nix/fetchers/fetch-to-store.hh"
+#include "nix/fetchers/input-cache.hh"
 
 #include <nlohmann/json.hpp>
 
@@ -218,11 +219,11 @@ static void fetchTree(
             throw Error("input '%s' is not allowed to use the '__final' attribute", input.to_string());
     }
 
-    auto [storePath, input2] = input.fetchToStore(state.store);
+    auto cachedInput = state.inputCache->getAccessor(state.store, input, fetchers::UseRegistries::No);
 
-    state.allowPath(storePath);
+    auto storePath = state.mountInput(cachedInput.lockedInput, input, cachedInput.accessor);
 
-    emitTreeAttrs(state, storePath, input2, v, params.emptyRevFallback, false);
+    emitTreeAttrs(state, storePath, cachedInput.lockedInput, v, params.emptyRevFallback, false);
 }
 
 static void prim_fetchTree(EvalState & state, const PosIdx pos, Value ** args, Value & v)

src/libfetchers/fetch-to-store.cc

@@ -1,6 +1,7 @@
 #include "nix/fetchers/fetch-to-store.hh"
 #include "nix/fetchers/fetchers.hh"
 #include "nix/fetchers/fetch-settings.hh"
+#include "nix/util/environment-variables.hh"
 
 namespace nix {
 
@@ -27,14 +28,22 @@ StorePath fetchToStore(
 
     std::optional<fetchers::Cache::Key> cacheKey;
 
-    if (!filter && path.accessor->fingerprint) {
-        cacheKey = makeFetchToStoreCacheKey(std::string{name}, *path.accessor->fingerprint, method, path.path.abs());
+    auto [subpath, fingerprint] = filter ? std::pair<CanonPath, std::optional<std::string>>{path.path, std::nullopt}
+                                         : path.accessor->getFingerprint(path.path);
+
+    if (fingerprint) {
+        cacheKey = makeFetchToStoreCacheKey(std::string{name}, *fingerprint, method, subpath.abs());
         if (auto res = settings.getCache()->lookupStorePath(*cacheKey, store)) {
             debug("store path cache hit for '%s'", path);
             return res->storePath;
         }
-    } else
+    } else {
+        static auto barf = getEnv("_NIX_TEST_BARF_ON_UNCACHEABLE").value_or("") == "1";
+        if (barf && !filter)
+            throw Error("source path '%s' is uncacheable (filter=%d)", path, (bool) filter);
+        // FIXME: could still provide in-memory caching keyed on `SourcePath`.
         debug("source path '%s' is uncacheable", path);
+    }
 
     Activity act(
         *logger,

src/libfetchers/fetchers.cc

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

src/libfetchers/filtering-source-accessor.cc

@@ -16,15 +16,26 @@ std::string FilteringSourceAccessor::readFile(const CanonPath & path)
     return next->readFile(prefix / path);
 }
 
+void FilteringSourceAccessor::readFile(const CanonPath & path, Sink & sink, std::function<void(uint64_t)> sizeCallback)
+{
+    checkAccess(path);
+    return next->readFile(prefix / path, sink, sizeCallback);
+}
+
 bool FilteringSourceAccessor::pathExists(const CanonPath & path)
 {
     return isAllowed(path) && next->pathExists(prefix / path);
 }
 
 std::optional<SourceAccessor::Stat> FilteringSourceAccessor::maybeLstat(const CanonPath & path)
+{
+    return isAllowed(path) ? next->maybeLstat(prefix / path) : std::nullopt;
+}
+
+SourceAccessor::Stat FilteringSourceAccessor::lstat(const CanonPath & path)
 {
     checkAccess(path);
-    return next->maybeLstat(prefix / path);
+    return next->lstat(prefix / path);
 }
 
 SourceAccessor::DirEntries FilteringSourceAccessor::readDirectory(const CanonPath & path)
@@ -49,6 +60,13 @@ std::string FilteringSourceAccessor::showPath(const CanonPath & path)
     return displayPrefix + next->showPath(prefix / path) + displaySuffix;
 }
 
+std::pair<CanonPath, std::optional<std::string>> FilteringSourceAccessor::getFingerprint(const CanonPath & path)
+{
+    if (fingerprint)
+        return {path, fingerprint};
+    return next->getFingerprint(prefix / path);
+}
+
 void FilteringSourceAccessor::checkAccess(const CanonPath & path)
 {
     if (!isAllowed(path))

src/libfetchers/git.cc

@@ -893,8 +893,7 @@ struct GitInputScheme : InputScheme
             return makeFingerprint(*rev);
         else {
             auto repoInfo = getRepoInfo(input);
-            if (auto repoPath = repoInfo.getPath();
-                repoPath && repoInfo.workdirInfo.headRev && repoInfo.workdirInfo.submodules.empty()) {
+            if (auto repoPath = repoInfo.getPath(); repoPath && repoInfo.workdirInfo.submodules.empty()) {
                 /* Calculate a fingerprint that takes into account the
                    deleted and modified/added files. */
                 HashSink hashSink{HashAlgorithm::SHA512};
@@ -907,7 +906,7 @@ struct GitInputScheme : InputScheme
                     writeString("deleted:", hashSink);
                     writeString(file.abs(), hashSink);
                 }
-                return makeFingerprint(*repoInfo.workdirInfo.headRev)
+                return makeFingerprint(repoInfo.workdirInfo.headRev.value_or(nullRev))
                        + ";d=" + hashSink.finish().hash.to_string(HashFormat::Base16, false);
             }
             return std::nullopt;

src/libfetchers/include/nix/fetchers/filtering-source-accessor.hh

@@ -36,8 +36,12 @@ struct FilteringSourceAccessor : SourceAccessor
 
     std::string readFile(const CanonPath & path) override;
 
+    void readFile(const CanonPath & path, Sink & sink, std::function<void(uint64_t)> sizeCallback) override;
+
     bool pathExists(const CanonPath & path) override;
 
+    Stat lstat(const CanonPath & path) override;
+
     std::optional<Stat> maybeLstat(const CanonPath & path) override;
 
     DirEntries readDirectory(const CanonPath & path) override;
@@ -46,6 +50,8 @@ struct FilteringSourceAccessor : SourceAccessor
 
     std::string showPath(const CanonPath & path) override;
 
+    std::pair<CanonPath, std::optional<std::string>> getFingerprint(const CanonPath & path) override;
+
     /**
      * Call `makeNotAllowedError` to throw a `RestrictedPathError`
      * exception if `isAllowed()` returns `false` for `path`.

src/libfetchers/path.cc

@@ -123,8 +123,6 @@ struct PathInputScheme : InputScheme
 
         auto absPath = getAbsPath(input);
 
-        Activity act(*logger, lvlTalkative, actUnknown, fmt("copying %s to the store", absPath));
-
         // FIXME: check whether access to 'path' is allowed.
         auto storePath = store->maybeParseStorePath(absPath.string());
 
@@ -133,43 +131,33 @@ struct PathInputScheme : InputScheme
 
         time_t mtime = 0;
         if (!storePath || storePath->name() != "source" || !store->isValidPath(*storePath)) {
+            Activity act(*logger, lvlTalkative, actUnknown, fmt("copying %s to the store", absPath));
             // FIXME: try to substitute storePath.
             auto src = sinkToSource(
                 [&](Sink & sink) { mtime = dumpPathAndGetMtime(absPath.string(), sink, defaultPathFilter); });
             storePath = store->addToStoreFromDump(*src, "source");
         }
 
-        // To avoid copying the path again to the /nix/store, we need to add a cache entry.
-        ContentAddressMethod method = ContentAddressMethod::Raw::NixArchive;
-        auto fp = getFingerprint(store, input);
-        if (fp) {
-            auto cacheKey = makeFetchToStoreCacheKey(input.getName(), *fp, method, "/");
-            input.settings->getCache()->upsert(cacheKey, *store, {}, *storePath);
-        }
+        auto accessor = ref{store->getFSAccessor(*storePath)};
+
+        // To prevent `fetchToStore()` copying the path again to Nix
+        // store, pre-create an entry in the fetcher cache.
+        auto info = store->queryPathInfo(*storePath);
+        accessor->fingerprint =
+            fmt("path:%s", store->queryPathInfo(*storePath)->narHash.to_string(HashFormat::SRI, true));
+        input.settings->getCache()->upsert(
+            makeFetchToStoreCacheKey(
+                input.getName(), *accessor->fingerprint, ContentAddressMethod::Raw::NixArchive, "/"),
+            *store,
+            {},
+            *storePath);
 
         /* 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 {ref{store->getFSAccessor(*storePath)}, 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;
-        }
+        return {accessor, std::move(input)};
     }
 
     std::optional<ExperimentalFeature> experimentalFeature() const override