Merge pull request #9325 from NixOS/accessor-add-to-store

Content addressing and adding to store cleanup

src/libstore/binary-cache-store.cc

@@ -12,6 +12,7 @@
 #include "thread-pool.hh"
 #include "callback.hh"
 #include "signals.hh"
+#include "archive.hh"
 
 #include <chrono>
 #include <future>
@@ -300,24 +301,60 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource
     }});
 }
 
-StorePath BinaryCacheStore::addToStoreFromDump(Source & dump, std::string_view name,
-                                               FileIngestionMethod method, HashAlgorithm hashAlgo, RepairFlag repair, const StorePathSet & references)
+StorePath BinaryCacheStore::addToStoreFromDump(
+    Source & dump,
+    std::string_view name,
+    ContentAddressMethod method,
+    HashAlgorithm hashAlgo,
+    const StorePathSet & references,
+    RepairFlag repair)
 {
-    if (method != FileIngestionMethod::Recursive || hashAlgo != HashAlgorithm::SHA256)
-        unsupported("addToStoreFromDump");
-    return addToStoreCommon(dump, repair, CheckSigs, [&](HashResult nar) {
+    std::optional<Hash> caHash;
+    std::string nar;
+
+    if (auto * dump2p = dynamic_cast<StringSource *>(&dump)) {
+        auto & dump2 = *dump2p;
+        // Hack, this gives us a "replayable" source so we can compute
+        // multiple hashes more easily.
+        caHash = hashString(HashAlgorithm::SHA256, dump2.s);
+        switch (method.getFileIngestionMethod()) {
+        case FileIngestionMethod::Recursive:
+            // The dump is already NAR in this case, just use it.
+            nar = dump2.s;
+            break;
+        case FileIngestionMethod::Flat:
+            // The dump is Flat, so we need to convert it to NAR with a
+            // single file.
+            StringSink s;
+            dumpString(dump2.s, s);
+            nar = std::move(s.s);
+            break;
+        }
+    } else {
+        // Otherwise, we have to do th same hashing as NAR so our single
+        // hash will suffice for both purposes.
+        if (method != FileIngestionMethod::Recursive || hashAlgo != HashAlgorithm::SHA256)
+            unsupported("addToStoreFromDump");
+    }
+    StringSource narDump { nar };
+
+    // Use `narDump` if we wrote to `nar`.
+    Source & narDump2 = nar.size() > 0
+        ? static_cast<Source &>(narDump)
+        : dump;
+
+    return addToStoreCommon(narDump2, repair, CheckSigs, [&](HashResult nar) {
         ValidPathInfo info {
             *this,
             name,
-            FixedOutputInfo {
-                .method = method,
-                .hash = nar.first,
-                .references = {
+            ContentAddressWithReferences::fromParts(
+                method,
+                caHash ? *caHash : nar.first,
+                {
                     .others = references,
                     // caller is not capable of creating a self-reference, because this is content-addressed without modulus
                     .self = false,
-                },
-            },
+                }),
             nar.first,
         };
         info.narSize = nar.second;
@@ -399,72 +436,36 @@ void BinaryCacheStore::queryPathInfoUncached(const StorePath & storePath,
 }
 
 StorePath BinaryCacheStore::addToStore(
-        std::string_view name,
-        const Path & srcPath,
-        FileIngestionMethod method,
-        HashAlgorithm hashAlgo,
-        PathFilter & filter,
-        RepairFlag repair,
-        const StorePathSet & references)
+    std::string_view name,
+    SourceAccessor & accessor,
+    const CanonPath & path,
+    ContentAddressMethod method,
+    HashAlgorithm hashAlgo,
+    const StorePathSet & references,
+    PathFilter & filter,
+    RepairFlag repair)
 {
     /* FIXME: Make BinaryCacheStore::addToStoreCommon support
        non-recursive+sha256 so we can just use the default
        implementation of this method in terms of addToStoreFromDump. */
 
-    HashSink sink { hashAlgo };
-    if (method == FileIngestionMethod::Recursive) {
-        dumpPath(srcPath, sink, filter);
-    } else {
-        readFile(srcPath, sink);
-    }
-    auto h = sink.finish().first;
+    auto h = hashPath(accessor, path, method.getFileIngestionMethod(), hashAlgo, filter).first;
 
     auto source = sinkToSource([&](Sink & sink) {
-        dumpPath(srcPath, sink, filter);
+        accessor.dumpPath(path, sink, filter);
     });
     return addToStoreCommon(*source, repair, CheckSigs, [&](HashResult nar) {
         ValidPathInfo info {
             *this,
             name,
-            FixedOutputInfo {
-                .method = method,
-                .hash = h,
-                .references = {
+            ContentAddressWithReferences::fromParts(
+                method,
+                h,
+                {
                     .others = references,
                     // caller is not capable of creating a self-reference, because this is content-addressed without modulus
                     .self = false,
-                },
-            },
-            nar.first,
-        };
-        info.narSize = nar.second;
-        return info;
-    })->path;
-}
-
-StorePath BinaryCacheStore::addTextToStore(
-    std::string_view name,
-    std::string_view s,
-    const StorePathSet & references,
-    RepairFlag repair)
-{
-    auto textHash = hashString(HashAlgorithm::SHA256, s);
-    auto path = makeTextPath(name, TextInfo { { textHash }, references });
-
-    if (!repair && isValidPath(path))
-        return path;
-
-    StringSink sink;
-    dumpString(s, sink);
-    StringSource source(sink.s);
-    return addToStoreCommon(source, repair, CheckSigs, [&](HashResult nar) {
-        ValidPathInfo info {
-            *this,
-            std::string { name },
-            TextInfo {
-                .hash = textHash,
-                .references = references,
-            },
+                }),
             nar.first,
         };
         info.narSize = nar.second;

src/libstore/binary-cache-store.hh

@@ -123,22 +123,22 @@ public:
     void addToStore(const ValidPathInfo & info, Source & narSource,
         RepairFlag repair, CheckSigsFlag checkSigs) override;
 
-    StorePath addToStoreFromDump(Source & dump, std::string_view name,
-                                 FileIngestionMethod method, HashAlgorithm hashAlgo, RepairFlag repair, const StorePathSet & references) override;
+    StorePath addToStoreFromDump(
+        Source & dump,
+        std::string_view name,
+        ContentAddressMethod method,
+        HashAlgorithm hashAlgo,
+        const StorePathSet & references,
+        RepairFlag repair) override;
 
     StorePath addToStore(
-            std::string_view name,
-            const Path & srcPath,
-            FileIngestionMethod method,
-            HashAlgorithm hashAlgo,
-            PathFilter & filter,
-            RepairFlag repair,
-            const StorePathSet & references) override;
-
-    StorePath addTextToStore(
         std::string_view name,
-        std::string_view s,
+        SourceAccessor & accessor,
+        const CanonPath & srcPath,
+        ContentAddressMethod method,
+        HashAlgorithm hashAlgo,
         const StorePathSet & references,
+        PathFilter & filter,
         RepairFlag repair) override;
 
     void registerDrvOutput(const Realisation & info) override;

src/libstore/build/local-derivation-goal.cc

@@ -20,6 +20,7 @@
 #include "child.hh"
 #include "unix-domain-socket.hh"
 #include "posix-fs-canonicalise.hh"
+#include "posix-source-accessor.hh"
 
 #include <regex>
 #include <queue>
@@ -1290,13 +1291,14 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual In
     { throw Error("queryPathFromHashPart"); }
 
     StorePath addToStore(
-            std::string_view name,
-            const Path & srcPath,
-            FileIngestionMethod method,
-            HashAlgorithm hashAlgo,
-            PathFilter & filter,
-            RepairFlag repair,
-            const StorePathSet & references) override
+        std::string_view name,
+        SourceAccessor & accessor,
+        const CanonPath & srcPath,
+        ContentAddressMethod method,
+        HashAlgorithm hashAlgo,
+        const StorePathSet & references,
+        PathFilter & filter,
+        RepairFlag repair) override
     { throw Error("addToStore"); }
 
     void addToStore(const ValidPathInfo & info, Source & narSource,
@@ -1306,26 +1308,15 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual In
         goal.addDependency(info.path);
     }
 
-    StorePath addTextToStore(
-        std::string_view name,
-        std::string_view s,
-        const StorePathSet & references,
-        RepairFlag repair = NoRepair) override
-    {
-        auto path = next->addTextToStore(name, s, references, repair);
-        goal.addDependency(path);
-        return path;
-    }
-
     StorePath addToStoreFromDump(
-            Source & dump,
-            std::string_view name,
-            FileIngestionMethod method,
-            HashAlgorithm hashAlgo,
-            RepairFlag repair,
-            const StorePathSet & references) override
+        Source & dump,
+        std::string_view name,
+        ContentAddressMethod method,
+        HashAlgorithm hashAlgo,
+        const StorePathSet & references,
+        RepairFlag repair) override
     {
-        auto path = next->addToStoreFromDump(dump, name, method, hashAlgo, repair, references);
+        auto path = next->addToStoreFromDump(dump, name, method, hashAlgo, references, repair);
         goal.addDependency(path);
         return path;
     }
@@ -2453,8 +2444,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
                 throw BuildError(
                     "output path %1% without valid stats info",
                     actualPath);
-            if (outputHash.method == ContentAddressMethod { FileIngestionMethod::Flat } ||
-                outputHash.method == ContentAddressMethod { TextIngestionMethod {} })
+            if (outputHash.method.getFileIngestionMethod() == FileIngestionMethod::Flat)
             {
                 /* The output path should be a regular file without execute permission. */
                 if (!S_ISREG(st->st_mode) || (st->st_mode & S_IXUSR) != 0)
@@ -2466,38 +2456,23 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
             rewriteOutput(outputRewrites);
             /* FIXME optimize and deduplicate with addToStore */
             std::string oldHashPart { scratchPath->hashPart() };
-            HashModuloSink caSink {outputHash.hashAlgo, oldHashPart };
-            std::visit(overloaded {
-                [&](const TextIngestionMethod &) {
-                    readFile(actualPath, caSink);
-                },
-                [&](const FileIngestionMethod & m2) {
-                    switch (m2) {
-                    case FileIngestionMethod::Recursive:
-                        dumpPath(actualPath, caSink);
-                        break;
-                    case FileIngestionMethod::Flat:
-                        readFile(actualPath, caSink);
-                        break;
-                    }
-                },
-            }, outputHash.method.raw);
-            auto got = caSink.finish().first;
+            auto got = ({
+                HashModuloSink caSink { outputHash.hashAlgo, oldHashPart };
+                PosixSourceAccessor accessor;
+                dumpPath(
+                    accessor, CanonPath { actualPath },
+                    caSink,
+                    outputHash.method.getFileIngestionMethod());
+                caSink.finish().first;
+            });
 
-            auto optCA = ContentAddressWithReferences::fromPartsOpt(
-                outputHash.method,
-                std::move(got),
-                rewriteRefs());
-            if (!optCA) {
-                // TODO track distinct failure modes separately (at the time of
-                // writing there is just one but `nullopt` is unclear) so this
-                // message can't get out of sync.
-                throw BuildError("output path '%s' has illegal content address, probably a spurious self-reference with text hashing");
-            }
             ValidPathInfo newInfo0 {
                 worker.store,
                 outputPathName(drv->name, outputName),
-                std::move(*optCA),
+                ContentAddressWithReferences::fromParts(
+                    outputHash.method,
+                    std::move(got),
+                    rewriteRefs()),
                 Hash::dummy,
             };
             if (*scratchPath != newInfo0.path) {
@@ -2511,9 +2486,14 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
                                std::string(newInfo0.path.hashPart())}});
             }
 
-            HashResult narHashAndSize = hashPath(HashAlgorithm::SHA256, actualPath);
-            newInfo0.narHash = narHashAndSize.first;
-            newInfo0.narSize = narHashAndSize.second;
+            {
+                PosixSourceAccessor accessor;
+                HashResult narHashAndSize = hashPath(
+                    accessor, CanonPath { actualPath },
+                    FileIngestionMethod::Recursive, HashAlgorithm::SHA256);
+                newInfo0.narHash = narHashAndSize.first;
+                newInfo0.narSize = narHashAndSize.second;
+            }
 
             assert(newInfo0.ca);
             return newInfo0;
@@ -2531,7 +2511,10 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
                         std::string { scratchPath->hashPart() },
                         std::string { requiredFinalPath.hashPart() });
                 rewriteOutput(outputRewrites);
-                auto narHashAndSize = hashPath(HashAlgorithm::SHA256, actualPath);
+                PosixSourceAccessor accessor;
+                HashResult narHashAndSize = hashPath(
+                    accessor, CanonPath { actualPath },
+                    FileIngestionMethod::Recursive, HashAlgorithm::SHA256);
                 ValidPathInfo newInfo0 { requiredFinalPath, narHashAndSize.first };
                 newInfo0.narSize = narHashAndSize.second;
                 auto refs = rewriteRefs();

src/libstore/build/worker.cc

@@ -519,7 +519,9 @@ bool Worker::pathContentsGood(const StorePath & path)
     if (!pathExists(store.printStorePath(path)))
         res = false;
     else {
-        HashResult current = hashPath(info->narHash.algo, store.printStorePath(path));
+        HashResult current = hashPath(
+            *store.getFSAccessor(), CanonPath { store.printStorePath(path) },
+            FileIngestionMethod::Recursive, info->narHash.algo);
         Hash nullHash(HashAlgorithm::SHA256);
         res = info->narHash == nullHash || info->narHash == current.first;
     }

src/libstore/content-address.cc

@@ -50,6 +50,18 @@ std::string ContentAddressMethod::render(HashAlgorithm ha) const
     }, raw);
 }
 
+FileIngestionMethod ContentAddressMethod::getFileIngestionMethod() const
+{
+    return std::visit(overloaded {
+        [&](const TextIngestionMethod & th) {
+            return FileIngestionMethod::Flat;
+        },
+        [&](const FileIngestionMethod & fim) {
+            return fim;
+        }
+    }, raw);
+}
+
 std::string ContentAddress::render() const
 {
     return std::visit(overloaded {
@@ -79,7 +91,7 @@ static std::pair<ContentAddressMethod, HashAlgorithm> parseContentAddressMethodP
         prefix = *optPrefix;
     }
 
-    auto parseHashType_ = [&](){
+    auto parseHashAlgorithm_ = [&](){
         auto hashTypeRaw = splitPrefixTo(rest, ':');
         if (!hashTypeRaw)
             throw UsageError("content address hash must be in form '<algo>:<hash>', but found: %s", wholeInput);
@@ -90,7 +102,7 @@ static std::pair<ContentAddressMethod, HashAlgorithm> parseContentAddressMethodP
     // Switch on prefix
     if (prefix == "text") {
         // No parsing of the ingestion method, "text" only support flat.
-        HashAlgorithm hashAlgo = parseHashType_();
+        HashAlgorithm hashAlgo = parseHashAlgorithm_();
         return {
             TextIngestionMethod {},
             std::move(hashAlgo),
@@ -100,7 +112,7 @@ static std::pair<ContentAddressMethod, HashAlgorithm> parseContentAddressMethodP
         auto method = FileIngestionMethod::Flat;
         if (splitPrefix(rest, "r:"))
             method = FileIngestionMethod::Recursive;
-        HashAlgorithm hashAlgo = parseHashType_();
+        HashAlgorithm hashAlgo = parseHashAlgorithm_();
         return {
             std::move(method),
             std::move(hashAlgo),
@@ -176,13 +188,13 @@ ContentAddressWithReferences ContentAddressWithReferences::withoutRefs(const Con
     }, ca.method.raw);
 }
 
-std::optional<ContentAddressWithReferences> ContentAddressWithReferences::fromPartsOpt(
-    ContentAddressMethod method, Hash hash, StoreReferences refs) noexcept
+ContentAddressWithReferences ContentAddressWithReferences::fromParts(
+    ContentAddressMethod method, Hash hash, StoreReferences refs)
 {
     return std::visit(overloaded {
-        [&](TextIngestionMethod _) -> std::optional<ContentAddressWithReferences> {
+        [&](TextIngestionMethod _) -> ContentAddressWithReferences {
             if (refs.self)
-                return std::nullopt;
+                throw Error("self-reference not allowed with text hashing");
             return ContentAddressWithReferences {
                 TextInfo {
                     .hash = std::move(hash),
@@ -190,7 +202,7 @@ std::optional<ContentAddressWithReferences> ContentAddressWithReferences::fromPa
                 }
             };
         },
-        [&](FileIngestionMethod m2) -> std::optional<ContentAddressWithReferences> {
+        [&](FileIngestionMethod m2) -> ContentAddressWithReferences {
             return ContentAddressWithReferences {
                 FixedOutputInfo {
                     .method = m2,

src/libstore/content-address.hh

@@ -4,6 +4,7 @@
 #include <variant>
 #include "hash.hh"
 #include "path.hh"
+#include "file-content-address.hh"
 #include "comparator.hh"
 #include "variant-wrapper.hh"
 
@@ -31,22 +32,6 @@ namespace nix {
  */
 struct TextIngestionMethod : std::monostate { };
 
-/**
- * An enumeration of the main ways we can serialize file system
- * objects.
- */
-enum struct FileIngestionMethod : uint8_t {
-    /**
-     * Flat-file hashing. Directly ingest the contents of a single file
-     */
-    Flat = 0,
-    /**
-     * Recursive (or NAR) hashing. Serializes the file-system object in Nix
-     * Archive format and ingest that
-     */
-    Recursive = 1
-};
-
 /**
  * Compute the prefix to the hash algorithm which indicates how the
  * files were ingested.
@@ -54,7 +39,7 @@ enum struct FileIngestionMethod : uint8_t {
 std::string makeFileIngestionPrefix(FileIngestionMethod m);
 
 /**
- * An enumeration of all the ways we can serialize file system objects.
+ * An enumeration of all the ways we can content-address store objects.
  *
  * Just the type of a content address. Combine with the hash itself, and
  * we have a `ContentAddress` as defined below. Combine that, in turn,
@@ -102,7 +87,15 @@ struct ContentAddressMethod
      *
      * The rough inverse of `parse()`.
      */
-    std::string render(HashAlgorithm ha) const;
+    std::string render(HashAlgorithm ht) const;
+
+    /**
+     * Get the underlying way to content-address file system objects.
+     *
+     * Different ways of hashing store objects may use the same method
+     * for hashing file systeme objects.
+     */
+    FileIngestionMethod getFileIngestionMethod() const;
 };
 
 
@@ -116,11 +109,11 @@ struct ContentAddressMethod
  * serialisation methods (flat file vs NAR). Thus, ‘ca’ has one of the
  * following forms:
  *
- * - ‘text:sha256:<sha256 hash of file contents>’: For paths
- *   computed by Store::makeTextPath() / Store::addTextToStore().
+ * - `TextIngestionMethod`:
+ *   ‘text:sha256:<sha256 hash of file contents>’
  *
- * - ‘fixed:<r?>:<ht>:<h>’: For paths computed by
- *   Store::makeFixedOutputPath() / Store::addToStore().
+ * - `FixedIngestionMethod`:
+ *   ‘fixed:<r?>:<hash type>:<hash of file contents>’
  */
 struct ContentAddress
 {
@@ -266,11 +259,12 @@ struct ContentAddressWithReferences
      *
      * @param refs References to other store objects or oneself.
      *
-     * Do note that not all combinations are supported; `nullopt` is
-     * returns for invalid combinations.
+     * @note note that all combinations are supported. This is a
+     * *partial function* and exceptions will be thrown for invalid
+     * combinations.
      */
-    static std::optional<ContentAddressWithReferences> fromPartsOpt(
-        ContentAddressMethod method, Hash hash, StoreReferences refs) noexcept;
+    static ContentAddressWithReferences fromParts(
+        ContentAddressMethod method, Hash hash, StoreReferences refs);
 
     ContentAddressMethod getMethod() const;
 

src/libstore/daemon.cc

@@ -403,22 +403,9 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
                 auto [contentAddressMethod, hashAlgo_] = ContentAddressMethod::parse(camStr);
                 auto hashAlgo = hashAlgo_; // work around clang bug
                 FramedSource source(from);
-                // TODO this is essentially RemoteStore::addCAToStore. Move it up to Store.
-                return std::visit(overloaded {
-                    [&](const TextIngestionMethod &) {
-                        if (hashAlgo != HashAlgorithm::SHA256)
-                            throw UnimplementedError("When adding text-hashed data called '%s', only SHA-256 is supported but '%s' was given",
-                                name, printHashAlgo(hashAlgo));
-                        // We could stream this by changing Store
-                        std::string contents = source.drain();
-                        auto path = store->addTextToStore(name, contents, refs, repair);
-                        return store->queryPathInfo(path);
-                    },
-                    [&](const FileIngestionMethod & fim) {
-                        auto path = store->addToStoreFromDump(source, name, fim, hashAlgo, repair, refs);
-                        return store->queryPathInfo(path);
-                    },
-                }, contentAddressMethod.raw);
+                // TODO these two steps are essentially RemoteStore::addCAToStore. Move it up to Store.
+                auto path = store->addToStoreFromDump(source, name, contentAddressMethod, hashAlgo, refs, repair);
+                return store->queryPathInfo(path);
             }();
             logger->stopWork();
 
@@ -496,7 +483,10 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         std::string s = readString(from);
         auto refs = WorkerProto::Serialise<StorePathSet>::read(*store, rconn);
         logger->startWork();
-        auto path = store->addTextToStore(suffix, s, refs, NoRepair);
+        auto path = ({
+            StringSource source { s };
+            store->addToStoreFromDump(source, suffix, TextIngestionMethod {}, HashAlgorithm::SHA256, refs, NoRepair);
+        });
         logger->stopWork();
         to << store->printStorePath(path);
         break;

src/libstore/derivations.cc

@@ -143,8 +143,14 @@ StorePath writeDerivation(Store & store,
     auto suffix = std::string(drv.name) + drvExtension;
     auto contents = drv.unparse(store, false);
     return readOnly || settings.readOnlyMode
-        ? store.computeStorePathForText(suffix, contents, references)
-        : store.addTextToStore(suffix, contents, references, repair);
+        ? store.makeFixedOutputPathFromCA(suffix, TextInfo {
+            .hash = hashString(HashAlgorithm::SHA256, contents),
+            .references = std::move(references),
+        })
+        : ({
+            StringSource s { contents };
+            store.addToStoreFromDump(s, suffix, TextIngestionMethod {}, HashAlgorithm::SHA256, references, repair);
+        });
 }
 
 

src/libstore/dummy-store.cc

@@ -58,13 +58,6 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store
         RepairFlag repair, CheckSigsFlag checkSigs) override
     { unsupported("addToStore"); }
 
-    StorePath addTextToStore(
-        std::string_view name,
-        std::string_view s,
-        const StorePathSet & references,
-        RepairFlag repair) override
-    { unsupported("addTextToStore"); }
-
     void narFromPath(const StorePath & path, Sink & sink) override
     { unsupported("narFromPath"); }
 

src/libstore/legacy-ssh-store.hh

@@ -59,21 +59,15 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
     { unsupported("queryPathFromHashPart"); }
 
     StorePath addToStore(
-            std::string_view name,
-            const Path & srcPath,
-            FileIngestionMethod method,
-            HashAlgorithm hashAlgo,
-            PathFilter & filter,
-            RepairFlag repair,
-            const StorePathSet & references) override
-    { unsupported("addToStore"); }
-
-    StorePath addTextToStore(
         std::string_view name,
-        std::string_view s,
+        SourceAccessor & accessor,
+        const CanonPath & srcPath,
+        ContentAddressMethod method,
+        HashAlgorithm hashAlgo,
         const StorePathSet & references,
+        PathFilter & filter,
         RepairFlag repair) override
-    { unsupported("addTextToStore"); }
+    { unsupported("addToStore"); }
 
 private:
 

src/libstore/local-store.cc

@@ -13,6 +13,7 @@
 #include "compression.hh"
 #include "signals.hh"
 #include "posix-fs-canonicalise.hh"
+#include "posix-source-accessor.hh"
 
 #include <iostream>
 #include <algorithm>
@@ -1088,11 +1089,22 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
 
             if (info.ca) {
                 auto & specified = *info.ca;
-                auto actualHash = hashCAPath(
-                    specified.method,
-                    specified.hash.algo,
-                    info.path
-                );
+                auto actualHash = ({
+                    HashModuloSink caSink {
+                        specified.hash.algo,
+                        std::string { info.path.hashPart() },
+                    };
+                    PosixSourceAccessor accessor;
+                    dumpPath(
+                        *getFSAccessor(false),
+                        CanonPath { printStorePath(info.path) },
+                        caSink,
+                        specified.method.getFileIngestionMethod());
+                    ContentAddress {
+                        .method = specified.method,
+                        .hash = caSink.finish().first,
+                    };
+                });
                 if (specified.hash != actualHash.hash) {
                     throw Error("ca hash mismatch importing path '%s';\n  specified: %s\n  got:       %s",
                         printStorePath(info.path),
@@ -1115,8 +1127,13 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
 }
 
 
-StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name,
-                                         FileIngestionMethod method, HashAlgorithm hashAlgo, RepairFlag repair, const StorePathSet & references)
+StorePath LocalStore::addToStoreFromDump(
+    Source & source0,
+    std::string_view name,
+    ContentAddressMethod method,
+    HashAlgorithm hashAlgo,
+    const StorePathSet & references,
+    RepairFlag repair)
 {
     /* For computing the store path. */
     auto hashSink = std::make_unique<HashSink>(hashAlgo);
@@ -1166,25 +1183,21 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name
         delTempDir = std::make_unique<AutoDelete>(tempDir);
         tempPath = tempDir + "/x";
 
-        if (method == FileIngestionMethod::Recursive)
-            restorePath(tempPath, bothSource);
-        else
-            writeFile(tempPath, bothSource);
+        restorePath(tempPath, bothSource, method.getFileIngestionMethod());
 
         dump.clear();
     }
 
     auto [hash, size] = hashSink->finish();
 
-    ContentAddressWithReferences desc = FixedOutputInfo {
-        .method = method,
-        .hash = hash,
-        .references = {
+    auto desc = ContentAddressWithReferences::fromParts(
+        method,
+        hash,
+        {
             .others = references,
             // caller is not capable of creating a self-reference, because this is content-addressed without modulus
             .self = false,
-        },
-    };
+        });
 
     auto dstPath = makeFixedOutputPathFromCA(name, desc);
 
@@ -1207,11 +1220,8 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name
 
             if (inMemory) {
                 StringSource dumpSource { dump };
-                /* Restore from the NAR in memory. */
-                if (method == FileIngestionMethod::Recursive)
-                    restorePath(realPath, dumpSource);
-                else
-                    writeFile(realPath, dumpSource);
+                /* Restore from the buffer in memory. */
+                restorePath(realPath, dumpSource, method.getFileIngestionMethod());
             } else {
                 /* Move the temporary path we restored above. */
                 moveFile(tempPath, realPath);
@@ -1247,58 +1257,6 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name
 }
 
 
-StorePath LocalStore::addTextToStore(
-    std::string_view name,
-    std::string_view s,
-    const StorePathSet & references, RepairFlag repair)
-{
-    auto hash = hashString(HashAlgorithm::SHA256, s);
-    auto dstPath = makeTextPath(name, TextInfo {
-        .hash = hash,
-        .references = references,
-    });
-
-    addTempRoot(dstPath);
-
-    if (repair || !isValidPath(dstPath)) {
-
-        auto realPath = Store::toRealPath(dstPath);
-
-        PathLocks outputLock({realPath});
-
-        if (repair || !isValidPath(dstPath)) {
-
-            deletePath(realPath);
-
-            autoGC();
-
-            writeFile(realPath, s);
-
-            canonicalisePathMetaData(realPath, {});
-
-            StringSink sink;
-            dumpString(s, sink);
-            auto narHash = hashString(HashAlgorithm::SHA256, sink.s);
-
-            optimisePath(realPath, repair);
-
-            ValidPathInfo info { dstPath, narHash };
-            info.narSize = sink.s.size();
-            info.references = references;
-            info.ca = {
-                .method = TextIngestionMethod {},
-                .hash = hash,
-            };
-            registerValidPath(info);
-        }
-
-        outputLock.setDeletion(true);
-    }
-
-    return dstPath;
-}
-
-
 /* Create a temporary directory in the store that won't be
    garbage-collected until the returned FD is closed. */
 std::pair<Path, AutoCloseFD> LocalStore::createTempDirInStore()
@@ -1389,7 +1347,10 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
         for (auto & link : readDirectory(linksDir)) {
             printMsg(lvlTalkative, "checking contents of '%s'", link.name);
             Path linkPath = linksDir + "/" + link.name;
-            std::string hash = hashPath(HashAlgorithm::SHA256, linkPath).first.to_string(HashFormat::Nix32, false);
+            PosixSourceAccessor accessor;
+            std::string hash = hashPath(
+                accessor, CanonPath { linkPath },
+                FileIngestionMethod::Recursive, HashAlgorithm::SHA256).first.to_string(HashFormat::Nix32, false);
             if (hash != link.name) {
                 printError("link '%s' was modified! expected hash '%s', got '%s'",
                     linkPath, link.name, hash);
@@ -1696,42 +1657,6 @@ void LocalStore::queryRealisationUncached(const DrvOutput & id,
     }
 }
 
-ContentAddress LocalStore::hashCAPath(
-    const ContentAddressMethod & method, const HashAlgorithm & hashAlgo,
-    const StorePath & path)
-{
-    return hashCAPath(method, hashAlgo, Store::toRealPath(path), path.hashPart());
-}
-
-ContentAddress LocalStore::hashCAPath(
-    const ContentAddressMethod & method,
-    const HashAlgorithm & hashAlgo,
-    const Path & path,
-    const std::string_view pathHash
-)
-{
-    HashModuloSink caSink ( hashAlgo, std::string(pathHash) );
-    std::visit(overloaded {
-        [&](const TextIngestionMethod &) {
-            readFile(path, caSink);
-        },
-        [&](const FileIngestionMethod & m2) {
-            switch (m2) {
-            case FileIngestionMethod::Recursive:
-                dumpPath(path, caSink);
-                break;
-            case FileIngestionMethod::Flat:
-                readFile(path, caSink);
-                break;
-            }
-        },
-    }, method.raw);
-    return ContentAddress {
-        .method = method,
-        .hash = caSink.finish().first,
-    };
-}
-
 void LocalStore::addBuildLog(const StorePath & drvPath, std::string_view log)
 {
     assert(drvPath.isDerivation());

src/libstore/local-store.hh

@@ -177,12 +177,11 @@ public:
     void addToStore(const ValidPathInfo & info, Source & source,
         RepairFlag repair, CheckSigsFlag checkSigs) override;
 
-    StorePath addToStoreFromDump(Source & dump, std::string_view name,
-                                 FileIngestionMethod method, HashAlgorithm hashAlgo, RepairFlag repair, const StorePathSet & references) override;
-
-    StorePath addTextToStore(
+    StorePath addToStoreFromDump(
+        Source & dump,
         std::string_view name,
-        std::string_view s,
+        ContentAddressMethod method,
+        HashAlgorithm hashAlgo,
         const StorePathSet & references,
         RepairFlag repair) override;
 
@@ -350,19 +349,6 @@ private:
     void signPathInfo(ValidPathInfo & info);
     void signRealisation(Realisation &);
 
-    // XXX: Make a generic `Store` method
-    ContentAddress hashCAPath(
-        const ContentAddressMethod & method,
-        const HashAlgorithm & hashAlgo,
-        const StorePath & path);
-
-    ContentAddress hashCAPath(
-        const ContentAddressMethod & method,
-        const HashAlgorithm & hashAlgo,
-        const Path & path,
-        const std::string_view pathHash
-    );
-
     void addBuildLog(const StorePath & drvPath, std::string_view log) override;
 
     friend struct LocalDerivationGoal;

src/libstore/optimise-store.cc

@@ -2,6 +2,7 @@
 #include "globals.hh"
 #include "signals.hh"
 #include "posix-fs-canonicalise.hh"
+#include "posix-source-accessor.hh"
 
 #include <cstdlib>
 #include <cstring>
@@ -146,7 +147,12 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
        Also note that if `path' is a symlink, then we're hashing the
        contents of the symlink (i.e. the result of readlink()), not
        the contents of the target (which may not even exist). */
-    Hash hash = hashPath(HashAlgorithm::SHA256, path).first;
+    Hash hash = ({
+        PosixSourceAccessor accessor;
+        hashPath(
+            accessor, CanonPath { path },
+            FileIngestionMethod::Recursive, HashAlgorithm::SHA256).first;
+    });
     debug("'%1%' has hash '%2%'", path, hash.to_string(HashFormat::Nix32, true));
 
     /* Check if this is a known hash. */
@@ -156,7 +162,12 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
     if (pathExists(linkPath)) {
         auto stLink = lstat(linkPath);
         if (st.st_size != stLink.st_size
-            || (repair && hash != hashPath(HashAlgorithm::SHA256, linkPath).first))
+            || (repair && hash != ({
+                PosixSourceAccessor accessor;
+                hashPath(
+                    accessor, CanonPath { linkPath },
+                    FileIngestionMethod::Recursive, HashAlgorithm::SHA256).first;
+           })))
         {
             // XXX: Consider overwriting linkPath with our valid version.
             warn("removing corrupted link '%s'", linkPath);

src/libstore/remote-store.cc

@@ -502,8 +502,13 @@ ref<const ValidPathInfo> RemoteStore::addCAToStore(
 }
 
 
-StorePath RemoteStore::addToStoreFromDump(Source & dump, std::string_view name,
-                                          FileIngestionMethod method, HashAlgorithm hashAlgo, RepairFlag repair, const StorePathSet & references)
+StorePath RemoteStore::addToStoreFromDump(
+    Source & dump,
+    std::string_view name,
+    ContentAddressMethod method,
+    HashAlgorithm hashAlgo,
+    const StorePathSet & references,
+    RepairFlag repair)
 {
     return addCAToStore(dump, name, method, hashAlgo, references, repair)->path;
 }
@@ -603,16 +608,6 @@ void RemoteStore::addMultipleToStore(
 }
 
 
-StorePath RemoteStore::addTextToStore(
-    std::string_view name,
-    std::string_view s,
-    const StorePathSet & references,
-    RepairFlag repair)
-{
-    StringSource source(s);
-    return addCAToStore(source, name, TextIngestionMethod {}, HashAlgorithm::SHA256, references, repair)->path;
-}
-
 void RemoteStore::registerDrvOutput(const Realisation & info)
 {
     auto conn(getConnection());

src/libstore/remote-store.hh

@@ -82,10 +82,15 @@ public:
             RepairFlag repair);
 
     /**
-     * Add a content-addressable store path. Does not support references. `dump` will be drained.
+     * Add a content-addressable store path. `dump` will be drained.
      */
-    StorePath addToStoreFromDump(Source & dump, std::string_view name,
-                                 FileIngestionMethod method = FileIngestionMethod::Recursive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, RepairFlag repair = NoRepair, const StorePathSet & references = StorePathSet()) override;
+    StorePath addToStoreFromDump(
+        Source & dump,
+        std::string_view name,
+        ContentAddressMethod method = FileIngestionMethod::Recursive,
+        HashAlgorithm hashAlgo = HashAlgorithm::SHA256,
+        const StorePathSet & references = StorePathSet(),
+        RepairFlag repair = NoRepair) override;
 
     void addToStore(const ValidPathInfo & info, Source & nar,
         RepairFlag repair, CheckSigsFlag checkSigs) override;
@@ -101,12 +106,6 @@ public:
         RepairFlag repair,
         CheckSigsFlag checkSigs) override;
 
-    StorePath addTextToStore(
-        std::string_view name,
-        std::string_view s,
-        const StorePathSet & references,
-        RepairFlag repair) override;
-
     void registerDrvOutput(const Realisation & info) override;
 
     void queryRealisationUncached(const DrvOutput &,

src/libstore/store-api.cc

@@ -205,25 +205,19 @@ StorePath StoreDirConfig::makeFixedOutputPath(std::string_view name, const Fixed
 }
 
 
-StorePath StoreDirConfig::makeTextPath(std::string_view name, const TextInfo & info) const
-{
-    assert(info.hash.algo == HashAlgorithm::SHA256);
-    return makeStorePath(
-        makeType(*this, "text", StoreReferences {
-            .others = info.references,
-            .self = false,
-        }),
-        info.hash,
-        name);
-}
-
-
 StorePath StoreDirConfig::makeFixedOutputPathFromCA(std::string_view name, const ContentAddressWithReferences & ca) const
 {
     // New template
     return std::visit(overloaded {
         [&](const TextInfo & ti) {
-            return makeTextPath(name, ti);
+            assert(ti.hash.algo == HashAlgorithm::SHA256);
+            return makeStorePath(
+                makeType(*this, "text", StoreReferences {
+                    .others = ti.references,
+                    .self = false,
+                }),
+                ti.hash,
+                name);
         },
         [&](const FixedOutputInfo & foi) {
             return makeFixedOutputPath(name, foi);
@@ -232,54 +226,45 @@ StorePath StoreDirConfig::makeFixedOutputPathFromCA(std::string_view name, const
 }
 
 
-std::pair<StorePath, Hash> StoreDirConfig::computeStorePathFromDump(
-        Source & dump,
-        std::string_view name,
-        FileIngestionMethod method,
-        HashAlgorithm hashAlgo,
-        const StorePathSet & references) const
-{
-    HashSink sink(hashAlgo);
-    dump.drainInto(sink);
-    auto h = sink.finish().first;
-    FixedOutputInfo caInfo {
-        .method = method,
-        .hash = h,
-        .references = {},
-    };
-    return std::make_pair(makeFixedOutputPath(name, caInfo), h);
-}
-
-
-StorePath StoreDirConfig::computeStorePathForText(
+std::pair<StorePath, Hash> StoreDirConfig::computeStorePath(
     std::string_view name,
-    std::string_view s,
-    const StorePathSet & references) const
+    SourceAccessor & accessor,
+    const CanonPath & path,
+    ContentAddressMethod method,
+    HashAlgorithm hashAlgo,
+    const StorePathSet & references,
+    PathFilter & filter) const
 {
-    return makeTextPath(name, TextInfo {
-        .hash = hashString(HashAlgorithm::SHA256, s),
-        .references = references,
-    });
+    auto h = hashPath(accessor, path, method.getFileIngestionMethod(), hashAlgo, filter).first;
+    return {
+        makeFixedOutputPathFromCA(
+            name,
+            ContentAddressWithReferences::fromParts(
+                method,
+                h,
+                {
+                    .others = references,
+                    .self = false,
+                })),
+        h,
+    };
 }
 
 
 StorePath Store::addToStore(
-        std::string_view name,
-        const Path & _srcPath,
-        FileIngestionMethod method,
-        HashAlgorithm hashAlgo,
-        PathFilter & filter,
-        RepairFlag repair,
-        const StorePathSet & references)
+    std::string_view name,
+    SourceAccessor & accessor,
+    const CanonPath & path,
+    ContentAddressMethod method,
+    HashAlgorithm hashAlgo,
+    const StorePathSet & references,
+    PathFilter & filter,
+    RepairFlag repair)
 {
-    Path srcPath(absPath(_srcPath));
     auto source = sinkToSource([&](Sink & sink) {
-        if (method == FileIngestionMethod::Recursive)
-            dumpPath(srcPath, sink, filter);
-        else
-            readFile(srcPath, sink);
+        dumpPath(accessor, path, sink, method.getFileIngestionMethod(), filter);
     });
-    return addToStoreFromDump(*source, name, method, hashAlgo, repair, references);
+    return addToStoreFromDump(*source, name, method, hashAlgo, references, repair);
 }
 
 void Store::addMultipleToStore(
@@ -404,9 +389,13 @@ digraph graphname {
     fileSink -> caHashSink
 }
 */
-ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
-                                    FileIngestionMethod method, HashAlgorithm hashAlgo,
-                                    std::optional<Hash> expectedCAHash)
+ValidPathInfo Store::addToStoreSlow(
+    std::string_view name,
+    SourceAccessor & accessor,
+    const CanonPath & srcPath,
+    ContentAddressMethod method, HashAlgorithm hashAlgo,
+    const StorePathSet & references,
+    std::optional<Hash> expectedCAHash)
 {
     HashSink narHashSink { HashAlgorithm::SHA256 };
     HashSink caHashSink { hashAlgo };
@@ -425,7 +414,7 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
        srcPath. The fact that we use scratchpadSink as a temporary buffer here
        is an implementation detail. */
     auto fileSource = sinkToSource([&](Sink & scratchpadSink) {
-        dumpPath(srcPath, scratchpadSink);
+        accessor.dumpPath(srcPath, scratchpadSink);
     });
 
     /* tapped provides the same data as fileSource, but we also write all the
@@ -433,9 +422,11 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
     TeeSource tapped { *fileSource, narSink };
 
     NullParseSink blank;
-    auto & parseSink = method == FileIngestionMethod::Flat
+    auto & parseSink = method.getFileIngestionMethod() == FileIngestionMethod::Flat
         ? (ParseSink &) fileSink
-        : (ParseSink &) blank;
+        : method.getFileIngestionMethod() == FileIngestionMethod::Recursive
+        ? (ParseSink &) blank
+        : (abort(), (ParseSink &)*(ParseSink *)nullptr); // handled both cases
 
     /* The information that flows from tapped (besides being replicated in
        narSink), is now put in parseSink. */
@@ -452,21 +443,24 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
     if (expectedCAHash && expectedCAHash != hash)
         throw Error("hash mismatch for '%s'", srcPath);
 
+
     ValidPathInfo info {
         *this,
         name,
-        FixedOutputInfo {
-            .method = method,
-            .hash = hash,
-            .references = {},
-        },
+        ContentAddressWithReferences::fromParts(
+            method,
+            hash,
+            {
+                .others = references,
+                .self = false,
+            }),
         narHash,
     };
     info.narSize = narSize;
 
     if (!isValidPath(info.path)) {
         auto source = sinkToSource([&](Sink & scratchpadSink) {
-            dumpPath(srcPath, scratchpadSink);
+            accessor.dumpPath(srcPath, scratchpadSink);
         });
         addToStore(info, *source);
     }

src/libstore/store-api.hh

@@ -427,22 +427,28 @@ public:
      * libutil/archive.hh).
      */
     virtual StorePath addToStore(
-            std::string_view name,
-            const Path & srcPath,
-            FileIngestionMethod method = FileIngestionMethod::Recursive,
-            HashAlgorithm hashAlgo = HashAlgorithm::SHA256,
-            PathFilter & filter = defaultPathFilter,
-            RepairFlag repair = NoRepair,
-            const StorePathSet & references = StorePathSet());
+        std::string_view name,
+        SourceAccessor & accessor,
+        const CanonPath & path,
+        ContentAddressMethod method = FileIngestionMethod::Recursive,
+        HashAlgorithm hashAlgo = HashAlgorithm::SHA256,
+        const StorePathSet & references = StorePathSet(),
+        PathFilter & filter = defaultPathFilter,
+        RepairFlag repair = NoRepair);
 
     /**
      * Copy the contents of a path to the store and register the
      * validity the resulting path, using a constant amount of
      * memory.
      */
-    ValidPathInfo addToStoreSlow(std::string_view name, const Path & srcPath,
-                                 FileIngestionMethod method = FileIngestionMethod::Recursive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256,
-                                 std::optional<Hash> expectedCAHash = {});
+    ValidPathInfo addToStoreSlow(
+        std::string_view name,
+        SourceAccessor & accessor,
+        const CanonPath & path,
+        ContentAddressMethod method = FileIngestionMethod::Recursive,
+        HashAlgorithm hashAlgo = HashAlgorithm::SHA256,
+        const StorePathSet & references = StorePathSet(),
+        std::optional<Hash> expectedCAHash = {});
 
     /**
      * Like addToStore(), but the contents of the path are contained
@@ -453,20 +459,14 @@ public:
      *
      * \todo remove?
      */
-    virtual StorePath addToStoreFromDump(Source & dump, std::string_view name,
-                                         FileIngestionMethod method = FileIngestionMethod::Recursive, HashAlgorithm hashAlgo = HashAlgorithm::SHA256, RepairFlag repair = NoRepair,
-                                         const StorePathSet & references = StorePathSet())
-    { unsupported("addToStoreFromDump"); }
-
-    /**
-     * Like addToStore, but the contents written to the output path is a
-     * regular file containing the given string.
-     */
-    virtual StorePath addTextToStore(
+    virtual StorePath addToStoreFromDump(
+        Source & dump,
         std::string_view name,
-        std::string_view s,
-        const StorePathSet & references,
-        RepairFlag repair = NoRepair) = 0;
+        ContentAddressMethod method = FileIngestionMethod::Recursive,
+        HashAlgorithm hashAlgo = HashAlgorithm::SHA256,
+        const StorePathSet & references = StorePathSet(),
+        RepairFlag repair = NoRepair)
+    { unsupported("addToStoreFromDump"); }
 
     /**
      * Add a mapping indicating that `deriver!outputName` maps to the output path

src/libstore/store-dir-config.hh

@@ -86,41 +86,20 @@ struct StoreDirConfig : public Config
 
     StorePath makeFixedOutputPath(std::string_view name, const FixedOutputInfo & info) const;
 
-    StorePath makeTextPath(std::string_view name, const TextInfo & info) const;
-
     StorePath makeFixedOutputPathFromCA(std::string_view name, const ContentAddressWithReferences & ca) const;
 
     /**
-     * Read-only variant of addToStoreFromDump(). It returns the store
-     * path to which a NAR or flat file would be written.
+     * Read-only variant of addToStore(). It returns the store
+     * path for the given file sytem object.
      */
-    std::pair<StorePath, Hash> computeStorePathFromDump(
-        Source & dump,
+    std::pair<StorePath, Hash> computeStorePath(
         std::string_view name,
-        FileIngestionMethod method = FileIngestionMethod::Recursive,
+        SourceAccessor & accessor,
+        const CanonPath & path,
+        ContentAddressMethod method = FileIngestionMethod::Recursive,
         HashAlgorithm hashAlgo = HashAlgorithm::SHA256,
-        const StorePathSet & references = {}) const;
-
-    /**
-     * Preparatory part of addTextToStore().
-     *
-     * !!! Computation of the path should take the references given to
-     * addTextToStore() into account, otherwise we have a (relatively
-     * minor) security hole: a caller can register a source file with
-     * bogus references.  If there are too many references, the path may
-     * not be garbage collected when it has to be (not really a problem,
-     * the caller could create a root anyway), or it may be garbage
-     * collected when it shouldn't be (more serious).
-     *
-     * Hashing the references would solve this (bogus references would
-     * simply yield a different store path, so other users wouldn't be
-     * affected), but it has some backwards compatibility issues (the
-     * hashing scheme changes), so I'm not doing that for now.
-     */
-    StorePath computeStorePathForText(
-        std::string_view name,
-        std::string_view s,
-        const StorePathSet & references) const;
+        const StorePathSet & references = {},
+        PathFilter & filter = defaultPathFilter) const;
 };
 
 }