Factor out NarCache from RemoteFSAccessor

Use

```
git show --color-moved --patience --color-moved-ws=ignore-all-space
```

to review and see that this is mostly code motion.

Co-Authored-By: John Ericson <John.Ericson@Obsidian.Systems>

src/libstore/include/nix/store/remote-fs-accessor.hh

@@ -3,6 +3,7 @@
 
 #include "nix/util/source-accessor.hh"
 #include "nix/util/ref.hh"
+#include "nix/util/nar-cache.hh"
 #include "nix/store/store-api.hh"
 
 namespace nix {
@@ -13,20 +14,15 @@ class RemoteFSAccessor : public SourceAccessor
 
     /**
      * Map from store path hash part to NAR hash. Used to then look up
-     * in `nars`. The indirection allows avoiding opening multiple
+     * in the NAR cache. The indirection allows avoiding opening multiple
      * redundant NAR accessors for the same NAR.
      */
     std::map<std::string, Hash, std::less<>> narHashes;
 
-    /**
-     * Map from NAR hash to NAR accessor.
-     */
-    std::map<Hash, ref<SourceAccessor>> nars;
+    NarCache narCache;
 
     bool requireValidPath;
 
-    std::optional<std::filesystem::path> cacheDir;
-
     std::pair<ref<SourceAccessor>, CanonPath> fetch(const CanonPath & path);
 
     friend struct BinaryCacheStore;

src/libstore/remote-fs-accessor.cc

@@ -1,21 +1,13 @@
-#include <nlohmann/json.hpp>
 #include "nix/store/remote-fs-accessor.hh"
-#include "nix/util/nar-accessor.hh"
-
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
 
 namespace nix {
 
 RemoteFSAccessor::RemoteFSAccessor(
-    ref<Store> store, bool requireValidPath, std::optional<std::filesystem::path> cacheDir_)
+    ref<Store> store, bool requireValidPath, std::optional<std::filesystem::path> cacheDir)
     : store(store)
+    , narCache(cacheDir)
     , requireValidPath(requireValidPath)
-    , cacheDir(std::move(cacheDir_))
 {
-    if (cacheDir)
-        createDirs(*cacheDir);
 }
 
 std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPath & path)
@@ -28,72 +20,18 @@ std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPat
 
 std::shared_ptr<SourceAccessor> RemoteFSAccessor::accessObject(const StorePath & storePath)
 {
-    if (auto * narHash = get(narHashes, storePath.hashPart())) {
-        if (auto * accessor = get(nars, *narHash))
-            return *accessor;
-    }
+    // Check if we already have the NAR hash for this store path
+    if (auto * narHash = get(narHashes, storePath.hashPart()))
+        return narCache.getOrInsert(*narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); });
 
+    // Query the path info to get the NAR hash
     auto info = store->queryPathInfo(storePath);
 
-    auto cacheAccessor = [&](ref<SourceAccessor> accessor) {
-        narHashes.emplace(storePath.hashPart(), info->narHash);
-        nars.emplace(info->narHash, accessor);
-        return accessor;
-    };
+    // Cache the mapping from store path to NAR hash
+    narHashes.emplace(storePath.hashPart(), info->narHash);
 
-    auto getNar = [&]() {
-        StringSink sink;
-        store->narFromPath(storePath, sink);
-        return std::move(sink.s);
-    };
-
-    if (cacheDir) {
-        auto makeCacheFile = [&](const std::string & ext) {
-            auto res = *cacheDir / info->narHash.to_string(HashFormat::Nix32, false);
-            res += ".";
-            res += ext;
-            return res;
-        };
-
-        auto cacheFile = makeCacheFile("nar");
-        auto listingFile = makeCacheFile("ls");
-
-        if (nix::pathExists(cacheFile)) {
-            try {
-                return cacheAccessor(makeLazyNarAccessor(
-                    nlohmann::json::parse(nix::readFile(listingFile)).template get<NarListing>(),
-                    seekableGetNarBytes(cacheFile)));
-            } catch (SystemError &) {
-            }
-
-            try {
-                return cacheAccessor(makeNarAccessor(nix::readFile(cacheFile)));
-            } catch (SystemError &) {
-            }
-        }
-
-        auto nar = getNar();
-
-        try {
-            /* FIXME: do this asynchronously. */
-            writeFile(cacheFile, nar);
-        } catch (...) {
-            ignoreExceptionExceptInterrupt();
-        }
-
-        auto narAccessor = makeNarAccessor(std::move(nar));
-
-        try {
-            nlohmann::json j = narAccessor->getListing();
-            writeFile(listingFile, j.dump());
-        } catch (...) {
-            ignoreExceptionExceptInterrupt();
-        }
-
-        return cacheAccessor(narAccessor);
-    }
-
-    return cacheAccessor(makeNarAccessor(getNar()));
+    // Get or create the NAR accessor
+    return narCache.getOrInsert(info->narHash, [&](Sink & sink) { store->narFromPath(storePath, sink); });
 }
 
 std::optional<SourceAccessor::Stat> RemoteFSAccessor::maybeLstat(const CanonPath & path)

src/libutil/include/nix/util/meson.build

@@ -53,6 +53,7 @@ headers = files(
   'mounted-source-accessor.hh',
   'muxable-pipe.hh',
   'nar-accessor.hh',
+  'nar-cache.hh',
   'nar-listing.hh',
   'os-string.hh',
   'pool.hh',

src/libutil/include/nix/util/nar-cache.hh

@@ -0,0 +1,47 @@
+#pragma once
+
+#include "nix/util/hash.hh"
+#include "nix/util/nar-accessor.hh"
+#include "nix/util/ref.hh"
+#include "nix/util/source-accessor.hh"
+
+#include <filesystem>
+#include <functional>
+#include <map>
+#include <optional>
+
+namespace nix {
+
+/**
+ * A cache for NAR accessors with optional disk caching.
+ */
+class NarCache
+{
+    /**
+     * Optional directory for caching NARs and listings on disk.
+     */
+    std::optional<std::filesystem::path> cacheDir;
+
+    /**
+     * Map from NAR hash to NAR accessor.
+     */
+    std::map<Hash, ref<SourceAccessor>> nars;
+
+public:
+
+    /**
+     * Create a NAR cache with an optional cache directory for disk storage.
+     */
+    NarCache(std::optional<std::filesystem::path> cacheDir = {});
+
+    /**
+     * Lookup or create a NAR accessor, optionally using disk cache.
+     *
+     * @param narHash The NAR hash to use as cache key
+     * @param populate Function called with a Sink to populate the NAR if not cached
+     * @return The cached or newly created accessor
+     */
+    ref<SourceAccessor> getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate);
+};
+
+} // namespace nix

src/libutil/meson.build

@@ -151,6 +151,7 @@ sources = [ config_priv_h ] + files(
   'memory-source-accessor/json.cc',
   'mounted-source-accessor.cc',
   'nar-accessor.cc',
+  'nar-cache.cc',
   'nar-listing.cc',
   'pos-table.cc',
   'position.cc',

src/libutil/nar-cache.cc

@@ -0,0 +1,84 @@
+#include "nix/util/nar-cache.hh"
+#include "nix/util/file-system.hh"
+
+#include <nlohmann/json.hpp>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+namespace nix {
+
+NarCache::NarCache(std::optional<std::filesystem::path> cacheDir_)
+    : cacheDir(std::move(cacheDir_))
+{
+    if (cacheDir)
+        createDirs(*cacheDir);
+}
+
+ref<SourceAccessor> NarCache::getOrInsert(const Hash & narHash, std::function<void(Sink &)> populate)
+{
+    // Check in-memory cache first
+    if (auto * accessor = get(nars, narHash))
+        return *accessor;
+
+    auto cacheAccessor = [&](ref<SourceAccessor> accessor) {
+        nars.emplace(narHash, accessor);
+        return accessor;
+    };
+
+    auto getNar = [&]() {
+        StringSink sink;
+        populate(sink);
+        return std::move(sink.s);
+    };
+
+    if (cacheDir) {
+        auto makeCacheFile = [&](const std::string & ext) {
+            auto res = *cacheDir / narHash.to_string(HashFormat::Nix32, false);
+            res += ".";
+            res += ext;
+            return res;
+        };
+
+        auto cacheFile = makeCacheFile("nar");
+        auto listingFile = makeCacheFile("ls");
+
+        if (nix::pathExists(cacheFile)) {
+            try {
+                return cacheAccessor(makeLazyNarAccessor(
+                    nlohmann::json::parse(nix::readFile(listingFile)).template get<NarListing>(),
+                    seekableGetNarBytes(cacheFile)));
+            } catch (SystemError &) {
+            }
+
+            try {
+                return cacheAccessor(makeNarAccessor(nix::readFile(cacheFile)));
+            } catch (SystemError &) {
+            }
+        }
+
+        auto nar = getNar();
+
+        try {
+            /* FIXME: do this asynchronously. */
+            writeFile(cacheFile, nar);
+        } catch (...) {
+            ignoreExceptionExceptInterrupt();
+        }
+
+        auto narAccessor = makeNarAccessor(std::move(nar));
+
+        try {
+            nlohmann::json j = narAccessor->getListing();
+            writeFile(listingFile, j.dump());
+        } catch (...) {
+            ignoreExceptionExceptInterrupt();
+        }
+
+        return cacheAccessor(narAccessor);
+    }
+
+    return cacheAccessor(makeNarAccessor(getNar()));
+}
+
+} // namespace nix