Merge pull request #1316 from NixOS/ca-derivations-prep

Prepare for CA derivation support with lower impact changes

src/hydra-eval-jobs/hydra-eval-jobs.cc

@@ -178,7 +178,11 @@ static void worker(
 
             if (auto drv = getDerivation(state, *v, false)) {
 
-                DrvInfo::Outputs outputs = drv->queryOutputs();
+                // CA derivations do not have static output paths, so we
+                // have to defensively not query output paths in case we
+                // encounter one.
+                DrvInfo::Outputs outputs = drv->queryOutputs(
+                    !experimentalFeatureSettings.isEnabled(Xp::CaDerivations));
 
                 if (drv->querySystem() == "unknown")
                     throw EvalError("derivation must have a 'system' attribute");
@@ -239,12 +243,21 @@ static void worker(
                 }
 
                 nlohmann::json out;
-                for (auto & j : outputs)
+                for (auto & [outputName, optOutputPath] : outputs) {
-                    // FIXME: handle CA/impure builds.
+                    if (optOutputPath) {
-                    if (j.second)
+                        out[outputName] = state.store->printStorePath(*optOutputPath);
-                        out[j.first] = state.store->printStorePath(*j.second);
+                    } else {
+                        // See the `queryOutputs` call above; we should
+                        // not encounter missing output paths otherwise.
+                        assert(experimentalFeatureSettings.isEnabled(Xp::CaDerivations));
+                        // TODO it would be better to set `null` than an
+                        // empty string here, to force the consumer of
+                        // this JSON to more explicitly handle this
+                        // case.
+                        out[outputName] = "";
+                    }
+                }
                 job["outputs"] = std::move(out);
-
                 reply["job"] = std::move(job);
             }
 

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

@@ -222,17 +222,22 @@ static BasicDerivation sendInputs(
     counter & nrStepsCopyingTo
 )
 {
-    BasicDerivation basicDrv(*step.drv);
+    /* Replace the input derivations by their output paths to send a
+       minimal closure to the builder.
 
-    for (const auto & [drvPath, node] : step.drv->inputDrvs.map) {
+       `tryResolve` currently does *not* rewrite input addresses, so it
-        auto drv2 = localStore.readDerivation(drvPath);
+       is safe to do this in all cases. (It should probably have a mode
-        for (auto & name : node.value) {
+       to do that, however, but we would not use it here.)
-            if (auto i = get(drv2.outputs, name)) {
+     */
-                auto outPath = i->path(localStore, drv2.name, name);
+    BasicDerivation basicDrv = ({
-                basicDrv.inputSrcs.insert(*outPath);
+        auto maybeBasicDrv = step.drv->tryResolve(destStore, &localStore);
-            }
+        if (!maybeBasicDrv)
-        }
+            throw Error(
-    }
+                "the derivation '%s' can’t be resolved. It’s probably "
+                "missing some outputs",
+                localStore.printStorePath(step.drvPath));
+        *maybeBasicDrv;
+    });
 
     /* Ensure that the inputs exist in the destination store. This is
        a no-op for regular stores, but for the binary cache store,
@@ -313,6 +318,30 @@ static BuildResult performBuild(
         result.stopTime = stopTime;
     }
 
+    // If the protocol was too old to give us `builtOutputs`, initialize
+    // it manually by introspecting the derivation.
+    if (GET_PROTOCOL_MINOR(conn.remoteVersion) < 6)
+    {
+        // If the remote is too old to handle CA derivations, we can’t get this
+        // far anyways
+        assert(drv.type().hasKnownOutputPaths());
+        DerivationOutputsAndOptPaths drvOutputs = drv.outputsAndOptPaths(localStore);
+        // Since this a `BasicDerivation`, `staticOutputHashes` will not
+        // do any real work.
+        auto outputHashes = staticOutputHashes(localStore, drv);
+        for (auto & [outputName, output] : drvOutputs) {
+            auto outputPath = output.second;
+            // We’ve just asserted that the output paths of the derivation
+            // were known
+            assert(outputPath);
+            auto outputHash = outputHashes.at(outputName);
+            auto drvOutput = DrvOutput { outputHash, outputName };
+            result.builtOutputs.insert_or_assign(
+                std::move(outputName),
+                Realisation { drvOutput, *outputPath });
+        }
+    }
+
     return result;
 }
 
@@ -578,6 +607,10 @@ void State::buildRemote(ref<Store> destStore,
             result.logFile = "";
         }
 
+        StorePathSet outputs;
+        for (auto & [_, realisation] : buildResult.builtOutputs)
+            outputs.insert(realisation.outPath);
+
         /* Copy the output paths. */
         if (!machine->isLocalhost() || localStore != std::shared_ptr<Store>(destStore)) {
             updateStep(ssReceivingOutputs);
@@ -586,12 +619,6 @@ void State::buildRemote(ref<Store> destStore,
 
             auto now1 = std::chrono::steady_clock::now();
 
-            StorePathSet outputs;
-            for (auto & i : step->drv->outputsAndOptPaths(*localStore)) {
-                if (i.second.second)
-                   outputs.insert(*i.second.second);
-            }
-
             size_t totalNarSize = 0;
             auto infos = build_remote::queryPathInfos(conn, *localStore, outputs, totalNarSize);
 
@@ -610,6 +637,21 @@ void State::buildRemote(ref<Store> destStore,
             result.overhead += std::chrono::duration_cast<std::chrono::milliseconds>(now2 - now1).count();
         }
 
+        /* Register the outputs of the newly built drv */
+        if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
+            auto outputHashes = staticOutputHashes(*localStore, *step->drv);
+            for (auto & [outputName, realisation] : buildResult.builtOutputs) {
+                // Register the resolved drv output
+                destStore->registerDrvOutput(realisation);
+
+                // Also register the unresolved one
+                auto unresolvedRealisation = realisation;
+                unresolvedRealisation.signatures.clear();
+                unresolvedRealisation.id.drvHash = outputHashes.at(outputName);
+                destStore->registerDrvOutput(unresolvedRealisation);
+            }
+        }
+
         /* Shut down the connection. */
         child.in = -1;
         child.sshPid.wait();

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

@@ -11,18 +11,18 @@ using namespace nix;
 BuildOutput getBuildOutput(
     nix::ref<Store> store,
     NarMemberDatas & narMembers,
-    const Derivation & drv)
+    const OutputPathMap derivationOutputs)
 {
     BuildOutput res;
 
     /* Compute the closure size. */
     StorePathSet outputs;
     StorePathSet closure;
-    for (auto & i : drv.outputsAndOptPaths(*store))
+    for (auto& [outputName, outputPath] : derivationOutputs) {
-        if (i.second.second) {
+        store->computeFSClosure(outputPath, closure);
-            store->computeFSClosure(*i.second.second, closure);
+        outputs.insert(outputPath);
-            outputs.insert(*i.second.second);
+        res.outputs.insert({outputName, outputPath});
-        }
+    }
     for (auto & path : closure) {
         auto info = store->queryPathInfo(path);
         res.closureSize += info->narSize;
@@ -107,13 +107,12 @@ BuildOutput getBuildOutput(
     /* If no build products were explicitly declared, then add all
        outputs as a product of type "nix-build". */
     if (!explicitProducts) {
-        for (auto & [name, output] : drv.outputs) {
+        for (auto & [name, output] : derivationOutputs) {
             BuildProduct product;
-            auto outPath = output.path(*store, drv.name, name);
+            product.path = store->printStorePath(output);
-            product.path = store->printStorePath(*outPath);
             product.type = "nix-build";
             product.subtype = name == "out" ? "" : name;
-            product.name = outPath->name();
+            product.name = output.name();
 
             auto file = narMembers.find(product.path);
             assert(file != narMembers.end());

src/hydra-queue-runner/builder.cc

@@ -223,7 +223,7 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore,
 
         if (result.stepStatus == bsSuccess) {
             updateStep(ssPostProcessing);
-            res = getBuildOutput(destStore, narMembers, *step->drv);
+            res = getBuildOutput(destStore, narMembers, destStore->queryDerivationOutputMap(step->drvPath, &*localStore));
         }
     }
 
@@ -277,9 +277,12 @@ State::StepResult State::doBuildStep(nix::ref<Store> destStore,
 
         assert(stepNr);
 
-        for (auto & i : step->drv->outputsAndOptPaths(*localStore)) {
+        for (auto & [outputName, optOutputPath] : destStore->queryPartialDerivationOutputMap(step->drvPath, &*localStore)) {
-            if (i.second.second)
+            if (!optOutputPath)
-               addRoot(*i.second.second);
+                throw Error(
+                    "Missing output %s for derivation %d which was supposed to have succeeded",
+                    outputName, localStore->printStorePath(step->drvPath));
+            addRoot(*optOutputPath);
         }
 
         /* Register success in the database for all Build objects that

src/hydra-queue-runner/hydra-build-result.hh

@@ -36,10 +36,12 @@ struct BuildOutput
 
     std::list<BuildProduct> products;
 
+    std::map<std::string, nix::StorePath> outputs;
+
     std::map<std::string, BuildMetric> metrics;
 };
 
 BuildOutput getBuildOutput(
     nix::ref<nix::Store> store,
     NarMemberDatas & narMembers,
-    const nix::Derivation & drv);
+    const nix::OutputPathMap derivationOutputs);

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

@@ -333,10 +333,10 @@ unsigned int State::createBuildStep(pqxx::work & txn, time_t startTime, BuildID
 
     if (r.affected_rows() == 0) goto restart;
 
-    for (auto & [name, output] : step->drv->outputs)
+    for (auto & [name, output] : getDestStore()->queryPartialDerivationOutputMap(step->drvPath, &*localStore))
         txn.exec_params0
             ("insert into BuildStepOutputs (build, stepnr, name, path) values ($1, $2, $3, $4)",
-            buildId, stepNr, name, localStore->printStorePath(*output.path(*localStore, step->drv->name, name)));
+            buildId, stepNr, name, output ? localStore->printStorePath(*output) : "");
 
     if (status == bsBusy)
         txn.exec(fmt("notify step_started, '%d\t%d'", buildId, stepNr));
@@ -373,11 +373,23 @@ void State::finishBuildStep(pqxx::work & txn, const RemoteResult & result,
     assert(result.logFile.find('\t') == std::string::npos);
     txn.exec(fmt("notify step_finished, '%d\t%d\t%s'",
             buildId, stepNr, result.logFile));
+
+    if (result.stepStatus == bsSuccess) {
+        // Update the corresponding `BuildStepOutputs` row to add the output path
+        auto res = txn.exec_params1("select drvPath from BuildSteps where build = $1 and stepnr = $2", buildId, stepNr);
+        assert(res.size());
+        StorePath drvPath = localStore->parseStorePath(res[0].as<std::string>());
+        // If we've finished building, all the paths should be known
+        for (auto & [name, output] : getDestStore()->queryDerivationOutputMap(drvPath, &*localStore))
+            txn.exec_params0
+                ("update BuildStepOutputs set path = $4 where build = $1 and stepnr = $2 and name = $3",
+                  buildId, stepNr, name, localStore->printStorePath(output));
+    }
 }
 
 
 int State::createSubstitutionStep(pqxx::work & txn, time_t startTime, time_t stopTime,
-    Build::ptr build, const StorePath & drvPath, const std::string & outputName, const StorePath & storePath)
+    Build::ptr build, const StorePath & drvPath, const nix::Derivation drv, const std::string & outputName, const StorePath & storePath)
 {
  restart:
     auto stepNr = allocBuildStep(txn, build->id);
@@ -478,6 +490,15 @@ void State::markSucceededBuild(pqxx::work & txn, Build::ptr build,
          res.releaseName != "" ? std::make_optional(res.releaseName) : std::nullopt,
          isCachedBuild ? 1 : 0);
 
+    for (auto & [outputName, outputPath] : res.outputs) {
+        txn.exec_params0
+            ("update BuildOutputs set path = $3 where build = $1 and name = $2",
+             build->id,
+             outputName,
+             localStore->printStorePath(outputPath)
+            );
+    }
+
     txn.exec_params0("delete from BuildProducts where build = $1", build->id);
 
     unsigned int productNr = 1;

src/hydra-queue-runner/queue-monitor.cc

@@ -192,15 +192,15 @@ bool State::getQueuedBuilds(Connection & conn,
                 if (!res[0].is_null()) propagatedFrom = res[0].as<BuildID>();
 
                 if (!propagatedFrom) {
-                    for (auto & i : ex.step->drv->outputsAndOptPaths(*localStore)) {
+                    for (auto & [outputName, optOutputPath] : destStore->queryPartialDerivationOutputMap(ex.step->drvPath, &*localStore)) {
-                        if (i.second.second) {
+                          // ca-derivations not actually supported yet
-                            auto res = txn.exec_params
+                          assert(optOutputPath);
-                                ("select max(s.build) from BuildSteps s join BuildStepOutputs o on s.build = o.build where path = $1 and startTime != 0 and stopTime != 0 and status = 1",
+                          auto res = txn.exec_params
-                                 localStore->printStorePath(*i.second.second));
+                              ("select max(s.build) from BuildSteps s join BuildStepOutputs o on s.build = o.build where path = $1 and startTime != 0 and stopTime != 0 and status = 1",
-                            if (!res[0][0].is_null()) {
+                               localStore->printStorePath(*optOutputPath));
-                                propagatedFrom = res[0][0].as<BuildID>();
+                          if (!res[0][0].is_null()) {
-                                break;
+                              propagatedFrom = res[0][0].as<BuildID>();
-                            }
+                              break;
                         }
                     }
                 }
@@ -236,12 +236,10 @@ bool State::getQueuedBuilds(Connection & conn,
         /* If we didn't get a step, it means the step's outputs are
            all valid. So we mark this as a finished, cached build. */
         if (!step) {
-            auto drv = localStore->readDerivation(build->drvPath);
+            BuildOutput res = getBuildOutputCached(conn, destStore, build->drvPath);
-            BuildOutput res = getBuildOutputCached(conn, destStore, drv);
 
-            for (auto & i : drv.outputsAndOptPaths(*localStore))
+            for (auto & i : destStore->queryDerivationOutputMap(build->drvPath, &*localStore))
-                if (i.second.second)
+                addRoot(i.second);
-                    addRoot(*i.second.second);
 
             {
             auto mc = startDbUpdate();
@@ -481,23 +479,36 @@ Step::ptr State::createStep(ref<Store> destStore,
         throw PreviousFailure{step};
 
     /* Are all outputs valid? */
+    auto outputHashes = staticOutputHashes(*localStore, *(step->drv));
     bool valid = true;
-    DerivationOutputs missing;
+    std::map<DrvOutput, std::optional<StorePath>> missing;
-    for (auto & i : step->drv->outputs)
+    for (auto & [outputName, maybeOutputPath] : destStore->queryPartialDerivationOutputMap(drvPath, &*localStore)) {
-        if (!destStore->isValidPath(*i.second.path(*localStore, step->drv->name, i.first))) {
+        auto outputHash = outputHashes.at(outputName);
-            valid = false;
+        if (maybeOutputPath && destStore->isValidPath(*maybeOutputPath))
-            missing.insert_or_assign(i.first, i.second);
+            continue;
-        }
+        valid = false;
+        missing.insert({{outputHash, outputName}, maybeOutputPath});
+    }
 
     /* Try to copy the missing paths from the local store or from
        substitutes. */
     if (!missing.empty()) {
 
         size_t avail = 0;
-        for (auto & i : missing) {
+        for (auto & [i, pathOpt] : missing) {
-            auto pathOpt = i.second.path(*localStore, step->drv->name, i.first);
+            // If we don't know the output path from the destination
-            assert(pathOpt); // CA derivations not yet supported
+            // store, see if the local store can tell us.
+            if (/* localStore != destStore && */ !pathOpt && experimentalFeatureSettings.isEnabled(Xp::CaDerivations))
+                if (auto maybeRealisation = localStore->queryRealisation(i))
+                    pathOpt = maybeRealisation->outPath;
+
+            if (!pathOpt) {
+                // No hope of getting the store object if we don't know
+                // the path.
+                continue;
+            }
             auto & path = *pathOpt;
+
             if (/* localStore != destStore && */ localStore->isValidPath(path))
                 avail++;
             else if (useSubstitutes) {
@@ -510,9 +521,10 @@ Step::ptr State::createStep(ref<Store> destStore,
 
         if (missing.size() == avail) {
             valid = true;
-            for (auto & i : missing) {
+            for (auto & [i, pathOpt] : missing) {
-                auto pathOpt = i.second.path(*localStore, step->drv->name, i.first);
+                // If we found everything, then we should know the path
-                assert(pathOpt); // CA derivations not yet supported
+                // to every missing store object now.
+                assert(pathOpt);
                 auto & path = *pathOpt;
 
                 try {
@@ -539,7 +551,7 @@ Step::ptr State::createStep(ref<Store> destStore,
                     {
                         auto mc = startDbUpdate();
                         pqxx::work txn(conn);
-                        createSubstitutionStep(txn, startTime, stopTime, build, drvPath, "out", path);
+                        createSubstitutionStep(txn, startTime, stopTime, build, drvPath, *(step->drv), "out", path);
                         txn.commit();
                     }
 
@@ -644,17 +656,19 @@ void State::processJobsetSharesChange(Connection & conn)
 }
 
 
-BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref<nix::Store> destStore, const nix::Derivation & drv)
+BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref<nix::Store> destStore, const nix::StorePath & drvPath)
 {
+    auto derivationOutputs = destStore->queryDerivationOutputMap(drvPath, &*localStore);
+
     {
     pqxx::work txn(conn);
 
-    for (auto & [name, output] : drv.outputsAndOptPaths(*localStore)) {
+    for (auto & [name, output] : derivationOutputs) {
         auto r = txn.exec_params
             ("select id, buildStatus, releaseName, closureSize, size from Builds b "
              "join BuildOutputs o on b.id = o.build "
              "where finished = 1 and (buildStatus = 0 or buildStatus = 6) and path = $1",
-             localStore->printStorePath(*output.second));
+             localStore->printStorePath(output));
         if (r.empty()) continue;
         BuildID id = r[0][0].as<BuildID>();
 
@@ -708,5 +722,5 @@ BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref<nix::Store>
     }
 
     NarMemberDatas narMembers;
-    return getBuildOutput(destStore, narMembers, drv);
+    return getBuildOutput(destStore, narMembers, derivationOutputs);
 }

src/hydra-queue-runner/state.hh

@@ -525,7 +525,7 @@ private:
         const std::string & machine);
 
     int createSubstitutionStep(pqxx::work & txn, time_t startTime, time_t stopTime,
-        Build::ptr build, const nix::StorePath & drvPath, const std::string & outputName, const nix::StorePath & storePath);
+        Build::ptr build, const nix::StorePath & drvPath, const nix::Derivation drv, const std::string & outputName, const nix::StorePath & storePath);
 
     void updateBuild(pqxx::work & txn, Build::ptr build, BuildStatus status);
 
@@ -541,7 +541,7 @@ private:
     void processQueueChange(Connection & conn);
 
     BuildOutput getBuildOutputCached(Connection & conn, nix::ref<nix::Store> destStore,
-        const nix::Derivation & drv);
+        const nix::StorePath & drvPath);
 
     Step::ptr createStep(nix::ref<nix::Store> store,
         Connection & conn, Build::ptr build, const nix::StorePath & drvPath,