libstore: remove Store::addMultipleToStore(Source&, ...)

only RemoteStore and the daemon loop ever call this method, and the
remote store call site isn't even particularly well done. we do not
have to create a bitstream, check whether our remote understands it
at all, only to parse it again if the remote doesn't understand it.
we can instead just use data we already have and avoid many copies.

Change-Id: Ib20740df16555269cbb19c7b12937280a00fc1f0

lix/libstore/daemon.cc

@@ -516,9 +516,16 @@ static void performOp(AsyncIoRoot & aio, TunnelLogger * logger, ref<Store> store
         logger->startWork();
         {
             FramedSource source(from);
-            aio.blockOn(store->addMultipleToStore(source,
-                RepairFlag{repair},
-                dontCheckSigs ? NoCheckSigs : CheckSigs));
+            auto expected = readNum<uint64_t>(source);
+            for (uint64_t i = 0; i < expected; ++i) {
+                auto info = WorkerProto::Serialise<ValidPathInfo>::read(
+                    *store, WorkerProto::ReadConn{source, clientVersion}
+                );
+                info.ultimate = false; // duplicated in RemoteStore::addMultipleToStore
+                aio.blockOn(store->addToStore(
+                    info, source, RepairFlag{repair}, dontCheckSigs ? NoCheckSigs : CheckSigs
+                ));
+            }
         }
         logger->stopWork();
         break;

lix/libstore/remote-store.cc

@@ -515,40 +515,29 @@ kj::Promise<Result<void>> RemoteStore::addMultipleToStore(
     Activity & act,
     RepairFlag repair,
     CheckSigsFlag checkSigs)
-try {
-    auto remoteVersion = getProtocol();
-
-    GeneratorSource source{
-        [](auto self, auto & pathsToCopy, auto remoteVersion) -> WireFormatGenerator {
-            co_yield pathsToCopy.size();
-            for (auto & [pathInfo, pathSource] : pathsToCopy) {
-                co_yield WorkerProto::Serialise<ValidPathInfo>::write(*self,
-                    WorkerProto::WriteConn {remoteVersion},
-                    pathInfo);
-                try {
-                    char buf[65536];
-                    while (true) {
-                        const auto read = pathSource->read(buf, sizeof(buf));
-                        co_yield std::span{buf, read};
-                    }
-                } catch (EndOfFile &) {
-                }
-            }
-        }(this, pathsToCopy, remoteVersion)
-    };
-
-    TRY_AWAIT(addMultipleToStore(source, repair, checkSigs));
-    co_return result::success();
-} catch (...) {
-    co_return result::current_exception();
-}
-
-kj::Promise<Result<void>> RemoteStore::addMultipleToStore(
-    Source & source,
-    RepairFlag repair,
-    CheckSigsFlag checkSigs)
 try {
     if (GET_PROTOCOL_MINOR(getConnection()->daemonVersion) >= 32) {
+        auto remoteVersion = getProtocol();
+
+        GeneratorSource source{
+            [](auto self, auto & pathsToCopy, auto remoteVersion) -> WireFormatGenerator {
+                co_yield pathsToCopy.size();
+                for (auto & [pathInfo, pathSource] : pathsToCopy) {
+                    co_yield WorkerProto::Serialise<ValidPathInfo>::write(*self,
+                        WorkerProto::WriteConn {remoteVersion},
+                        pathInfo);
+                    try {
+                        char buf[65536];
+                        while (true) {
+                            const auto read = pathSource->read(buf, sizeof(buf));
+                            co_yield std::span{buf, read};
+                        }
+                    } catch (EndOfFile &) {
+                    }
+                }
+            }(this, pathsToCopy, remoteVersion)
+        };
+
         auto conn(getConnection());
         conn->to
             << WorkerProto::Op::AddMultipleToStore
@@ -557,8 +546,12 @@ try {
         conn.withFramedSink([&](Sink & sink) {
             source.drainInto(sink);
         });
-    } else
-        TRY_AWAIT(Store::addMultipleToStore(source, repair, checkSigs));
+    } else {
+        for (auto & [pathInfo, pathSource] : pathsToCopy) {
+            pathInfo.ultimate = false; // duplicated in daemon.cc AddMultipleToStore
+            TRY_AWAIT(addToStore(pathInfo, *pathSource, repair, checkSigs));
+        }
+    }
     co_return result::success();
 } catch (...) {
     co_return result::current_exception();

lix/libstore/remote-store.hh

@@ -93,11 +93,6 @@ public:
     kj::Promise<Result<void>> addToStore(const ValidPathInfo & info, Source & nar,
         RepairFlag repair, CheckSigsFlag checkSigs) override;
 
-    kj::Promise<Result<void>> addMultipleToStore(
-        Source & source,
-        RepairFlag repair,
-        CheckSigsFlag checkSigs) override;
-
     kj::Promise<Result<void>> addMultipleToStore(
         PathsSource & pathsToCopy,
         Activity & act,

lix/libstore/store-api.cc

@@ -368,27 +368,6 @@ try {
     co_return result::current_exception();
 }
 
-kj::Promise<Result<void>> Store::addMultipleToStore(
-    Source & source,
-    RepairFlag repair,
-    CheckSigsFlag checkSigs)
-try {
-    auto remoteVersion = getProtocol();
-
-    auto expected = readNum<uint64_t>(source);
-    for (uint64_t i = 0; i < expected; ++i) {
-        // FIXME we should not be using the worker protocol here at all!
-        auto info = WorkerProto::Serialise<ValidPathInfo>::read(*this,
-            WorkerProto::ReadConn {source, remoteVersion}
-        );
-        info.ultimate = false;
-        TRY_AWAIT(addToStore(info, source, repair, checkSigs));
-    }
-    co_return result::success();
-} catch (...) {
-    co_return result::current_exception();
-}
-
 namespace {
 /**
  * If the NAR archive contains a single file at top-level, then save

lix/libstore/store-api.hh

@@ -511,11 +511,6 @@ public:
     /**
      * Import multiple paths into the store.
      */
-    virtual kj::Promise<Result<void>> addMultipleToStore(
-        Source & source,
-        RepairFlag repair = NoRepair,
-        CheckSigsFlag checkSigs = CheckSigs);
-
     virtual kj::Promise<Result<void>> addMultipleToStore(
         PathsSource & pathsToCopy,
         Activity & act,