Merge pull request #3639 from obsidiansystems/do-fixme-store-removes

Remove `addToStore` variant as requested by `FIXME`
NixOS · Jun 2, 2020 · bfa1acd · bfa1acd
2 parents c16fdda + fac0c2d
commit bfa1acd
Show file tree

Hide file tree

Showing 7 changed files with 20 additions and 29 deletions.
diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc
@@ -71,7 +71,8 @@ DownloadFileResult downloadFile(
         info.narHash = hashString(htSHA256, *sink.s);
         info.narSize = sink.s->size();
         info.ca = makeFixedOutputCA(FileIngestionMethod::Flat, hash);
-        store->addToStore(info, sink.s, NoRepair, NoCheckSigs);
+        auto source = StringSource { *sink.s };
+        store->addToStore(info, source, NoRepair, NoCheckSigs);
         storePath = std::move(info.path);
     }
 

diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc
@@ -113,9 +113,12 @@ void BinaryCacheStore::writeNarInfo(ref<NarInfo> narInfo)
         diskCache->upsertNarInfo(getUri(), hashPart, std::shared_ptr<NarInfo>(narInfo));
 }
 
-void BinaryCacheStore::addToStore(const ValidPathInfo & info, const ref<std::string> & nar,
+void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource,
     RepairFlag repair, CheckSigsFlag checkSigs, std::shared_ptr<FSAccessor> accessor)
 {
+    // FIXME: See if we can use the original source to reduce memory usage.
+    auto nar = make_ref<std::string>(narSource.drain());
+
     if (!repair && isValidPath(info.path)) return;
 
     /* Verify that all references are valid. This may do some .narinfo
@@ -347,7 +350,8 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath
 
     ValidPathInfo info(makeFixedOutputPath(method, h, name));
 
-    addToStore(info, sink.s, repair, CheckSigs, nullptr);
+    auto source = StringSource { *sink.s };
+    addToStore(info, source, repair, CheckSigs, nullptr);
 
     return std::move(info.path);
 }
@@ -361,7 +365,8 @@ StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s
     if (repair || !isValidPath(info.path)) {
         StringSink sink;
         dumpString(s, sink);
-        addToStore(info, sink.s, repair, CheckSigs, nullptr);
+        auto source = StringSource { *sink.s };
+        addToStore(info, source, repair, CheckSigs, nullptr);
     }
 
     return std::move(info.path);

diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh
@@ -74,7 +74,7 @@ public:
     std::optional<StorePath> queryPathFromHashPart(const std::string & hashPart) override
     { unsupported("queryPathFromHashPart"); }
 
-    void addToStore(const ValidPathInfo & info, const ref<std::string> & nar,
+    void addToStore(const ValidPathInfo & info, Source & narSource,
         RepairFlag repair, CheckSigsFlag checkSigs,
         std::shared_ptr<FSAccessor> accessor) override;
 

diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc
@@ -1,3 +1,4 @@
+#include "serialise.hh"
 #include "store-api.hh"
 #include "archive.hh"
 #include "worker-protocol.hh"
@@ -100,7 +101,9 @@ StorePaths Store::importPaths(Source & source, std::shared_ptr<FSAccessor> acces
         if (readInt(source) == 1)
             readString(source);
 
-        addToStore(info, tee.source.data, NoRepair, checkSigs, accessor);
+        // Can't use underlying source, which would have been exhausted
+        auto source = StringSource { *tee.source.data };
+        addToStore(info, source, NoRepair, checkSigs, accessor);
 
         res.push_back(info.path.clone());
     }

diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
@@ -840,21 +840,6 @@ std::string makeFixedOutputCA(FileIngestionMethod recursive, const Hash & hash)
 }
 
 
-void Store::addToStore(const ValidPathInfo & info, Source & narSource,
-    RepairFlag repair, CheckSigsFlag checkSigs,
-    std::shared_ptr<FSAccessor> accessor)
-{
-    addToStore(info, make_ref<std::string>(narSource.drain()), repair, checkSigs, accessor);
-}
-
-void Store::addToStore(const ValidPathInfo & info, const ref<std::string> & nar,
-    RepairFlag repair, CheckSigsFlag checkSigs,
-    std::shared_ptr<FSAccessor> accessor)
-{
-    StringSource source(*nar);
-    addToStore(info, source, repair, checkSigs, accessor);
-}
-
 }
 
 

diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
@@ -450,12 +450,7 @@ public:
     /* Import a path into the store. */
     virtual void addToStore(const ValidPathInfo & info, Source & narSource,
         RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs,
-        std::shared_ptr<FSAccessor> accessor = 0);
-
-    // FIXME: remove
-    virtual void addToStore(const ValidPathInfo & info, const ref<std::string> & nar,
-        RepairFlag repair = NoRepair, CheckSigsFlag checkSigs = CheckSigs,
-        std::shared_ptr<FSAccessor> accessor = 0);
+        std::shared_ptr<FSAccessor> accessor = 0) = 0;
 
     /* Copy the contents of a path to the store and register the
        validity the resulting path.  The resulting path is returned.

diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc
@@ -50,8 +50,10 @@ struct CmdAddToStore : MixDryRun, StoreCommand
         info.narSize = sink.s->size();
         info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, info.narHash);
 
-        if (!dryRun)
-            store->addToStore(info, sink.s);
+        if (!dryRun) {
+            auto source = StringSource { *sink.s };
+            store->addToStore(info, source);
+        }
 
         logger->stdout("%s", store->printStorePath(info.path));
     }