From c71f80b6ebedd481b4e2d360463e5466d392ba19 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Mon, 22 Sep 2025 01:16:52 +0300 Subject: [PATCH 1/3] libstore: Implement boost::hash for StorePath --- src/libstore/include/nix/store/path.hh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libstore/include/nix/store/path.hh b/src/libstore/include/nix/store/path.hh index 8124cf580..74ee0422b 100644 --- a/src/libstore/include/nix/store/path.hh +++ b/src/libstore/include/nix/store/path.hh @@ -108,4 +108,13 @@ struct hash } // namespace std +namespace nix { + +inline std::size_t hash_value(const StorePath & path) +{ + return std::hash{}(path); +} + +} // namespace nix + JSON_IMPL(nix::StorePath) From c4c92c4c6148199ec07e59b1c90a8584997e3a53 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Mon, 22 Sep 2025 01:16:56 +0300 Subject: [PATCH 2/3] libstore: Make writable dummy store thread-safe Tested by building with b_sanitize=thread and running: nix flake prefetch-inputs --store "dummy://?read-only=false" It might make sense to move this utility class out of dummy-store.cc, but it seems fine for now. --- src/libstore/dummy-store.cc | 165 +++++++++++++++++++++++++++--------- 1 file changed, 125 insertions(+), 40 deletions(-) diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 06b518c15..367cdb5d2 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -4,6 +4,8 @@ #include "nix/util/memory-source-accessor.hh" #include "nix/store/dummy-store.hh" +#include + namespace nix { std::string DummyStoreConfig::doc() @@ -13,6 +15,99 @@ std::string DummyStoreConfig::doc() ; } +namespace { + +class WholeStoreViewAccessor : public SourceAccessor +{ + using BaseName = std::string; + + /** + * Map from store path basenames to corresponding accessors. + */ + boost::concurrent_flat_map> subdirs; + + /** + * Helper accessor for accessing just the CanonPath::root. + */ + MemorySourceAccessor rootPathAccessor; + + /** + * Helper empty accessor. + */ + MemorySourceAccessor emptyAccessor; + + auto + callWithAccessorForPath(CanonPath path, std::invocable auto callback) + { + if (path.isRoot()) + return callback(rootPathAccessor, path); + + BaseName baseName(*path.begin()); + MemorySourceAccessor * res = nullptr; + + subdirs.cvisit(baseName, [&](const auto & kv) { + path = path.removePrefix(CanonPath{baseName}); + res = &*kv.second; + }); + + if (!res) + res = &emptyAccessor; + + return callback(*res, path); + } + +public: + WholeStoreViewAccessor() + { + MemorySink sink{rootPathAccessor}; + sink.createDirectory(CanonPath::root); + } + + void addObject(std::string_view baseName, ref accessor) + { + subdirs.emplace(baseName, std::move(accessor)); + } + + std::string readFile(const CanonPath & path) override + { + return callWithAccessorForPath( + path, [](SourceAccessor & accessor, const CanonPath & path) { return accessor.readFile(path); }); + } + + void readFile(const CanonPath & path, Sink & sink, std::function sizeCallback) override + { + return callWithAccessorForPath(path, [&](SourceAccessor & accessor, const CanonPath & path) { + return accessor.readFile(path, sink, sizeCallback); + }); + } + + bool pathExists(const CanonPath & path) override + { + return callWithAccessorForPath( + path, [](SourceAccessor & accessor, const CanonPath & path) { return accessor.pathExists(path); }); + } + + std::optional maybeLstat(const CanonPath & path) override + { + return callWithAccessorForPath( + path, [](SourceAccessor & accessor, const CanonPath & path) { return accessor.maybeLstat(path); }); + } + + DirEntries readDirectory(const CanonPath & path) override + { + return callWithAccessorForPath( + path, [](SourceAccessor & accessor, const CanonPath & path) { return accessor.readDirectory(path); }); + } + + std::string readLink(const CanonPath & path) override + { + return callWithAccessorForPath( + path, [](SourceAccessor & accessor, const CanonPath & path) { return accessor.readLink(path); }); + } +}; + +} // namespace + struct DummyStore : virtual Store { using Config = DummyStoreConfig; @@ -29,7 +124,7 @@ struct DummyStore : virtual Store * This is map conceptually owns the file system objects for each * store object. */ - std::map contents; + boost::concurrent_flat_map contents; /** * This view conceptually just borrows the file systems objects of @@ -38,23 +133,23 @@ struct DummyStore : virtual Store * * This is needed just in order to implement `Store::getFSAccessor`. */ - ref wholeStoreView = make_ref(); + ref wholeStoreView = make_ref(); DummyStore(ref config) : Store{*config} , config(config) { wholeStoreView->setPathDisplay(config->storeDir); - MemorySink sink{*wholeStoreView}; - sink.createDirectory(CanonPath::root); } void queryPathInfoUncached( const StorePath & path, Callback> callback) noexcept override { - if (auto it = contents.find(path); it != contents.end()) - callback(std::make_shared(StorePath{path}, it->second.info)); - else + bool visited = contents.cvisit(path, [&](const auto & kv) { + callback(std::make_shared(StorePath{kv.first}, kv.second.info)); + }); + + if (!visited) callback(nullptr); } @@ -87,19 +182,14 @@ struct DummyStore : virtual Store parseDump(tempSink, source); auto path = info.path; - auto [it, _] = contents.insert({ - path, - { - std::move(info), - make_ref(std::move(*temp)), - }, - }); - - auto & pathAndContents = it->second; - - bool inserted = wholeStoreView->open(CanonPath(path.to_string()), pathAndContents.contents->root); - if (!inserted) - unreachable(); + auto accessor = make_ref(std::move(*temp)); + contents.insert( + {path, + PathInfoAndContents{ + std::move(info), + accessor, + }}); + wholeStoreView->addObject(path.to_string(), accessor); } StorePath addToStoreFromDump( @@ -156,33 +246,28 @@ struct DummyStore : virtual Store info.narSize = narHash.second.value(); auto path = info.path; - - auto [it, _] = contents.insert({ - path, - { - std::move(info), - make_ref(std::move(*temp)), - }, - }); - - auto & pathAndContents = it->second; - - bool inserted = wholeStoreView->open(CanonPath(path.to_string()), pathAndContents.contents->root); - if (!inserted) - unreachable(); + auto accessor = make_ref(std::move(*temp)); + contents.insert( + {path, + PathInfoAndContents{ + std::move(info), + accessor, + }}); + wholeStoreView->addObject(path.to_string(), accessor); return path; } void narFromPath(const StorePath & path, Sink & sink) override { - auto object = contents.find(path); - if (object == contents.end()) - throw Error("path '%s' is not valid", printStorePath(path)); + bool visited = contents.cvisit(path, [&](const auto & kv) { + const auto & [info, accessor] = kv.second; + SourcePath sourcePath(accessor); + dumpPath(sourcePath, sink, FileSerialisationMethod::NixArchive); + }); - const auto & [info, accessor] = object->second; - SourcePath sourcePath(accessor); - dumpPath(sourcePath, sink, FileSerialisationMethod::NixArchive); + if (!visited) + throw Error("path '%s' is not valid", printStorePath(path)); } void From 5915fe319011b4be8accb2e3e4a21e9e2000d7db Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Mon, 22 Sep 2025 01:17:00 +0300 Subject: [PATCH 3/3] Revert "Use shared pointers in the memory source accessor" This is no longer necessary. This reverts commit 4df60e639b7e492ac5f651f2b3aa02055de5549a. --- src/libutil-tests/git.cc | 16 ++++++++-------- .../include/nix/util/memory-source-accessor.hh | 16 ++++------------ src/libutil/memory-source-accessor.cc | 6 +++--- 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/src/libutil-tests/git.cc b/src/libutil-tests/git.cc index a06c5896d..6180a4cfc 100644 --- a/src/libutil-tests/git.cc +++ b/src/libutil-tests/git.cc @@ -233,30 +233,30 @@ TEST_F(GitTest, both_roundrip) .contents{ { "foo", - make_ref(File::Regular{ + File::Regular{ .contents = "hello\n\0\n\tworld!", - }), + }, }, { "bar", - make_ref(File::Directory{ + File::Directory{ .contents = { { "baz", - make_ref(File::Regular{ + File::Regular{ .executable = true, .contents = "good day,\n\0\n\tworld!", - }), + }, }, { "quux", - make_ref(File::Symlink{ + File::Symlink{ .target = "/over/there", - }), + }, }, }, - }), + }, }, }, }; diff --git a/src/libutil/include/nix/util/memory-source-accessor.hh b/src/libutil/include/nix/util/memory-source-accessor.hh index 53f1b0241..eba282fe1 100644 --- a/src/libutil/include/nix/util/memory-source-accessor.hh +++ b/src/libutil/include/nix/util/memory-source-accessor.hh @@ -35,7 +35,7 @@ struct MemorySourceAccessor : virtual SourceAccessor { using Name = std::string; - std::map, std::less<>> contents; + std::map> contents; bool operator==(const Directory &) const noexcept; // TODO libc++ 16 (used by darwin) missing `std::map::operator <=>`, can't do yet. @@ -89,21 +89,13 @@ struct MemorySourceAccessor : virtual SourceAccessor SourcePath addFile(CanonPath path, std::string && contents); }; -inline bool -MemorySourceAccessor::File::Directory::operator==(const MemorySourceAccessor::File::Directory & other) const noexcept -{ - return std::ranges::equal(contents, other.contents, [](const auto & lhs, const auto & rhs) -> bool { - return lhs.first == rhs.first && *lhs.second == *rhs.second; - }); -}; +inline bool MemorySourceAccessor::File::Directory::operator==( + const MemorySourceAccessor::File::Directory &) const noexcept = default; inline bool MemorySourceAccessor::File::Directory::operator<(const MemorySourceAccessor::File::Directory & other) const noexcept { - return std::ranges::lexicographical_compare( - contents, other.contents, [](const auto & lhs, const auto & rhs) -> bool { - return lhs.first < rhs.first && *lhs.second < *rhs.second; - }); + return contents < other.contents; } inline bool MemorySourceAccessor::File::operator==(const MemorySourceAccessor::File &) const noexcept = default; diff --git a/src/libutil/memory-source-accessor.cc b/src/libutil/memory-source-accessor.cc index 7d53d6785..caff5b56a 100644 --- a/src/libutil/memory-source-accessor.cc +++ b/src/libutil/memory-source-accessor.cc @@ -39,11 +39,11 @@ MemorySourceAccessor::File * MemorySourceAccessor::open(const CanonPath & path, i, { std::string{name}, - make_ref(File::Directory{}), + File::Directory{}, }); } } - cur = &*i->second; + cur = &i->second; } if (newF && create) @@ -107,7 +107,7 @@ MemorySourceAccessor::DirEntries MemorySourceAccessor::readDirectory(const Canon if (auto * d = std::get_if(&f->raw)) { DirEntries res; for (auto & [name, file] : d->contents) - res.insert_or_assign(name, file->lstat().type); + res.insert_or_assign(name, file.lstat().type); return res; } else throw Error("file '%s' is not a directory", path);