From 1fbf4961ed4e501a4ac6ee5ea311a0e9368eef69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Tue, 1 Sep 2026 07:41:44 -0500 Subject: [PATCH] remote-fs-accessor: return null for missing store objects --- .../local-binary-cache-store.cc | 10 +++++++ .../include/nix/store/remote-fs-accessor.hh | 4 --- src/libstore/remote-fs-accessor.cc | 26 ++++++++++++------- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/libstore-tests/local-binary-cache-store.cc b/src/libstore-tests/local-binary-cache-store.cc index 9ae1519a6fc1..f964c6260c23 100644 --- a/src/libstore-tests/local-binary-cache-store.cc +++ b/src/libstore-tests/local-binary-cache-store.cc @@ -1,6 +1,7 @@ #include #include "nix/store/local-binary-cache-store.hh" +#include "nix/util/file-system.hh" namespace nix { @@ -22,4 +23,13 @@ TEST(LocalBinaryCacheStore, constructConfig) EXPECT_EQ(config.binaryCacheDir, "/foo/bar/baz"); } +TEST(LocalBinaryCacheStore, fsAccessorsHandleMissingObject) +{ + auto cacheDir = createTempDir(); + AutoDelete delCacheDir{cacheDir}; + auto store = make_ref(cacheDir, LocalBinaryCacheStoreConfig::Params{})->openStore(); + EXPECT_EQ(store->getFSAccessor(StorePath::dummy), nullptr); + EXPECT_EQ(store->getFSAccessor()->maybeLstat(CanonPath(StorePath::dummy.to_string())), std::nullopt); +} + } // namespace nix diff --git a/src/libstore/include/nix/store/remote-fs-accessor.hh b/src/libstore/include/nix/store/remote-fs-accessor.hh index 26fae0d63dd2..3fe40fbe5675 100644 --- a/src/libstore/include/nix/store/remote-fs-accessor.hh +++ b/src/libstore/include/nix/store/remote-fs-accessor.hh @@ -23,8 +23,6 @@ class RemoteFSAccessor : public SourceAccessor NarCache narCache; - bool requireValidPath; - std::pair, CanonPath> fetch(const CanonPath & path); friend struct BinaryCacheStore; @@ -33,8 +31,6 @@ public: /** * @return nullptr if the store does not contain any object at that path. - * - * @todo This actually doesn't return nullptr, but throws on invalid paths. */ std::shared_ptr accessObject(const StorePath & path); diff --git a/src/libstore/remote-fs-accessor.cc b/src/libstore/remote-fs-accessor.cc index acf6a4680de4..f89126a84169 100644 --- a/src/libstore/remote-fs-accessor.cc +++ b/src/libstore/remote-fs-accessor.cc @@ -4,19 +4,19 @@ namespace nix { void RemoteFSAccessor::anchor() {} -RemoteFSAccessor::RemoteFSAccessor(ref store, bool requireValidPath, std::optional cacheDir) +RemoteFSAccessor::RemoteFSAccessor(ref store, bool, std::optional cacheDir) : store(store) , narCache(cacheDir) - , requireValidPath(requireValidPath) { } std::pair, CanonPath> RemoteFSAccessor::fetch(const CanonPath & path) { auto [storePath, restPath] = store->toStorePath(store->storeDir + path.abs()); - if (requireValidPath && !store->isValidPath(storePath)) + auto accessor = accessObject(storePath); + if (!accessor) throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath)); - return {ref{accessObject(storePath)}, restPath}; + return {ref{std::move(accessor)}, restPath}; } std::shared_ptr RemoteFSAccessor::accessObject(const StorePath & storePath) @@ -25,8 +25,12 @@ std::shared_ptr RemoteFSAccessor::accessObject(const StorePath & 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); + std::shared_ptr info; + try { + info = store->queryPathInfo(storePath); + } catch (InvalidPath &) { + return nullptr; + } // Cache the mapping from store path to NAR hash narHashes.emplace(storePath.hashPart(), info->narHash); @@ -39,10 +43,12 @@ std::optional RemoteFSAccessor::maybeLstat(const CanonPath { if (path.isRoot()) return Stat{.type = tDirectory}; - /* FIXME: Correctly handle invalid names (return nullopt) and don't fail on - non-existent paths. */ - auto res = fetch(path); - return res.first->maybeLstat(res.second); + /* FIXME: Correctly handle invalid names (return nullopt). */ + auto [storePath, restPath] = store->toStorePath(store->storeDir + path.abs()); + auto accessor = accessObject(storePath); + if (!accessor) + return std::nullopt; + return accessor->maybeLstat(restPath); } SourceAccessor::DirEntries RemoteFSAccessor::readDirectory(const CanonPath & path)