From eab2919119b8c41d70f62411d7e7dd972d6820da Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 6 May 2024 19:05:42 +0200 Subject: [PATCH 1/3] Use SourcePath in more places Now that SourcePath uses a SourceAccessor instead of an InputAccessor, we can use it in function signatures instead of passing a SourceAccessor and CanonPath separately. --- src/libfetchers/fetch-to-store.cc | 4 ++-- src/libfetchers/unix/mercurial.cc | 4 ++-- src/libstore/binary-cache-store.cc | 7 +++--- src/libstore/binary-cache-store.hh | 3 +-- src/libstore/legacy-ssh-store.hh | 3 +-- src/libstore/store-api.cc | 20 +++++++---------- src/libstore/store-api.hh | 6 ++--- src/libstore/store-dir-config.hh | 5 +++-- .../unix/build/local-derivation-goal.cc | 16 +++++--------- src/libstore/unix/build/worker.cc | 2 +- src/libstore/unix/local-store.cc | 10 ++++----- src/libstore/unix/optimise-store.cc | 6 ++--- src/libutil/archive.cc | 7 +++--- src/libutil/file-content-address.cc | 17 +++++++------- src/libutil/file-content-address.hh | 8 ++++--- src/libutil/git.cc | 22 +++++++++---------- src/libutil/git.hh | 14 ++++++------ src/libutil/posix-source-accessor.cc | 5 +++-- src/libutil/posix-source-accessor.hh | 4 +++- src/libutil/source-path.hh | 5 +++++ src/nix-store/nix-store.cc | 8 +++---- src/nix/add-to-store.cc | 4 ++-- src/nix/hash.cc | 11 +++++----- src/nix/prefetch.cc | 3 +-- tests/unit/libutil/git.cc | 16 +++++++------- 25 files changed, 101 insertions(+), 109 deletions(-) diff --git a/src/libfetchers/fetch-to-store.cc b/src/libfetchers/fetch-to-store.cc index c105fe1fc..4fce6180d 100644 --- a/src/libfetchers/fetch-to-store.cc +++ b/src/libfetchers/fetch-to-store.cc @@ -42,9 +42,9 @@ StorePath fetchToStore( auto storePath = mode == FetchMode::DryRun ? store.computeStorePath( - name, *path.accessor, path.path, method, HashAlgorithm::SHA256, {}, filter2).first + name, path, method, HashAlgorithm::SHA256, {}, filter2).first : store.addToStore( - name, *path.accessor, path.path, method, HashAlgorithm::SHA256, {}, filter2, repair); + name, path, method, HashAlgorithm::SHA256, {}, filter2, repair); if (cacheKey && mode == FetchMode::Copy) fetchers::getCache()->add(store, *cacheKey, {}, storePath, true); diff --git a/src/libfetchers/unix/mercurial.cc b/src/libfetchers/unix/mercurial.cc index 4d95f54f0..838cb41bf 100644 --- a/src/libfetchers/unix/mercurial.cc +++ b/src/libfetchers/unix/mercurial.cc @@ -212,7 +212,7 @@ struct MercurialInputScheme : InputScheme auto storePath = store->addToStore( input.getName(), - *makeFSSourceAccessor(), CanonPath { actualPath }, + {makeFSSourceAccessor(), CanonPath(actualPath)}, FileIngestionMethod::Recursive, HashAlgorithm::SHA256, {}, filter); @@ -318,7 +318,7 @@ struct MercurialInputScheme : InputScheme deletePath(tmpDir + "/.hg_archival.txt"); - auto storePath = store->addToStore(name, *makeFSSourceAccessor(), CanonPath { tmpDir }); + auto storePath = store->addToStore(name, {makeFSSourceAccessor(), CanonPath(tmpDir)}); Attrs infoAttrs({ {"rev", input.getRev()->gitRev()}, diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 97b6ec052..5153ca64f 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -442,8 +442,7 @@ void BinaryCacheStore::queryPathInfoUncached(const StorePath & storePath, StorePath BinaryCacheStore::addToStore( std::string_view name, - SourceAccessor & accessor, - const CanonPath & path, + const SourcePath & path, ContentAddressMethod method, HashAlgorithm hashAlgo, const StorePathSet & references, @@ -454,10 +453,10 @@ StorePath BinaryCacheStore::addToStore( non-recursive+sha256 so we can just use the default implementation of this method in terms of addToStoreFromDump. */ - auto h = hashPath(accessor, path, method.getFileIngestionMethod(), hashAlgo, filter); + auto h = hashPath(path, method.getFileIngestionMethod(), hashAlgo, filter); auto source = sinkToSource([&](Sink & sink) { - accessor.dumpPath(path, sink, filter); + path.dumpPath(sink, filter); }); return addToStoreCommon(*source, repair, CheckSigs, [&](HashResult nar) { ValidPathInfo info { diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 7c2828309..695bc9252 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -133,8 +133,7 @@ public: StorePath addToStore( std::string_view name, - SourceAccessor & accessor, - const CanonPath & srcPath, + const SourcePath & path, ContentAddressMethod method, HashAlgorithm hashAlgo, const StorePathSet & references, diff --git a/src/libstore/legacy-ssh-store.hh b/src/libstore/legacy-ssh-store.hh index ca2f115d2..343823693 100644 --- a/src/libstore/legacy-ssh-store.hh +++ b/src/libstore/legacy-ssh-store.hh @@ -60,8 +60,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor StorePath addToStore( std::string_view name, - SourceAccessor & accessor, - const CanonPath & srcPath, + const SourcePath & path, ContentAddressMethod method, HashAlgorithm hashAlgo, const StorePathSet & references, diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 118e5de9f..008cc11e7 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -167,14 +167,13 @@ StorePath StoreDirConfig::makeFixedOutputPathFromCA(std::string_view name, const std::pair StoreDirConfig::computeStorePath( std::string_view name, - SourceAccessor & accessor, - const CanonPath & path, + const SourcePath & path, ContentAddressMethod method, HashAlgorithm hashAlgo, const StorePathSet & references, PathFilter & filter) const { - auto h = hashPath(accessor, path, method.getFileIngestionMethod(), hashAlgo, filter); + auto h = hashPath(path, method.getFileIngestionMethod(), hashAlgo, filter); return { makeFixedOutputPathFromCA( name, @@ -192,8 +191,7 @@ std::pair StoreDirConfig::computeStorePath( StorePath Store::addToStore( std::string_view name, - SourceAccessor & accessor, - const CanonPath & path, + const SourcePath & path, ContentAddressMethod method, HashAlgorithm hashAlgo, const StorePathSet & references, @@ -214,7 +212,7 @@ StorePath Store::addToStore( break; } auto source = sinkToSource([&](Sink & sink) { - dumpPath(accessor, path, sink, fsm, filter); + dumpPath(path, sink, fsm, filter); }); return addToStoreFromDump(*source, name, fsm, method, hashAlgo, references, repair); } @@ -343,8 +341,7 @@ digraph graphname { */ ValidPathInfo Store::addToStoreSlow( std::string_view name, - SourceAccessor & accessor, - const CanonPath & srcPath, + const SourcePath & srcPath, ContentAddressMethod method, HashAlgorithm hashAlgo, const StorePathSet & references, std::optional expectedCAHash) @@ -366,7 +363,7 @@ ValidPathInfo Store::addToStoreSlow( srcPath. The fact that we use scratchpadSink as a temporary buffer here is an implementation detail. */ auto fileSource = sinkToSource([&](Sink & scratchpadSink) { - accessor.dumpPath(srcPath, scratchpadSink); + srcPath.dumpPath(scratchpadSink); }); /* tapped provides the same data as fileSource, but we also write all the @@ -389,13 +386,12 @@ ValidPathInfo Store::addToStoreSlow( auto hash = method == FileIngestionMethod::Recursive && hashAlgo == HashAlgorithm::SHA256 ? narHash : method == FileIngestionMethod::Git - ? git::dumpHash(hashAlgo, accessor, srcPath).hash + ? git::dumpHash(hashAlgo, srcPath).hash : caHashSink.finish().first; if (expectedCAHash && expectedCAHash != hash) throw Error("hash mismatch for '%s'", srcPath); - ValidPathInfo info { *this, name, @@ -412,7 +408,7 @@ ValidPathInfo Store::addToStoreSlow( if (!isValidPath(info.path)) { auto source = sinkToSource([&](Sink & scratchpadSink) { - accessor.dumpPath(srcPath, scratchpadSink); + srcPath.dumpPath(scratchpadSink); }); addToStore(info, *source); } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 5f683a211..ae8c22437 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -439,8 +439,7 @@ public: */ virtual StorePath addToStore( std::string_view name, - SourceAccessor & accessor, - const CanonPath & path, + const SourcePath & path, ContentAddressMethod method = FileIngestionMethod::Recursive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), @@ -454,8 +453,7 @@ public: */ ValidPathInfo addToStoreSlow( std::string_view name, - SourceAccessor & accessor, - const CanonPath & path, + const SourcePath & path, ContentAddressMethod method = FileIngestionMethod::Recursive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = StorePathSet(), diff --git a/src/libstore/store-dir-config.hh b/src/libstore/store-dir-config.hh index 7ca8c2665..643f8854d 100644 --- a/src/libstore/store-dir-config.hh +++ b/src/libstore/store-dir-config.hh @@ -13,6 +13,8 @@ namespace nix { +struct SourcePath; + MakeError(BadStorePath, Error); struct StoreDirConfig : public Config @@ -94,8 +96,7 @@ struct StoreDirConfig : public Config */ std::pair computeStorePath( std::string_view name, - SourceAccessor & accessor, - const CanonPath & path, + const SourcePath & path, ContentAddressMethod method = FileIngestionMethod::Recursive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, const StorePathSet & references = {}, diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 0ebd22c75..24f6897c6 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -1306,8 +1306,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual In StorePath addToStore( std::string_view name, - SourceAccessor & accessor, - const CanonPath & srcPath, + const SourcePath & srcPath, ContentAddressMethod method, HashAlgorithm hashAlgo, const StorePathSet & references, @@ -2485,7 +2484,6 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() /* FIXME optimize and deduplicate with addToStore */ std::string oldHashPart { scratchPath->hashPart() }; auto got = [&]{ - PosixSourceAccessor accessor; auto fim = outputHash.method.getFileIngestionMethod(); switch (fim) { case FileIngestionMethod::Flat: @@ -2494,15 +2492,15 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() HashModuloSink caSink { outputHash.hashAlgo, oldHashPart }; auto fim = outputHash.method.getFileIngestionMethod(); dumpPath( - accessor, CanonPath { actualPath }, + {makeFSSourceAccessor(), CanonPath(actualPath)}, caSink, (FileSerialisationMethod) fim); return caSink.finish().first; } case FileIngestionMethod::Git: { return git::dumpHash( - outputHash.hashAlgo, accessor, - CanonPath { tmpDir + "/tmp" }).hash; + outputHash.hashAlgo, + {makeFSSourceAccessor(), CanonPath(tmpDir + "/tmp")}).hash; } } assert(false); @@ -2529,9 +2527,8 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() } { - PosixSourceAccessor accessor; HashResult narHashAndSize = hashPath( - accessor, CanonPath { actualPath }, + {makeFSSourceAccessor(), CanonPath(actualPath)}, FileSerialisationMethod::Recursive, HashAlgorithm::SHA256); newInfo0.narHash = narHashAndSize.first; newInfo0.narSize = narHashAndSize.second; @@ -2553,9 +2550,8 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() std::string { scratchPath->hashPart() }, std::string { requiredFinalPath.hashPart() }); rewriteOutput(outputRewrites); - PosixSourceAccessor accessor; HashResult narHashAndSize = hashPath( - accessor, CanonPath { actualPath }, + {makeFSSourceAccessor(), CanonPath(actualPath)}, FileSerialisationMethod::Recursive, HashAlgorithm::SHA256); ValidPathInfo newInfo0 { requiredFinalPath, narHashAndSize.first }; newInfo0.narSize = narHashAndSize.second; diff --git a/src/libstore/unix/build/worker.cc b/src/libstore/unix/build/worker.cc index 815ded3d5..03fc280a4 100644 --- a/src/libstore/unix/build/worker.cc +++ b/src/libstore/unix/build/worker.cc @@ -530,7 +530,7 @@ bool Worker::pathContentsGood(const StorePath & path) res = false; else { Hash current = hashPath( - *store.getFSAccessor(), CanonPath { store.printStorePath(path) }, + {store.getFSAccessor(), CanonPath(store.printStorePath(path))}, FileIngestionMethod::Recursive, info->narHash.algo); Hash nullHash(HashAlgorithm::SHA256); res = info->narHash == nullHash || info->narHash == current; diff --git a/src/libstore/unix/local-store.cc b/src/libstore/unix/local-store.cc index 1593affd6..7b3ba1347 100644 --- a/src/libstore/unix/local-store.cc +++ b/src/libstore/unix/local-store.cc @@ -1132,12 +1132,12 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, specified.hash.algo, std::string { info.path.hashPart() }, }; - dumpPath(*accessor, path, caSink, (FileSerialisationMethod) fim); + dumpPath({accessor, path}, caSink, (FileSerialisationMethod) fim); h = caSink.finish().first; break; } case FileIngestionMethod::Git: - h = git::dumpHash(specified.hash.algo, *accessor, path).hash; + h = git::dumpHash(specified.hash.algo, {accessor, path}).hash; break; } ContentAddress { @@ -1247,14 +1247,12 @@ StorePath LocalStore::addToStoreFromDump( auto [dumpHash, size] = hashSink->finish(); - PosixSourceAccessor accessor; - auto desc = ContentAddressWithReferences::fromParts( hashMethod, methodsMatch ? dumpHash : hashPath( - accessor, CanonPath { tempPath }, + {makeFSSourceAccessor(), CanonPath(tempPath)}, hashMethod.getFileIngestionMethod(), hashAlgo), { .others = references, @@ -1394,7 +1392,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) Path linkPath = linksDir + "/" + link.name; PosixSourceAccessor accessor; std::string hash = hashPath( - accessor, CanonPath { linkPath }, + {makeFSSourceAccessor(), CanonPath(linkPath)}, FileIngestionMethod::Recursive, HashAlgorithm::SHA256).to_string(HashFormat::Nix32, false); if (hash != link.name) { printError("link '%s' was modified! expected hash '%s', got '%s'", diff --git a/src/libstore/unix/optimise-store.cc b/src/libstore/unix/optimise-store.cc index daaaaf073..bedb77849 100644 --- a/src/libstore/unix/optimise-store.cc +++ b/src/libstore/unix/optimise-store.cc @@ -148,9 +148,8 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, contents of the symlink (i.e. the result of readlink()), not the contents of the target (which may not even exist). */ Hash hash = ({ - PosixSourceAccessor accessor; hashPath( - accessor, CanonPath { path }, + {make_ref(), CanonPath(path)}, FileSerialisationMethod::Recursive, HashAlgorithm::SHA256).first; }); debug("'%1%' has hash '%2%'", path, hash.to_string(HashFormat::Nix32, true)); @@ -163,9 +162,8 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, auto stLink = lstat(linkPath); if (st.st_size != stLink.st_size || (repair && hash != ({ - PosixSourceAccessor accessor; hashPath( - accessor, CanonPath { linkPath }, + {make_ref(), CanonPath(linkPath)}, FileSerialisationMethod::Recursive, HashAlgorithm::SHA256).first; }))) { diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 351ee094b..04f777d00 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -8,6 +8,7 @@ #include "archive.hh" #include "config.hh" #include "posix-source-accessor.hh" +#include "source-path.hh" #include "file-system.hh" #include "signals.hh" @@ -110,9 +111,9 @@ void SourceAccessor::dumpPath( time_t dumpPathAndGetMtime(const Path & path, Sink & sink, PathFilter & filter) { - auto [accessor, canonPath] = PosixSourceAccessor::createAtRoot(path); - accessor.dumpPath(canonPath, sink, filter); - return accessor.mtime; + auto path2 = PosixSourceAccessor::createAtRoot(path); + path2.dumpPath(sink, filter); + return path2.accessor.dynamic_pointer_cast()->mtime; } void dumpPath(const Path & path, Sink & sink, PathFilter & filter) diff --git a/src/libutil/file-content-address.cc b/src/libutil/file-content-address.cc index 570247b9e..769042d00 100644 --- a/src/libutil/file-content-address.cc +++ b/src/libutil/file-content-address.cc @@ -1,6 +1,7 @@ #include "file-content-address.hh" #include "archive.hh" #include "git.hh" +#include "source-path.hh" namespace nix { @@ -68,17 +69,17 @@ std::string_view renderFileIngestionMethod(FileIngestionMethod method) void dumpPath( - SourceAccessor & accessor, const CanonPath & path, + const SourcePath & path, Sink & sink, FileSerialisationMethod method, PathFilter & filter) { switch (method) { case FileSerialisationMethod::Flat: - accessor.readFile(path, sink); + path.readFile(sink); break; case FileSerialisationMethod::Recursive: - accessor.dumpPath(path, sink, filter); + path.dumpPath(sink, filter); break; } } @@ -101,27 +102,27 @@ void restorePath( HashResult hashPath( - SourceAccessor & accessor, const CanonPath & path, + const SourcePath & path, FileSerialisationMethod method, HashAlgorithm ha, PathFilter & filter) { HashSink sink { ha }; - dumpPath(accessor, path, sink, method, filter); + dumpPath(path, sink, method, filter); return sink.finish(); } Hash hashPath( - SourceAccessor & accessor, const CanonPath & path, + const SourcePath & path, FileIngestionMethod method, HashAlgorithm ht, PathFilter & filter) { switch (method) { case FileIngestionMethod::Flat: case FileIngestionMethod::Recursive: - return hashPath(accessor, path, (FileSerialisationMethod) method, ht, filter).first; + return hashPath(path, (FileSerialisationMethod) method, ht, filter).first; case FileIngestionMethod::Git: - return git::dumpHash(ht, accessor, path, filter).hash; + return git::dumpHash(ht, path, filter).hash; } assert(false); } diff --git a/src/libutil/file-content-address.hh b/src/libutil/file-content-address.hh index b361ab243..145a8fb1f 100644 --- a/src/libutil/file-content-address.hh +++ b/src/libutil/file-content-address.hh @@ -7,6 +7,8 @@ namespace nix { +struct SourcePath; + /** * An enumeration of the ways we can serialize file system * objects. @@ -45,7 +47,7 @@ std::string_view renderFileSerialisationMethod(FileSerialisationMethod method); * Dump a serialization of the given file system object. */ void dumpPath( - SourceAccessor & accessor, const CanonPath & path, + const SourcePath & path, Sink & sink, FileSerialisationMethod method, PathFilter & filter = defaultPathFilter); @@ -72,7 +74,7 @@ void restorePath( * ``` */ HashResult hashPath( - SourceAccessor & accessor, const CanonPath & path, + const SourcePath & path, FileSerialisationMethod method, HashAlgorithm ha, PathFilter & filter = defaultPathFilter); @@ -138,7 +140,7 @@ std::string_view renderFileIngestionMethod(FileIngestionMethod method); * useful defined for a merkle format. */ Hash hashPath( - SourceAccessor & accessor, const CanonPath & path, + const SourcePath & path, FileIngestionMethod method, HashAlgorithm ha, PathFilter & filter = defaultPathFilter); diff --git a/src/libutil/git.cc b/src/libutil/git.cc index a60589baa..8c538c988 100644 --- a/src/libutil/git.cc +++ b/src/libutil/git.cc @@ -8,7 +8,6 @@ #include "signals.hh" #include "config.hh" #include "hash.hh" -#include "posix-source-accessor.hh" #include "git.hh" #include "serialise.hh" @@ -269,18 +268,18 @@ void dumpTree(const Tree & entries, Sink & sink, Mode dump( - SourceAccessor & accessor, const CanonPath & path, + const SourcePath & path, Sink & sink, std::function hook, PathFilter & filter, const ExperimentalFeatureSettings & xpSettings) { - auto st = accessor.lstat(path); + auto st = path.lstat(); switch (st.type) { case SourceAccessor::tRegular: { - accessor.readFile(path, sink, [&](uint64_t size) { + path.readFile(sink, [&](uint64_t size) { dumpBlobPrefix(size, sink, xpSettings); }); return st.isExecutable @@ -291,9 +290,9 @@ Mode dump( case SourceAccessor::tDirectory: { Tree entries; - for (auto & [name, _] : accessor.readDirectory(path)) { + for (auto & [name, _] : path.readDirectory()) { auto child = path / name; - if (!filter(child.abs())) continue; + if (!filter(child.path.abs())) continue; auto entry = hook(child); @@ -309,7 +308,7 @@ Mode dump( case SourceAccessor::tSymlink: { - auto target = accessor.readLink(path); + auto target = path.readLink(); dumpBlobPrefix(target.size(), sink, xpSettings); sink(target); return Mode::Symlink; @@ -323,13 +322,14 @@ Mode dump( TreeEntry dumpHash( - HashAlgorithm ha, - SourceAccessor & accessor, const CanonPath & path, PathFilter & filter) + HashAlgorithm ha, + const SourcePath & path, + PathFilter & filter) { std::function hook; - hook = [&](const CanonPath & path) -> TreeEntry { + hook = [&](const SourcePath & path) -> TreeEntry { auto hashSink = HashSink(ha); - auto mode = dump(accessor, path, hashSink, hook, filter); + auto mode = dump(path, hashSink, hook, filter); auto hash = hashSink.finish().first; return { .mode = mode, diff --git a/src/libutil/git.hh b/src/libutil/git.hh index cfea48fbe..a65edb964 100644 --- a/src/libutil/git.hh +++ b/src/libutil/git.hh @@ -8,7 +8,7 @@ #include "types.hh" #include "serialise.hh" #include "hash.hh" -#include "source-accessor.hh" +#include "source-path.hh" #include "fs-sink.hh" namespace nix::git { @@ -125,7 +125,7 @@ std::optional convertMode(SourceAccessor::Type type); * Given a `Hash`, return a `SourceAccessor` and `CanonPath` pointing to * the file system object with that path. */ -using RestoreHook = std::pair(Hash); +using RestoreHook = SourcePath(Hash); /** * Wrapper around `parse` and `RestoreSink` @@ -157,10 +157,10 @@ void dumpTree( * Note that if the child is a directory, its child in must also be so * processed in order to compute this information. */ -using DumpHook = TreeEntry(const CanonPath & path); +using DumpHook = TreeEntry(const SourcePath & path); Mode dump( - SourceAccessor & accessor, const CanonPath & path, + const SourcePath & path, Sink & sink, std::function hook, PathFilter & filter = defaultPathFilter, @@ -172,9 +172,9 @@ Mode dump( * A smaller wrapper around `dump`. */ TreeEntry dumpHash( - HashAlgorithm ha, - SourceAccessor & accessor, const CanonPath & path, - PathFilter & filter = defaultPathFilter); + HashAlgorithm ha, + const SourcePath & path, + PathFilter & filter = defaultPathFilter); /** * A line from the output of `git ls-remote --symref`. diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc index f7dffb871..f3551db42 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -1,4 +1,5 @@ #include "posix-source-accessor.hh" +#include "source-path.hh" #include "signals.hh" #include "sync.hh" @@ -17,11 +18,11 @@ PosixSourceAccessor::PosixSourceAccessor() : PosixSourceAccessor(std::filesystem::path {}) { } -std::pair PosixSourceAccessor::createAtRoot(const std::filesystem::path & path) +SourcePath PosixSourceAccessor::createAtRoot(const std::filesystem::path & path) { std::filesystem::path path2 = absPath(path.string()); return { - PosixSourceAccessor { path2.root_path() }, + make_ref(path2.root_path()), CanonPath { path2.relative_path().string() }, }; } diff --git a/src/libutil/posix-source-accessor.hh b/src/libutil/posix-source-accessor.hh index 717c8f017..40f60bb54 100644 --- a/src/libutil/posix-source-accessor.hh +++ b/src/libutil/posix-source-accessor.hh @@ -4,6 +4,8 @@ namespace nix { +struct SourcePath; + /** * A source accessor that uses the Unix filesystem. */ @@ -53,7 +55,7 @@ struct PosixSourceAccessor : virtual SourceAccessor * and * [`std::filesystem::path::relative_path`](https://en.cppreference.com/w/cpp/filesystem/path/relative_path). */ - static std::pair createAtRoot(const std::filesystem::path & path); + static SourcePath createAtRoot(const std::filesystem::path & path); private: diff --git a/src/libutil/source-path.hh b/src/libutil/source-path.hh index 7e4c3c65d..83ec6295d 100644 --- a/src/libutil/source-path.hh +++ b/src/libutil/source-path.hh @@ -41,6 +41,11 @@ struct SourcePath */ std::string readFile() const; + void readFile( + Sink & sink, + std::function sizeCallback = [](uint64_t size){}) const + { return accessor->readFile(path, sink, sizeCallback); } + /** * Return whether this `SourcePath` denotes a file (of any type) * that exists diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 719675cba..b23d99ad6 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -184,7 +184,7 @@ static void opAdd(Strings opFlags, Strings opArgs) for (auto & i : opArgs) { auto [accessor, canonPath] = PosixSourceAccessor::createAtRoot(i); cout << fmt("%s\n", store->printStorePath(store->addToStore( - std::string(baseNameOf(i)), accessor, canonPath))); + std::string(baseNameOf(i)), {accessor, canonPath}))); } } @@ -209,8 +209,7 @@ static void opAddFixed(Strings opFlags, Strings opArgs) auto [accessor, canonPath] = PosixSourceAccessor::createAtRoot(i); std::cout << fmt("%s\n", store->printStorePath(store->addToStoreSlow( baseNameOf(i), - accessor, - canonPath, + {accessor, canonPath}, method, hashAlgo).path)); } @@ -562,8 +561,7 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise) #endif if (!hashGiven) { HashResult hash = hashPath( - *store->getFSAccessor(false), CanonPath { store->printStorePath(info->path) }, - + {store->getFSAccessor(false), CanonPath { store->printStorePath(info->path) }}, FileSerialisationMethod::Recursive, HashAlgorithm::SHA256); info->narHash = hash.first; info->narSize = hash.second; diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index 02154715f..af6743375 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -41,9 +41,9 @@ struct CmdAddToStore : MixDryRun, StoreCommand auto storePath = dryRun ? store->computeStorePath( - *namePart, accessor, path2, caMethod, hashAlgo, {}).first + *namePart, {accessor, path2}, caMethod, hashAlgo, {}).first : store->addToStoreSlow( - *namePart, accessor, path2, caMethod, hashAlgo, {}).path; + *namePart, {accessor, path2}, caMethod, hashAlgo, {}).path; logger->cout("%s", store->printStorePath(storePath)); } diff --git a/src/nix/hash.cc b/src/nix/hash.cc index f849bf0cf..f969886ea 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -87,30 +87,29 @@ struct CmdHashBase : Command return std::make_unique(hashAlgo); }; - auto [accessor_, canonPath] = PosixSourceAccessor::createAtRoot(path); - auto & accessor = accessor_; + auto path2 = PosixSourceAccessor::createAtRoot(path); Hash h { HashAlgorithm::SHA256 }; // throwaway def to appease C++ switch (mode) { case FileIngestionMethod::Flat: case FileIngestionMethod::Recursive: { auto hashSink = makeSink(); - dumpPath(accessor, canonPath, *hashSink, (FileSerialisationMethod) mode); + dumpPath(path2, *hashSink, (FileSerialisationMethod) mode); h = hashSink->finish().first; break; } case FileIngestionMethod::Git: { std::function hook; - hook = [&](const CanonPath & path) -> git::TreeEntry { + hook = [&](const SourcePath & path) -> git::TreeEntry { auto hashSink = makeSink(); - auto mode = dump(accessor, path, *hashSink, hook); + auto mode = dump(path, *hashSink, hook); auto hash = hashSink->finish().first; return { .mode = mode, .hash = hash, }; }; - h = hook(canonPath).hash; + h = hook(path2).hash; break; } } diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc index f905cabef..6c2eb5aaf 100644 --- a/src/nix/prefetch.cc +++ b/src/nix/prefetch.cc @@ -125,9 +125,8 @@ std::tuple prefetchFile( Activity act(*logger, lvlChatty, actUnknown, fmt("adding '%s' to the store", url)); - auto [accessor, canonPath] = PosixSourceAccessor::createAtRoot(tmpFile); auto info = store->addToStoreSlow( - *name, accessor, canonPath, + *name, PosixSourceAccessor::createAtRoot(tmpFile), ingestionMethod, hashAlgo, {}, expectedHash); storePath = info.path; assert(info.ca); diff --git a/tests/unit/libutil/git.cc b/tests/unit/libutil/git.cc index 4f92488d6..ff934c117 100644 --- a/tests/unit/libutil/git.cc +++ b/tests/unit/libutil/git.cc @@ -154,8 +154,8 @@ TEST_F(GitTest, tree_write) { TEST_F(GitTest, both_roundrip) { using File = MemorySourceAccessor::File; - MemorySourceAccessor files; - files.root = File::Directory { + auto files = make_ref(); + files->root = File::Directory { .contents { { "foo", @@ -189,12 +189,12 @@ TEST_F(GitTest, both_roundrip) { std::map cas; std::function dumpHook; - dumpHook = [&](const CanonPath & path) { + dumpHook = [&](const SourcePath & path) { StringSink s; HashSink hashSink { HashAlgorithm::SHA1 }; TeeSink s2 { s, hashSink }; auto mode = dump( - files, path, s2, dumpHook, + path, s2, dumpHook, defaultPathFilter, mockXpSettings); auto hash = hashSink.finish().first; cas.insert_or_assign(hash, std::move(s.s)); @@ -204,11 +204,11 @@ TEST_F(GitTest, both_roundrip) { }; }; - auto root = dumpHook(CanonPath::root); + auto root = dumpHook({files}); - MemorySourceAccessor files2; + auto files2 = make_ref(); - MemorySink sinkFiles2 { files2 }; + MemorySink sinkFiles2 { *files2 }; std::function mkSinkHook; mkSinkHook = [&](auto prefix, auto & hash, auto blobMode) { @@ -229,7 +229,7 @@ TEST_F(GitTest, both_roundrip) { mkSinkHook("", root.hash, BlobMode::Regular); - ASSERT_EQ(files, files2); + ASSERT_EQ(*files, *files2); } TEST(GitLsRemote, parseSymrefLineWithReference) { From ef28c7329c68c8e792400c9e9dec65bd4c20584d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 6 May 2024 19:16:52 +0200 Subject: [PATCH 2/3] Rename makeFSSourceAccessor -> getFSSourceAccessor() This makes it clearer that it returns a shared accessor object. --- src/libcmd/installables.cc | 2 +- src/libexpr/eval.cc | 4 ++-- src/libfetchers/unix/mercurial.cc | 4 ++-- src/libstore/unix/build/local-derivation-goal.cc | 8 ++++---- src/libstore/unix/local-store.cc | 4 ++-- src/libutil/posix-source-accessor.cc | 2 +- src/libutil/source-accessor.hh | 7 +++++-- 7 files changed, 17 insertions(+), 14 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 0d42a62cc..43e312540 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -146,7 +146,7 @@ MixFlakeOptions::MixFlakeOptions() .category = category, .labels = {"flake-lock-path"}, .handler = {[&](std::string lockFilePath) { - lockFlags.referenceLockFilePath = {makeFSSourceAccessor(), CanonPath(absPath(lockFilePath))}; + lockFlags.referenceLockFilePath = {getFSSourceAccessor(), CanonPath(absPath(lockFilePath))}; }}, .completer = completePath }); diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index ad6cdc6d2..d7e3a2cdb 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -399,14 +399,14 @@ EvalState::EvalState( , emptyBindings(0) , rootFS( evalSettings.restrictEval || evalSettings.pureEval - ? ref(AllowListSourceAccessor::create(makeFSSourceAccessor(), {}, + ? ref(AllowListSourceAccessor::create(getFSSourceAccessor(), {}, [](const CanonPath & path) -> RestrictedPathError { auto modeInformation = evalSettings.pureEval ? "in pure evaluation mode (use '--impure' to override)" : "in restricted mode"; throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", path, modeInformation); })) - : makeFSSourceAccessor()) + : getFSSourceAccessor()) , corepkgsFS(make_ref()) , internalFS(make_ref()) , derivationInternal{corepkgsFS->addFile( diff --git a/src/libfetchers/unix/mercurial.cc b/src/libfetchers/unix/mercurial.cc index 838cb41bf..049faffa8 100644 --- a/src/libfetchers/unix/mercurial.cc +++ b/src/libfetchers/unix/mercurial.cc @@ -212,7 +212,7 @@ struct MercurialInputScheme : InputScheme auto storePath = store->addToStore( input.getName(), - {makeFSSourceAccessor(), CanonPath(actualPath)}, + {getFSSourceAccessor(), CanonPath(actualPath)}, FileIngestionMethod::Recursive, HashAlgorithm::SHA256, {}, filter); @@ -318,7 +318,7 @@ struct MercurialInputScheme : InputScheme deletePath(tmpDir + "/.hg_archival.txt"); - auto storePath = store->addToStore(name, {makeFSSourceAccessor(), CanonPath(tmpDir)}); + auto storePath = store->addToStore(name, {getFSSourceAccessor(), CanonPath(tmpDir)}); Attrs infoAttrs({ {"rev", input.getRev()->gitRev()}, diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 24f6897c6..3b010350d 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -2492,7 +2492,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() HashModuloSink caSink { outputHash.hashAlgo, oldHashPart }; auto fim = outputHash.method.getFileIngestionMethod(); dumpPath( - {makeFSSourceAccessor(), CanonPath(actualPath)}, + {getFSSourceAccessor(), CanonPath(actualPath)}, caSink, (FileSerialisationMethod) fim); return caSink.finish().first; @@ -2500,7 +2500,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() case FileIngestionMethod::Git: { return git::dumpHash( outputHash.hashAlgo, - {makeFSSourceAccessor(), CanonPath(tmpDir + "/tmp")}).hash; + {getFSSourceAccessor(), CanonPath(tmpDir + "/tmp")}).hash; } } assert(false); @@ -2528,7 +2528,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() { HashResult narHashAndSize = hashPath( - {makeFSSourceAccessor(), CanonPath(actualPath)}, + {getFSSourceAccessor(), CanonPath(actualPath)}, FileSerialisationMethod::Recursive, HashAlgorithm::SHA256); newInfo0.narHash = narHashAndSize.first; newInfo0.narSize = narHashAndSize.second; @@ -2551,7 +2551,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() std::string { requiredFinalPath.hashPart() }); rewriteOutput(outputRewrites); HashResult narHashAndSize = hashPath( - {makeFSSourceAccessor(), CanonPath(actualPath)}, + {getFSSourceAccessor(), CanonPath(actualPath)}, FileSerialisationMethod::Recursive, HashAlgorithm::SHA256); ValidPathInfo newInfo0 { requiredFinalPath, narHashAndSize.first }; newInfo0.narSize = narHashAndSize.second; diff --git a/src/libstore/unix/local-store.cc b/src/libstore/unix/local-store.cc index 7b3ba1347..a3de523a3 100644 --- a/src/libstore/unix/local-store.cc +++ b/src/libstore/unix/local-store.cc @@ -1252,7 +1252,7 @@ StorePath LocalStore::addToStoreFromDump( methodsMatch ? dumpHash : hashPath( - {makeFSSourceAccessor(), CanonPath(tempPath)}, + {getFSSourceAccessor(), CanonPath(tempPath)}, hashMethod.getFileIngestionMethod(), hashAlgo), { .others = references, @@ -1392,7 +1392,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) Path linkPath = linksDir + "/" + link.name; PosixSourceAccessor accessor; std::string hash = hashPath( - {makeFSSourceAccessor(), CanonPath(linkPath)}, + {getFSSourceAccessor(), CanonPath(linkPath)}, FileIngestionMethod::Recursive, HashAlgorithm::SHA256).to_string(HashFormat::Nix32, false); if (hash != link.name) { printError("link '%s' was modified! expected hash '%s', got '%s'", diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc index f3551db42..e9c554939 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -167,7 +167,7 @@ void PosixSourceAccessor::assertNoSymlinks(CanonPath path) } } -ref makeFSSourceAccessor() +ref getFSSourceAccessor() { static auto rootFS = make_ref(); return rootFS; diff --git a/src/libutil/source-accessor.hh b/src/libutil/source-accessor.hh index b3fb9fe08..d7fb0af5f 100644 --- a/src/libutil/source-accessor.hh +++ b/src/libutil/source-accessor.hh @@ -197,10 +197,13 @@ MakeError(RestrictedPathError, Error); /** * Return an accessor for the root filesystem. */ -ref makeFSSourceAccessor(); +ref getFSSourceAccessor(); /** - * Return an accessor for the filesystem rooted at `root`. + * Construct an accessor for the filesystem rooted at `root`. Note + * that it is not possible to escape `root` by appending `..` path + * elements, and that absolute symlinks are resolved relative to + * `root`. */ ref makeFSSourceAccessor(std::filesystem::path root); From b7eb26e362c2b963b7300316d37c618f07505ccd Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 6 May 2024 20:00:44 +0200 Subject: [PATCH 3/3] Fix perl build --- perl/lib/Nix/Store.xs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 1d56e0127..ee211ef64 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -256,9 +256,8 @@ SV * hashPath(char * algo, int base32, char * path) PPCODE: try { - auto [accessor, canonPath] = PosixSourceAccessor::createAtRoot(path); Hash h = hashPath( - accessor, canonPath, + PosixSourceAccessor::createAtRoot(path), FileIngestionMethod::Recursive, parseHashAlgo(algo)); auto s = h.to_string(base32 ? HashFormat::Nix32 : HashFormat::Base16, false); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); @@ -336,10 +335,9 @@ StoreWrapper::addToStore(char * srcPath, int recursive, char * algo) PPCODE: try { auto method = recursive ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; - auto [accessor, canonPath] = PosixSourceAccessor::createAtRoot(srcPath); auto path = THIS->store->addToStore( std::string(baseNameOf(srcPath)), - accessor, canonPath, + PosixSourceAccessor::createAtRoot(srcPath), method, parseHashAlgo(algo)); XPUSHs(sv_2mortal(newSVpv(THIS->store->printStorePath(path).c_str(), 0))); } catch (Error & e) {