Remove now-redundant text-hashing store methods

`addTextToStore` and `computeStorePathFromDump` are now redundant. Co-authored-by: Robert Hensing <[email protected]>
NixOS · Nov 13, 2023 · 26b254b · 26b254b
1 parent 069b0e6
commit 26b254b
Show file tree

Hide file tree

Showing 18 changed files with 93 additions and 214 deletions.
diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs
@@ -12,6 +12,7 @@
 #include "globals.hh"
 #include "store-api.hh"
 #include "crypto.hh"
+#include "posix-source-accessor.hh"
 
 #include <sodium.h>
 #include <nlohmann/json.hpp>
@@ -204,7 +205,10 @@ void importPaths(int fd, int dontCheckSigs)
 SV * hashPath(char * algo, int base32, char * path)
     PPCODE:
         try {
-            Hash h = hashPath(parseHashType(algo), path).first;
+            PosixSourceAccessor accessor;
+            Hash h = hashPath(
+                accessor, CanonPath::fromCwd(path),
+                FileIngestionMethod::Recursive, parseHashType(algo)).first;
             auto s = h.to_string(base32 ? HashFormat::Base32 : HashFormat::Base16, false);
             XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0)));
         } catch (Error & e) {
@@ -280,7 +284,11 @@ SV * addToStore(char * srcPath, int recursive, char * algo)
     PPCODE:
         try {
             auto method = recursive ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat;
-            auto path = store()->addToStore(std::string(baseNameOf(srcPath)), srcPath, method, parseHashType(algo));
+            PosixSourceAccessor accessor;
+            auto path = store()->addToStore(
+                std::string(baseNameOf(srcPath)),
+                accessor, CanonPath::fromCwd(srcPath),
+                method, parseHashType(algo));
             XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(path).c_str(), 0)));
         } catch (Error & e) {
             croak("%s", e.what());

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
@@ -2090,8 +2090,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(htSHA256, contents),
+            .references = std::move(refs),
+        })
+        : ({
+            StringSource s { contents };
+            state.store->addToStoreFromDump(s, name, TextIngestionMethod {}, htSHA256, 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

diff --git a/src/libstore/binary-cache-store.cc b/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 != htSHA256)
-        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(htSHA256, 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 != htSHA256)
+            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(htSHA256, 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
 {

diff --git a/src/libstore/binary-cache-store.hh b/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 &,

diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
@@ -1307,17 +1307,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,

diff --git a/src/libstore/content-address.hh b/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
 {

diff --git a/src/libstore/daemon.cc b/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 {}, htSHA256, refs, NoRepair);
+        });
         logger->stopWork();
         to << store->printStorePath(path);
         break;

diff --git a/src/libstore/derivations.cc b/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(htSHA256, contents),
+            .references = std::move(references),
+        })
+        : ({
+            StringSource s { contents };
+            store.addToStoreFromDump(s, suffix, TextIngestionMethod {}, htSHA256, references, repair);
+        });
 }
 
 

diff --git a/src/libstore/dummy-store.cc b/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"); }
 

diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc
@@ -277,13 +277,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)

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
@@ -1414,58 +1414,6 @@ StorePath LocalStore::addToStoreFromDump(
 }
 
 
-StorePath LocalStore::addTextToStore(
-    std::string_view name,
-    std::string_view s,
-    const StorePathSet & references, RepairFlag repair)
-{
-    auto hash = hashString(htSHA256, 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(htSHA256, 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()

diff --git a/src/libstore/local-store.hh b/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:

diff --git a/src/libstore/remote-store.cc b/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 {}, htSHA256, references, repair)->path;
-}
-
 void RemoteStore::registerDrvOutput(const Realisation & info)
 {
     auto conn(getConnection());

diff --git a/src/libstore/remote-store.hh b/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 &,