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

This is sometimes easier / more performant to implement, and
independently it is also a more convenient interface for many callers.

The existing store-wide `getFSAccessor` is only used for

 - `nix why-depends`
 - the evaluator

I hope we can get rid of it for those, too, and then we have the option
of getting rid of the store-wide method.

Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>

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);
 

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)};

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',
 )

src/libfetchers/include/nix/fetchers/store-path-accessor.hh

@@ -1,14 +0,0 @@
-#pragma once
-
-#include "nix/util/source-path.hh"
-
-namespace nix {
-
-class StorePath;
-class Store;
-
-ref<SourceAccessor> makeStorePathAccessor(ref<Store> store, const StorePath & storePath);
-
-SourcePath getUnfilteredRootPath(CanonPath path);
-
-} // namespace nix

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() + "»");
 

src/libfetchers/meson.build

@@ -50,7 +50,6 @@ sources = files(
   'mercurial.cc',
   'path.cc',
   'registry.cc',
-  'store-path-accessor.cc',
   'tarball.cc',
 )
 

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

src/libfetchers/store-path-accessor.cc

@@ -1,11 +0,0 @@
-#include "nix/fetchers/store-path-accessor.hh"
-#include "nix/store/store-api.hh"
-
-namespace nix {
-
-ref<SourceAccessor> makeStorePathAccessor(ref<Store> store, const StorePath & storePath)
-{
-    return projectSubdirSourceAccessor(store->getFSAccessor(), storePath.to_string());
-}
-
-} // namespace nix

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() + "»");
 

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

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;
     }

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;

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");
     }

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

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

@@ -27,6 +27,11 @@ class RemoteFSAccessor : public SourceAccessor
 
 public:
 
+    /**
+     * @return nullptr 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 = "");
 

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);

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.
+     *
+     * @return nullptr 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).

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);

src/libstore/local-fs-store.cc

@@ -91,6 +91,23 @@ 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)
+{
+    auto absPath = std::filesystem::path{config.realStoreDir.get()} / path.to_string();
+    if (requireValidPath) {
+        /* Only return non-null if the store object is a fully-valid
+           member of the store. */
+        if (!isValidPath(path))
+            return nullptr;
+    } else {
+        /* Return non-null as long as the some file system data exists,
+           even if the store object is not fully registered. */
+        if (!pathExists(absPath))
+            return nullptr;
+    }
+    return std::make_shared<PosixSourceAccessor>(std::move(absPath));
+}
+
 void LocalFSStore::narFromPath(const StorePath & path, Sink & sink)
 {
     if (!isValidPath(path))

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 [storePath, restPath] = store->toStorePath(store->storeDir + path.abs());
-    auto restPath = CanonPath(restPath_);
-
     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,7 +92,7 @@ std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPat
             });
 
             nars.emplace(storePath.hashPart(), narAccessor);
-            return {narAccessor, restPath};
+            return narAccessor;
 
         } catch (SystemError &) {
         }
@@ -98,14 +100,14 @@ std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPat
         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)

src/libstore/remote-store.cc

@@ -794,9 +794,19 @@ void RemoteStore::narFromPath(const StorePath & path, Sink & sink)
     conn->narFromPath(*this, &conn.daemonException, path, [&](Source & source) { copyNAR(conn->from, sink); });
 }
 
+ref<RemoteFSAccessor> RemoteStore::getRemoteFSAccessor(bool requireValidPath)
+{
+    return make_ref<RemoteFSAccessor>(ref<Store>(shared_from_this()), requireValidPath);
+}
+
 ref<SourceAccessor> RemoteStore::getFSAccessor(bool requireValidPath)
 {
-    return make_ref<RemoteFSAccessor>(ref<Store>(shared_from_this()));
+    return getRemoteFSAccessor(requireValidPath);
+}
+
+std::shared_ptr<SourceAccessor> RemoteStore::getFSAccessor(const StorePath & path, bool requireValidPath)
+{
+    return getRemoteFSAccessor(requireValidPath)->accessObject(path);
 }
 
 void RemoteStore::ConnectionHandle::withFramedSink(std::function<void(Sink & sink)> fun)

src/libstore/ssh-store.cc

@@ -151,6 +151,11 @@ struct MountedSSHStore : virtual SSHStore, virtual LocalFSStore
         return LocalFSStore::getFSAccessor(requireValidPath);
     }
 
+    std::shared_ptr<SourceAccessor> getFSAccessor(const StorePath & path, bool requireValidPath) override
+    {
+        return LocalFSStore::getFSAccessor(path, requireValidPath);
+    }
+
     std::optional<std::string> getBuildLogExact(const StorePath & path) override
     {
         return LocalFSStore::getBuildLogExact(path);

src/libstore/store-api.cc

@@ -1120,10 +1120,9 @@ Derivation Store::derivationFromPath(const StorePath & drvPath)
 
 static Derivation readDerivationCommon(Store & store, const StorePath & drvPath, bool requireValidPath)
 {
-    auto accessor = store.getFSAccessor(requireValidPath);
+    auto accessor = store.getFSAccessor(drvPath, requireValidPath);
     try {
-        return parseDerivation(
+        return parseDerivation(store, accessor->readFile(CanonPath::root), Derivation::nameFromPath(drvPath));
-            store, accessor->readFile(CanonPath(drvPath.to_string())), Derivation::nameFromPath(drvPath));
     } catch (FormatError & e) {
         throw Error("error parsing derivation '%s': %s", store.printStorePath(drvPath), e.msg());
     }

src/nix/cat.cc

@@ -41,7 +41,10 @@ struct CmdCatStore : StoreCommand, MixCat
     void run(ref<Store> store) override
     {
         auto [storePath, rest] = store->toStorePath(path);
-        cat(store->getFSAccessor(), CanonPath{storePath.to_string()} / CanonPath{rest});
+        auto accessor = store->getFSAccessor(storePath);
+        if (!accessor)
+            throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath));
+        cat(ref{std::move(accessor)}, CanonPath{rest});
     }
 };
 

src/nix/ls.cc

@@ -115,7 +115,10 @@ struct CmdLsStore : StoreCommand, MixLs
     void run(ref<Store> store) override
     {
         auto [storePath, rest] = store->toStorePath(path);
-        list(store->getFSAccessor(), CanonPath{storePath.to_string()} / CanonPath{rest});
+        auto accessor = store->getFSAccessor(storePath);
+        if (!accessor)
+            throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath));
+        list(ref{std::move(accessor)}, CanonPath{rest});
     }
 };
 

src/nix/nix-store/nix-store.cc

@@ -603,7 +603,7 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise)
 #endif
             if (!hashGiven) {
                 HashResult hash = hashPath(
-                    {store->getFSAccessor(false), CanonPath{info->path.to_string()}},
+                    {ref{store->getFSAccessor(info->path, false)}},
                     FileSerialisationMethod::NixArchive,
                     HashAlgorithm::SHA256);
                 info->narHash = hash.hash;