Remove now-redundant text-hashing store methods

`addTextToStore` and `computeStorePathFromDump` are now redundant.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>

src/libexpr/primops.cc

@@ -2072,8 +2072,14 @@ static void prim_toFile(EvalState & state, const PosIdx pos, Value * * args, Val
     }
 
     auto storePath = settings.readOnlyMode
-        ? state.store->computeStorePathForText(name, contents, refs)
-        : state.store->addTextToStore(name, contents, refs, state.repair);
+        ? state.store->makeFixedOutputPathFromCA(name, TextInfo {
+            .hash = hashString(HashAlgorithm::SHA256, contents),
+            .references = std::move(refs),
+        })
+        : ({
+            StringSource s { contents };
+            state.store->addToStoreFromDump(s, name, TextIngestionMethod {}, HashAlgorithm::SHA256, refs, state.repair);
+        });
 
     /* Note: we don't need to add `context' to the context of the
        result, since `storePath' itself has references to the paths

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>
@@ -308,15 +309,47 @@ StorePath BinaryCacheStore::addToStoreFromDump(
     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,
             ContentAddressWithReferences::fromParts(
                 method,
-                nar.first,
+                caHash ? *caHash : nar.first,
                 {
                     .others = references,
                     // caller is not capable of creating a self-reference, because this is content-addressed without modulus
@@ -440,36 +473,6 @@ StorePath BinaryCacheStore::addToStore(
     })->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;
-        return info;
-    })->path;
-}
-
 void BinaryCacheStore::queryRealisationUncached(const DrvOutput & id,
     Callback<std::shared_ptr<const Realisation>> callback) noexcept
 {

src/libstore/binary-cache-store.hh

@@ -141,12 +141,6 @@ public:
         PathFilter & filter,
         RepairFlag repair) 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/build/local-derivation-goal.cc

@@ -1308,17 +1308,6 @@ 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,

src/libstore/content-address.hh

@@ -109,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
 {

src/libstore/daemon.cc

@@ -483,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

@@ -69,13 +69,6 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
         RepairFlag repair) override
     { unsupported("addToStore"); }
 
-    StorePath addTextToStore(
-        std::string_view name,
-        std::string_view s,
-        const StorePathSet & references,
-        RepairFlag repair) override
-    { unsupported("addTextToStore"); }
-
 private:
 
     void putBuildSettings(Connection & conn);

src/libstore/local-store.cc

@@ -1257,58 +1257,6 @@ StorePath LocalStore::addToStoreFromDump(
 }
 
 
-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()

src/libstore/local-store.hh

@@ -185,12 +185,6 @@ public:
         const StorePathSet & references,
         RepairFlag repair) override;
 
-    StorePath addTextToStore(
-        std::string_view name,
-        std::string_view s,
-        const StorePathSet & references,
-        RepairFlag repair) override;
-
     void addTempRoot(const StorePath & path) override;
 
 private:

src/libstore/remote-store.cc

@@ -608,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

@@ -106,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);
@@ -257,18 +251,6 @@ std::pair<StorePath, Hash> StoreDirConfig::computeStorePath(
 }
 
 
-StorePath StoreDirConfig::computeStorePathForText(
-    std::string_view name,
-    std::string_view s,
-    const StorePathSet & references) const
-{
-    return makeTextPath(name, TextInfo {
-        .hash = hashString(HashAlgorithm::SHA256, s),
-        .references = references,
-    });
-}
-
-
 StorePath Store::addToStore(
     std::string_view name,
     SourceAccessor & accessor,

src/libstore/store-api.hh

@@ -468,16 +468,6 @@ public:
         RepairFlag repair = NoRepair)
     { unsupported("addToStoreFromDump"); }
 
-    /**
-     * Like addToStore, but the contents written to the output path is a
-     * regular file containing the given string.
-     */
-    virtual StorePath addTextToStore(
-        std::string_view name,
-        std::string_view s,
-        const StorePathSet & references,
-        RepairFlag repair = NoRepair) = 0;
-
     /**
      * Add a mapping indicating that `deriver!outputName` maps to the output path
      * `output`.

src/libstore/store-dir-config.hh

@@ -86,8 +86,6 @@ 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;
 
     /**
@@ -102,27 +100,6 @@ struct StoreDirConfig : public Config
         HashAlgorithm hashAlgo = HashAlgorithm::SHA256,
         const StorePathSet & references = {},
         PathFilter & filter = defaultPathFilter) 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;
 };
 
 }

src/nix-env/user-env.cc

@@ -104,10 +104,15 @@ bool createUserEnv(EvalState & state, DrvInfos & elems,
     /* Also write a copy of the list of user environment elements to
        the store; we need it for future modifications of the
        environment. */
-    std::ostringstream str;
-    manifest.print(state.symbols, str, true);
-    auto manifestFile = state.store->addTextToStore("env-manifest.nix",
-        str.str(), references);
+    auto manifestFile = ({
+        std::ostringstream str;
+        manifest.print(state.symbols, str, true);
+        // TODO with C++20 we can use str.view() instead and avoid copy.
+        std::string str2 = str.str();
+        StringSource source { str2 };
+        state.store->addToStoreFromDump(
+            source, "env-manifest.nix", TextIngestionMethod {}, HashAlgorithm::SHA256, references);
+    });
 
     /* Get the environment builder expression. */
     Value envBuilder;

src/nix/develop.cc

@@ -223,7 +223,11 @@ static StorePath getDerivationEnvironment(ref<Store> store, ref<Store> evalStore
     if (builder != "bash")
         throw Error("'nix develop' only works on derivations that use 'bash' as their builder");
 
-    auto getEnvShPath = evalStore->addTextToStore("get-env.sh", getEnvSh, {});
+    auto getEnvShPath = ({
+        StringSource source { getEnvSh };
+        evalStore->addToStoreFromDump(
+            source, "get-env.sh", TextIngestionMethod {}, HashAlgorithm::SHA256, {});
+    });
 
     drv.args = {store->printStorePath(getEnvShPath)};