Create a second Store::getFSAccessor for a single store object

Ericson2314 · Ericson2314 · commit cd83dd5d55d7 · 2025-09-22T15:53:45.000-04:00
This is sometimes easier / more performant to implement, and
independently it is also a more convenient interface for many callers.
diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc
@@ -3,7 +3,6 @@
 #include "nix/util/source-path.hh"
 #include "nix/fetchers/fetch-to-store.hh"
 #include "nix/util/json-utils.hh"
-#include "nix/fetchers/store-path-accessor.hh"
 #include "nix/fetchers/fetch-settings.hh"
 
 #include <nlohmann/json.hpp>
@@ -332,7 +331,8 @@ std::pair<ref<SourceAccessor>, Input> Input::getAccessorUnchecked(ref<Store> sto
 
             debug("using substituted/cached input '%s' in '%s'", to_string(), store->printStorePath(storePath));
 
-            auto accessor = makeStorePathAccessor(store, storePath);
+            // We just ensured the store object was there
+            auto accessor = ref{store->getFSAccessor(storePath)};
 
             accessor->fingerprint = getFingerprint(store);
 
diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc
@@ -398,9 +398,8 @@ struct GitHubInputScheme : GitArchiveInputScheme
 
         Headers headers = makeHeadersWithAuthTokens(*input.settings, host, input);
 
-        auto accessor = store->getFSAccessor();
         auto downloadResult = downloadFile(store, *input.settings, url, "source", headers);
-        auto json = nlohmann::json::parse(accessor->readFile(CanonPath(downloadResult.storePath.to_string())));
+        auto json = nlohmann::json::parse(store->getFSAccessor(downloadResult.storePath)->readFile(CanonPath::root));
 
         return RefInfo{
             .rev = Hash::parseAny(std::string{json["sha"]}, HashAlgorithm::SHA1),
@@ -473,9 +472,8 @@ struct GitLabInputScheme : GitArchiveInputScheme
 
         Headers headers = makeHeadersWithAuthTokens(*input.settings, host, input);
 
-        auto accessor = store->getFSAccessor();
         auto downloadResult = downloadFile(store, *input.settings, url, "source", headers);
-        auto json = nlohmann::json::parse(accessor->readFile(CanonPath(downloadResult.storePath.to_string())));
+        auto json = nlohmann::json::parse(store->getFSAccessor(downloadResult.storePath)->readFile(CanonPath::root));
 
         if (json.is_array() && json.size() >= 1 && json[0]["id"] != nullptr) {
             return RefInfo{.rev = Hash::parseAny(std::string(json[0]["id"]), HashAlgorithm::SHA1)};
diff --git a/src/libfetchers/include/nix/fetchers/meson.build b/src/libfetchers/include/nix/fetchers/meson.build
@@ -11,6 +11,5 @@ headers = files(
   'git-utils.hh',
   'input-cache.hh',
   'registry.hh',
-  'store-path-accessor.hh',
   'tarball.hh',
 )
diff --git a/src/libfetchers/include/nix/fetchers/store-path-accessor.hh b/src/libfetchers/include/nix/fetchers/store-path-accessor.hh
diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc
@@ -6,7 +6,6 @@
 #include "nix/util/tarfile.hh"
 #include "nix/store/store-api.hh"
 #include "nix/util/url-parts.hh"
-#include "nix/fetchers/store-path-accessor.hh"
 #include "nix/fetchers/fetch-settings.hh"
 
 #include <sys/time.h>
@@ -331,7 +330,8 @@ struct MercurialInputScheme : InputScheme
 
         auto storePath = fetchToStore(store, input);
 
-        auto accessor = makeStorePathAccessor(store, storePath);
+        // We just added it, it should be there.
+        auto accessor = ref{store->getFSAccessor(storePath)};
 
         accessor->setPathDisplay("«" + input.to_string() + "»");
 
diff --git a/src/libfetchers/meson.build b/src/libfetchers/meson.build
@@ -50,7 +50,6 @@ sources = files(
   'mercurial.cc',
   'path.cc',
   'registry.cc',
-  'store-path-accessor.cc',
   'tarball.cc',
 )
 
diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc
@@ -1,7 +1,6 @@
 #include "nix/fetchers/fetchers.hh"
 #include "nix/store/store-api.hh"
 #include "nix/util/archive.hh"
-#include "nix/fetchers/store-path-accessor.hh"
 #include "nix/fetchers/cache.hh"
 #include "nix/fetchers/fetch-to-store.hh"
 #include "nix/fetchers/fetch-settings.hh"
@@ -153,7 +152,7 @@ struct PathInputScheme : InputScheme
         if (!input.getLastModified())
             input.attrs.insert_or_assign("lastModified", uint64_t(mtime));
 
-        return {makeStorePathAccessor(store, *storePath), std::move(input)};
+        return {ref{store->getFSAccessor(*storePath)}, std::move(input)};
     }
 
     std::optional<std::string> getFingerprint(ref<Store> store, const Input & input) const override
diff --git a/src/libfetchers/store-path-accessor.cc b/src/libfetchers/store-path-accessor.cc
diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc
@@ -6,7 +6,6 @@
 #include "nix/util/archive.hh"
 #include "nix/util/tarfile.hh"
 #include "nix/util/types.hh"
-#include "nix/fetchers/store-path-accessor.hh"
 #include "nix/store/store-api.hh"
 #include "nix/fetchers/git-utils.hh"
 #include "nix/fetchers/fetch-settings.hh"
@@ -354,7 +353,7 @@ struct FileInputScheme : CurlInputScheme
         auto narHash = store->queryPathInfo(file.storePath)->narHash;
         input.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));
 
-        auto accessor = makeStorePathAccessor(store, file.storePath);
+        auto accessor = ref{store->getFSAccessor(file.storePath)};
 
         accessor->setPathDisplay("«" + input.to_string() + "»");
 
diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc
@@ -539,11 +539,21 @@ void BinaryCacheStore::registerDrvOutput(const Realisation & info)
     upsertFile(filePath, static_cast<nlohmann::json>(info).dump(), "application/json");
 }
 
-ref<SourceAccessor> BinaryCacheStore::getFSAccessor(bool requireValidPath)
+ref<RemoteFSAccessor> BinaryCacheStore::getRemoteFSAccessor(bool requireValidPath)
 {
     return make_ref<RemoteFSAccessor>(ref<Store>(shared_from_this()), requireValidPath, config.localNarCache);
 }
 
+ref<SourceAccessor> BinaryCacheStore::getFSAccessor(bool requireValidPath)
+{
+    return getRemoteFSAccessor(requireValidPath);
+}
+
+std::shared_ptr<SourceAccessor> BinaryCacheStore::getFSAccessor(const StorePath & storePath, bool requireValidPath)
+{
+    return getRemoteFSAccessor(requireValidPath)->accessObject(storePath);
+}
+
 void BinaryCacheStore::addSignatures(const StorePath & storePath, const StringSet & sigs)
 {
     /* Note: this is inherently racy since there is no locking on
diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc
@@ -276,7 +276,14 @@ struct DummyStore : virtual Store
         callback(nullptr);
     }
 
-    virtual ref<SourceAccessor> getFSAccessor(bool requireValidPath) override
+    std::shared_ptr<SourceAccessor> getFSAccessor(const StorePath & path, bool requireValidPath) override
+    {
+        std::shared_ptr<SourceAccessor> res;
+        contents.cvisit(path, [&](const auto & kv) { res = kv.second.contents.get_ptr(); });
+        return res;
+    }
+
+    ref<SourceAccessor> getFSAccessor(bool requireValidPath) override
     {
         return wholeStoreView;
     }
diff --git a/src/libstore/include/nix/store/binary-cache-store.hh b/src/libstore/include/nix/store/binary-cache-store.hh
@@ -12,6 +12,7 @@
 namespace nix {
 
 struct NarInfo;
+class RemoteFSAccessor;
 
 struct BinaryCacheStoreConfig : virtual StoreConfig
 {
@@ -136,6 +137,11 @@ private:
         CheckSigsFlag checkSigs,
         std::function<ValidPathInfo(HashResult)> mkInfo);
 
+    /**
+     * Same as `getFSAccessor`, but with a more preceise return type.
+     */
+    ref<RemoteFSAccessor> getRemoteFSAccessor(bool requireValidPath = true);
+
 public:
 
     bool isValidPathUncached(const StorePath & path) override;
@@ -175,6 +181,8 @@ public:
 
     ref<SourceAccessor> getFSAccessor(bool requireValidPath = true) override;
 
+    std::shared_ptr<SourceAccessor> getFSAccessor(const StorePath &, bool requireValidPath = true) override;
+
     void addSignatures(const StorePath & storePath, const StringSet & sigs) override;
 
     std::optional<std::string> getBuildLogExact(const StorePath & path) override;
diff --git a/src/libstore/include/nix/store/legacy-ssh-store.hh b/src/libstore/include/nix/store/legacy-ssh-store.hh
@@ -142,7 +142,12 @@ public:
         unsupported("ensurePath");
     }
 
-    virtual ref<SourceAccessor> getFSAccessor(bool requireValidPath) override
+    ref<SourceAccessor> getFSAccessor(bool requireValidPath) override
+    {
+        unsupported("getFSAccessor");
+    }
+
+    std::shared_ptr<SourceAccessor> getFSAccessor(const StorePath & path, bool requireValidPath) override
     {
         unsupported("getFSAccessor");
     }
diff --git a/src/libstore/include/nix/store/local-fs-store.hh b/src/libstore/include/nix/store/local-fs-store.hh
@@ -68,6 +68,7 @@ struct LocalFSStore : virtual Store, virtual GcStore, virtual LogStore
 
     void narFromPath(const StorePath & path, Sink & sink) override;
     ref<SourceAccessor> getFSAccessor(bool requireValidPath = true) override;
+    std::shared_ptr<SourceAccessor> getFSAccessor(const StorePath & path, bool requireValidPath = true) override;
 
     /**
      * Creates symlink from the `gcRoot` to the `storePath` and
diff --git a/src/libstore/include/nix/store/remote-fs-accessor.hh b/src/libstore/include/nix/store/remote-fs-accessor.hh
@@ -27,6 +27,11 @@ class RemoteFSAccessor : public SourceAccessor
 
 public:
 
+    /**
+     * @returns null if the store does not contain any object at that path.
+     */
+    std::shared_ptr<SourceAccessor> accessObject(const StorePath & path);
+
     RemoteFSAccessor(
         ref<Store> store, bool requireValidPath = true, const /* FIXME: use std::optional */ Path & cacheDir = "");
 
diff --git a/src/libstore/include/nix/store/remote-store.hh b/src/libstore/include/nix/store/remote-store.hh
@@ -16,6 +16,7 @@ struct FdSink;
 struct FdSource;
 template<typename T>
 class Pool;
+class RemoteFSAccessor;
 
 struct RemoteStoreConfig : virtual StoreConfig
 {
@@ -176,10 +177,18 @@ protected:
 
     virtual ref<SourceAccessor> getFSAccessor(bool requireValidPath = true) override;
 
+    virtual std::shared_ptr<SourceAccessor>
+    getFSAccessor(const StorePath & path, bool requireValidPath = true) override;
+
     virtual void narFromPath(const StorePath & path, Sink & sink) override;
 
 private:
 
+    /**
+     * Same as the default implemenation of `RemoteStore::getFSAccessor`, but with a more preceise return type.
+     */
+    ref<RemoteFSAccessor> getRemoteFSAccessor(bool requireValidPath = true);
+
     std::atomic_bool failed{false};
 
     void copyDrvsFromEvalStore(const std::vector<DerivedPath> & paths, std::shared_ptr<Store> evalStore);
diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh
@@ -717,10 +717,20 @@ public:
     };
 
     /**
-     * @return An object to access files in the Nix store.
+     * @return An object to access files in the Nix store, across all
+     * store objects.
      */
     virtual ref<SourceAccessor> getFSAccessor(bool requireValidPath = true) = 0;
 
+    /**
+     * @return An object to access files for a specific store object in
+     * the Nix store.
+     *
+     * @returns null if the store doesn't contain an object at the
+     * givine path.
+     */
+    virtual std::shared_ptr<SourceAccessor> getFSAccessor(const StorePath & path, bool requireValidPath = true) = 0;
+
     /**
      * Repair the contents of the given path by redownloading it using
      * a substituter (if available).
diff --git a/src/libstore/include/nix/store/uds-remote-store.hh b/src/libstore/include/nix/store/uds-remote-store.hh
@@ -61,6 +61,11 @@ struct UDSRemoteStore : virtual IndirectRootStore, virtual RemoteStore
         return LocalFSStore::getFSAccessor(requireValidPath);
     }
 
+    std::shared_ptr<SourceAccessor> getFSAccessor(const StorePath & path, bool requireValidPath = true) override
+    {
+        return LocalFSStore::getFSAccessor(path, requireValidPath);
+    }
+
     void narFromPath(const StorePath & path, Sink & sink) override
     {
         LocalFSStore::narFromPath(path, sink);
diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc
@@ -36,6 +36,12 @@ LocalFSStore::LocalFSStore(const Config & config)
 {
 }
 
+static void requireStoreObject(Store & store, const StorePath & storePath, bool requireValidPath)
+{
+    if (requireValidPath && !store.isValidPath(storePath))
+        throw InvalidPath("path '%1%' is not a valid store path", store.printStorePath(storePath));
+}
+
 struct LocalStoreAccessor : PosixSourceAccessor
 {
     ref<LocalFSStore> store;
@@ -51,8 +57,7 @@ struct LocalStoreAccessor : PosixSourceAccessor
     void requireStoreObject(const CanonPath & path)
     {
         auto [storePath, rest] = store->toStorePath(store->storeDir + path.abs());
-        if (requireValidPath && !store->isValidPath(storePath))
-            throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath));
+        nix::requireStoreObject(*store, storePath, requireValidPath);
     }
 
     std::optional<Stat> maybeLstat(const CanonPath & path) override
@@ -91,6 +96,12 @@ ref<SourceAccessor> LocalFSStore::getFSAccessor(bool requireValidPath)
         ref<LocalFSStore>(std::dynamic_pointer_cast<LocalFSStore>(shared_from_this())), requireValidPath);
 }
 
+std::shared_ptr<SourceAccessor> LocalFSStore::getFSAccessor(const StorePath & path, bool requireValidPath)
+{
+    requireStoreObject(*this, path, requireValidPath);
+    return std::make_shared<PosixSourceAccessor>(std::filesystem::path{config.realStoreDir.get()} / path.to_string());
+}
+
 void LocalFSStore::narFromPath(const StorePath & path, Sink & sink)
 {
     if (!isValidPath(path))
diff --git a/src/libstore/remote-fs-accessor.cc b/src/libstore/remote-fs-accessor.cc
@@ -51,15 +51,17 @@ ref<SourceAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std:
 
 std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPath & path)
 {
-    auto [storePath, restPath_] = store->toStorePath(store->storeDir + path.abs());
-    auto restPath = CanonPath(restPath_);
-
+    auto [storePath, restPath] = store->toStorePath(store->storeDir + path.abs());
     if (requireValidPath && !store->isValidPath(storePath))
         throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath));
+    return {ref{accessObject(storePath)}, CanonPath{restPath}};
+}
 
+std::shared_ptr<SourceAccessor> RemoteFSAccessor::accessObject(const StorePath & storePath)
+{
     auto i = nars.find(std::string(storePath.hashPart()));
     if (i != nars.end())
-        return {i->second, restPath};
+        return i->second;
 
     std::string listing;
     Path cacheFile;
@@ -90,22 +92,22 @@ std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPat
             });
 
             nars.emplace(storePath.hashPart(), narAccessor);
-            return {narAccessor, restPath};
+            return narAccessor;
 
         } catch (SystemError &) {
         }
 
         try {
             auto narAccessor = makeNarAccessor(nix::readFile(cacheFile));
             nars.emplace(storePath.hashPart(), narAccessor);
-            return {narAccessor, restPath};
+            return narAccessor;
         } catch (SystemError &) {
         }
     }
 
     StringSink sink;
     store->narFromPath(storePath, sink);
-    return {addToCache(storePath.hashPart(), std::move(sink.s)), restPath};
+    return addToCache(storePath.hashPart(), std::move(sink.s));
 }
 
 std::optional<SourceAccessor::Stat> RemoteFSAccessor::maybeLstat(const CanonPath & path)
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
diff --git a/src/nix/cat.cc b/src/nix/cat.cc
diff --git a/src/nix/ls.cc b/src/nix/ls.cc
diff --git a/src/nix/nix-store/nix-store.cc b/src/nix/nix-store/nix-store.cc

Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,5 @@ headers = files(`
`11`	`11`	`'git-utils.hh',`
`12`	`12`	`'input-cache.hh',`
`13`	`13`	`'registry.hh',`
`14`		`- 'store-path-accessor.hh',`
`15`	`14`	`'tarball.hh',`
`16`	`15`	`)`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,6 @@ sources = files(`
`50`	`50`	`'mercurial.cc',`
`51`	`51`	`'path.cc',`
`52`	`52`	`'registry.cc',`
`53`		`- 'store-path-accessor.cc',`
`54`	`53`	`'tarball.cc',`
`55`	`54`	`)`
`56`	`55`
Original file line number	Diff line number	Diff line change
`@@ -539,11 +539,21 @@ void BinaryCacheStore::registerDrvOutput(const Realisation & info)`
`539`	`539`	`upsertFile(filePath, static_cast<nlohmann::json>(info).dump(), "application/json");`
`540`	`540`	`}`
`541`	`541`
`542`		`-ref<SourceAccessor> BinaryCacheStore::getFSAccessor(bool requireValidPath)`
	`542`	`+ref<RemoteFSAccessor> BinaryCacheStore::getRemoteFSAccessor(bool requireValidPath)`
`543`	`543`	`{`
`544`	`544`	`return make_ref<RemoteFSAccessor>(ref<Store>(shared_from_this()), requireValidPath, config.localNarCache);`
`545`	`545`	`}`
`546`	`546`
	`547`	`+ref<SourceAccessor> BinaryCacheStore::getFSAccessor(bool requireValidPath)`
	`548`	`+{`
	`549`	`+ return getRemoteFSAccessor(requireValidPath);`
	`550`	`+}`
	`551`	`+`
	`552`	`+std::shared_ptr<SourceAccessor> BinaryCacheStore::getFSAccessor(const StorePath & storePath, bool requireValidPath)`
	`553`	`+{`
	`554`	`+ return getRemoteFSAccessor(requireValidPath)->accessObject(storePath);`
	`555`	`+}`
	`556`	`+`
`547`	`557`	`void BinaryCacheStore::addSignatures(const StorePath & storePath, const StringSet & sigs)`
`548`	`558`	`{`
`549`	`559`	`/* Note: this is inherently racy since there is no locking on`