WIP: Avoid custom logic copying outputs from the remote builder

We need a replacement for the nar member logic, however. And maybe also
a test that fails until this is fixed (this one should not be passing).

src/hydra-queue-runner/build-remote.cc

@@ -36,71 +36,6 @@ bool ::Machine::isLocalhost() const
 
 namespace nix::build_remote {
 
-static void copyClosureTo(
-    ::Machine::Connection & conn,
-    Store & destStore,
-    const StorePathSet & paths,
-    SubstituteFlag useSubstitutes = NoSubstitute)
-{
-    StorePathSet closure;
-    destStore.computeFSClosure(paths, closure);
-
-    /* Send the "query valid paths" command with the "lock" option
-       enabled. This prevents a race where the remote host
-       garbage-collect paths that are already there. Optionally, ask
-       the remote host to substitute missing paths. */
-    // FIXME: substitute output pollutes our build log
-    /* Get back the set of paths that are already valid on the remote
-       host. */
-    auto present = conn.store->queryValidPaths(
-        closure, true, useSubstitutes);
-
-    if (present.size() == closure.size()) return;
-
-    auto sorted = destStore.topoSortPaths(closure);
-
-    StorePathSet missing;
-    for (auto i = sorted.rbegin(); i != sorted.rend(); ++i)
-        if (!present.count(*i)) missing.insert(*i);
-
-    printMsg(lvlDebug, "sending %d missing paths", missing.size());
-
-    std::unique_lock<std::timed_mutex> sendLock(conn.machine->state->sendLock,
-        std::chrono::seconds(600));
-
-    conn.store->addMultipleToStoreLegacy(destStore, missing);
-}
-
-
-// FIXME: use Store::topoSortPaths().
-static StorePaths reverseTopoSortPaths(const std::map<StorePath, UnkeyedValidPathInfo> & paths)
-{
-    StorePaths sorted;
-    StorePathSet visited;
-
-    std::function<void(const StorePath & path)> dfsVisit;
-
-    dfsVisit = [&](const StorePath & path) {
-        if (!visited.insert(path).second) return;
-
-        auto info = paths.find(path);
-        auto references = info == paths.end() ? StorePathSet() : info->second.references;
-
-        for (auto & i : references)
-            /* Don't traverse into paths that don't exist.  That can
-               happen due to substitutes for non-existent paths. */
-            if (i != path && paths.count(i))
-                dfsVisit(i);
-
-        sorted.push_back(path);
-    };
-
-    for (auto & i : paths)
-        dfsVisit(i.first);
-
-    return sorted;
-}
-
 static std::pair<Path, AutoCloseFD> openLogFile(const std::string & logDir, const StorePath & drvPath)
 {
     std::string base(drvPath.to_string());
@@ -163,13 +98,13 @@ static BasicDerivation sendInputs(
         auto now1 = std::chrono::steady_clock::now();
 
         /* Copy the input closure. */
-        if (conn.machine->isLocalhost()) {
+        copyClosure(
-            StorePathSet closure;
+            destStore,
-            destStore.computeFSClosure(basicDrv.inputSrcs, closure);
+            conn.machine->isLocalhost() ? localStore : *conn.store,
-            copyPaths(destStore, localStore, closure, NoRepair, NoCheckSigs, NoSubstitute);
+            basicDrv.inputSrcs,
-        } else {
+            NoRepair,
-            copyClosureTo(conn, destStore, basicDrv.inputSrcs, Substitute);
+            NoCheckSigs,
-        }
+            Substitute);
 
         auto now2 = std::chrono::steady_clock::now();
 
@@ -241,54 +176,6 @@ static BuildResult performBuild(
     return result;
 }
 
-static void copyPathFromRemote(
-    ::Machine::Connection & conn,
-    NarMemberDatas & narMembers,
-    Store & localStore,
-    Store & destStore,
-    const ValidPathInfo & info
-)
-{
-    /* Receive the NAR from the remote and add it to the
-        destination store. Meanwhile, extract all the info from the
-        NAR that getBuildOutput() needs. */
-    auto source2 = sinkToSource([&](Sink & sink)
-    {
-        /* Note: we should only send the command to dump the store
-            path to the remote if the NAR is actually going to get read
-            by the destination store, which won't happen if this path
-            is already valid on the destination store. Since this
-            lambda function only gets executed if someone tries to read
-            from source2, we will send the command from here rather
-            than outside the lambda. */
-        conn.store->narFromPath(info.path, [&](Source & source) {
-            TeeSource tee{source, sink};
-            extractNarData(tee, conn.store->printStorePath(info.path), narMembers);
-        });
-    });
-
-    destStore.addToStore(info, *source2, NoRepair, NoCheckSigs);
-}
-
-static void copyPathsFromRemote(
-    ::Machine::Connection & conn,
-    NarMemberDatas & narMembers,
-    Store & localStore,
-    Store & destStore,
-    const std::map<StorePath, UnkeyedValidPathInfo> & infos
-)
-{
-      auto pathsSorted = reverseTopoSortPaths(infos);
-
-      for (auto & path : pathsSorted) {
-          auto & info = infos.find(path)->second;
-          copyPathFromRemote(
-              conn, narMembers, localStore, destStore,
-              ValidPathInfo { path, info });
-      }
-
-}
-
 }
 
 /* using namespace nix::build_remote; */
@@ -487,21 +374,12 @@ void State::buildRemote(ref<Store> destStore,
 
             auto now1 = std::chrono::steady_clock::now();
 
-            auto infos = conn.store->queryPathInfosUncached(outputs);
-
-            size_t totalNarSize = 0;
-            for (auto & [_, info] : infos) totalNarSize += info.narSize;
-
-            if (totalNarSize > maxOutputSize) {
-                result.stepStatus = bsNarSizeLimitExceeded;
-                return;
-            }
-
             /* Copy each path. */
-            printMsg(lvlDebug, "copying outputs of ‘%s’ from ‘%s’ (%d bytes)",
+            printMsg(lvlDebug, "copying outputs of ‘%s’ from ‘%s’",
-                localStore->printStorePath(step->drvPath), machine->storeUri.render(), totalNarSize);
+                localStore->printStorePath(step->drvPath), machine->storeUri.render());
+
+            copyClosure(*conn.store, *destStore, outputs);
 
-            build_remote::copyPathsFromRemote(conn, narMembers, *localStore, *destStore, infos);
             auto now2 = std::chrono::steady_clock::now();
 
             result.overhead += std::chrono::duration_cast<std::chrono::milliseconds>(now2 - now1).count();