From ebfefb9161046c83bbbebd26f4cb37ac69628b40 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 11 Dec 2023 12:46:36 -0500 Subject: [PATCH] Sync up with some changes done to the main CA branch --- flake.lock | 8 +- flake.nix | 2 +- src/hydra-queue-runner/build-remote.cc | 55 +++------ src/hydra-queue-runner/builder.cc | 11 +- src/hydra-queue-runner/hydra-queue-runner.cc | 4 +- src/hydra-queue-runner/queue-monitor.cc | 119 ++++++++++--------- 6 files changed, 94 insertions(+), 105 deletions(-) diff --git a/flake.lock b/flake.lock index 2871e70a..e7f9224c 100644 --- a/flake.lock +++ b/flake.lock @@ -42,16 +42,16 @@ "nixpkgs-regression": "nixpkgs-regression" }, "locked": { - "lastModified": 1701122567, - "narHash": "sha256-iA8DqS+W2fWTfR+nNJSvMHqQ+4NpYMRT3b+2zS6JTvE=", + "lastModified": 1702314838, + "narHash": "sha256-calxK+fZ4/tZy1fbph8qyx4ePUAf4ZdvIugpzWeFIGE=", "owner": "NixOS", "repo": "nix", - "rev": "50f8f1c8bc019a4c0fd098b9ac674b94cfc6af0d", + "rev": "ae451e2247b18be6bd36b9d85e41b632e774f40b", "type": "github" }, "original": { "owner": "NixOS", - "ref": "2.19.2", + "ref": "2.19-maintenance", "repo": "nix", "type": "github" } diff --git a/flake.nix b/flake.nix index 5a5aef55..8280e076 100644 --- a/flake.nix +++ b/flake.nix @@ -2,7 +2,7 @@ description = "A Nix-based continuous build system"; inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-23.05"; - inputs.nix.url = "github:NixOS/nix/2.19.2"; + inputs.nix.url = "github:NixOS/nix/2.19-maintenance"; inputs.nix.inputs.nixpkgs.follows = "nixpkgs"; outputs = { self, nixpkgs, nix }: diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 377bdb56..65f9f3fb 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -182,40 +182,6 @@ static StorePaths reverseTopoSortPaths(const std::map return sorted; } -/** - * Replace the input derivations by their output paths to send a minimal closure - * to the builder. - * - * If we can afford it, resolve it, so that the newly generated derivation still - * has some sensible output paths. - */ -BasicDerivation inlineInputDerivations(Store & store, Derivation & drv, const StorePath & drvPath) -{ - BasicDerivation ret; - if (!drv.type().hasKnownOutputPaths()) { - auto maybeBasicDrv = drv.tryResolve(store); - if (!maybeBasicDrv) - throw Error( - "the derivation '%s' can’t be resolved. It’s probably " - "missing some outputs", - store.printStorePath(drvPath)); - ret = *maybeBasicDrv; - } else { - // If the derivation is a real `InputAddressed` derivation, we must - // resolve it manually to keep the original output paths - ret = BasicDerivation(drv); - for (auto & [drvPath, node] : drv.inputDrvs.map) { - auto drv2 = store.readDerivation(drvPath); - auto drv2Outputs = drv2.outputsAndOptPaths(store); - for (auto & name : node.value) { - auto inputPath = drv2Outputs.at(name); - ret.inputSrcs.insert(*inputPath.second); - } - } - } - return ret; -} - static std::pair openLogFile(const std::string & logDir, const StorePath & drvPath) { std::string base(drvPath.to_string()); @@ -262,7 +228,22 @@ static BasicDerivation sendInputs( counter & nrStepsCopyingTo ) { - BasicDerivation basicDrv = inlineInputDerivations(localStore, *step.drv, step.drvPath); + /* Replace the input derivations by their output paths to send a + minimal closure to the builder. + + `tryResolve` currently does *not* rewrite input addresses, so it + is safe to do this in all cases. (It should probably have a mode + to do that, however, but we would not use it here.) + */ + BasicDerivation basicDrv = ({ + 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, @@ -351,6 +332,8 @@ static BuildResult performBuild( // 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; @@ -665,14 +648,12 @@ void State::buildRemote(ref destStore, auto outputHashes = staticOutputHashes(*localStore, *step->drv); for (auto & [outputName, realisation] : buildResult.builtOutputs) { // Register the resolved drv output - localStore->registerDrvOutput(realisation); destStore->registerDrvOutput(realisation); // Also register the unresolved one auto unresolvedRealisation = realisation; unresolvedRealisation.signatures.clear(); unresolvedRealisation.id.drvHash = outputHashes.at(outputName); - localStore->registerDrvOutput(unresolvedRealisation); destStore->registerDrvOutput(unresolvedRealisation); } } diff --git a/src/hydra-queue-runner/builder.cc b/src/hydra-queue-runner/builder.cc index 42983941..b1b58c05 100644 --- a/src/hydra-queue-runner/builder.cc +++ b/src/hydra-queue-runner/builder.cc @@ -223,7 +223,7 @@ State::StepResult State::doBuildStep(nix::ref destStore, if (result.stepStatus == bsSuccess) { updateStep(ssPostProcessing); - res = getBuildOutput(destStore, narMembers, localStore->queryDerivationOutputMap(step->drvPath)); + res = getBuildOutput(destStore, narMembers, destStore->queryDerivationOutputMap(step->drvPath, &*localStore)); } } @@ -277,9 +277,12 @@ State::StepResult State::doBuildStep(nix::ref destStore, assert(stepNr); - for (auto & i : localStore->queryPartialDerivationOutputMap(step->drvPath)) { - if (i.second) - addRoot(*i.second); + for (auto & [outputName, optOutputPath] : destStore->queryPartialDerivationOutputMap(step->drvPath, &*localStore)) { + if (!optOutputPath) + 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 diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 2f2c6091..d514ecfa 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -312,7 +312,7 @@ unsigned int State::createBuildStep(pqxx::work & txn, time_t startTime, BuildID if (r.affected_rows() == 0) goto restart; - for (auto & [name, output] : localStore->queryPartialDerivationOutputMap(step->drvPath)) + 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, output ? localStore->printStorePath(*output) : ""); @@ -359,7 +359,7 @@ void State::finishBuildStep(pqxx::work & txn, const RemoteResult & result, assert(res.size()); StorePath drvPath = localStore->parseStorePath(res[0].as()); // If we've finished building, all the paths should be known - for (auto & [name, output] : localStore->queryDerivationOutputMap(drvPath)) + 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)); diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 04917932..a4d9b4dc 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -192,11 +192,11 @@ bool State::getQueuedBuilds(Connection & conn, if (!res[0].is_null()) propagatedFrom = res[0].as(); if (!propagatedFrom) { - for (auto & i : localStore->queryPartialDerivationOutputMap(ex.step->drvPath)) { + for (auto & [outputName, _] : destStore->queryPartialDerivationOutputMap(ex.step->drvPath, &*localStore)) { auto res = txn.exec_params ("select max(s.build) from BuildSteps s join BuildStepOutputs o on s.build = o.build where drvPath = $1 and name = $2 and startTime != 0 and stopTime != 0 and status = 1", localStore->printStorePath(ex.step->drvPath), - i.first); + outputName); if (!res[0][0].is_null()) { propagatedFrom = res[0][0].as(); break; @@ -237,7 +237,7 @@ bool State::getQueuedBuilds(Connection & conn, if (!step) { BuildOutput res = getBuildOutputCached(conn, destStore, build->drvPath); - for (auto & i : localStore->queryDerivationOutputMap(build->drvPath)) + for (auto & i : destStore->queryDerivationOutputMap(build->drvPath, &*localStore)) addRoot(i.second); { @@ -481,20 +481,12 @@ Step::ptr State::createStep(ref destStore, auto outputHashes = staticOutputHashes(*localStore, *(step->drv)); bool valid = true; std::map> missing; - for (auto &[outputName, maybeOutputPath] : step->drv->outputsAndOptPaths(*destStore)) { + for (auto & [outputName, maybeOutputPath] : destStore->queryPartialDerivationOutputMap(drvPath, &*localStore)) { auto outputHash = outputHashes.at(outputName); - if (maybeOutputPath.second) { - if (!destStore->isValidPath(*maybeOutputPath.second)) { - valid = false; - missing.insert({{outputHash, outputName}, maybeOutputPath.second}); - } - } else { - experimentalFeatureSettings.require(Xp::CaDerivations); - if (!destStore->queryRealisation(DrvOutput{outputHash, outputName})) { - valid = false; - missing.insert({{outputHash, outputName}, std::nullopt}); - } - } + if (maybeOutputPath && destStore->isValidPath(*maybeOutputPath)) + continue; + valid = false; + missing.insert({{outputHash, outputName}, maybeOutputPath}); } /* Try to copy the missing paths from the local store or from @@ -503,14 +495,24 @@ Step::ptr State::createStep(ref destStore, size_t avail = 0; for (auto & [i, maybePath] : missing) { - if ((maybePath && localStore->isValidPath(*maybePath))) + // If we don't know the output path from the destination + // store, see if the local store can tell us. + if (/* localStore != destStore && */ !maybePath && experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) + if (auto maybeRealisation = localStore->queryRealisation(i)) + maybePath = maybeRealisation->outPath; + + if (!maybePath) { + // No hope of getting the store object if we don't know + // the path. + continue; + } + auto & path = *maybePath; + + if (/* localStore != destStore && */ localStore->isValidPath(path)) avail++; - else if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && localStore->queryRealisation(i)) { - maybePath = localStore->queryRealisation(i)->outPath; - avail++; - } else if (useSubstitutes && maybePath) { + else if (useSubstitutes) { SubstitutablePathInfos infos; - localStore->querySubstitutablePathInfos({{*maybePath, {}}}, infos); + localStore->querySubstitutablePathInfos({{path, {}}}, infos); if (infos.size() == 1) avail++; } @@ -518,44 +520,47 @@ Step::ptr State::createStep(ref destStore, if (missing.size() == avail) { valid = true; - for (auto & [i, path] : missing) { - if (path) { - try { - time_t startTime = time(0); + for (auto & [i, maybePath] : missing) { + // If we found everything, then we should know the path + // to every missing store object now. + assert(maybePath); + auto & path = *maybePath; - if (localStore->isValidPath(*path)) - printInfo("copying output ‘%1%’ of ‘%2%’ from local store", - localStore->printStorePath(*path), - localStore->printStorePath(drvPath)); - else { - printInfo("substituting output ‘%1%’ of ‘%2%’", - localStore->printStorePath(*path), - localStore->printStorePath(drvPath)); - localStore->ensurePath(*path); - // FIXME: should copy directly from substituter to destStore. - } + try { + time_t startTime = time(0); - copyClosure(*localStore, *destStore, - StorePathSet { *path }, - NoRepair, CheckSigs, NoSubstitute); - - time_t stopTime = time(0); - - { - auto mc = startDbUpdate(); - pqxx::work txn(conn); - createSubstitutionStep(txn, startTime, stopTime, build, drvPath, *(step->drv), "out", *path); - txn.commit(); - } - - } catch (Error & e) { - printError("while copying/substituting output ‘%s’ of ‘%s’: %s", - localStore->printStorePath(*path), - localStore->printStorePath(drvPath), - e.what()); - valid = false; - break; + if (localStore->isValidPath(path)) + printInfo("copying output ‘%1%’ of ‘%2%’ from local store", + localStore->printStorePath(path), + localStore->printStorePath(drvPath)); + else { + printInfo("substituting output ‘%1%’ of ‘%2%’", + localStore->printStorePath(path), + localStore->printStorePath(drvPath)); + localStore->ensurePath(path); + // FIXME: should copy directly from substituter to destStore. } + + copyClosure(*localStore, *destStore, + StorePathSet { path }, + NoRepair, CheckSigs, NoSubstitute); + + time_t stopTime = time(0); + + { + auto mc = startDbUpdate(); + pqxx::work txn(conn); + createSubstitutionStep(txn, startTime, stopTime, build, drvPath, *(step->drv), "out", path); + txn.commit(); + } + + } catch (Error & e) { + printError("while copying/substituting output ‘%s’ of ‘%s’: %s", + localStore->printStorePath(path), + localStore->printStorePath(drvPath), + e.what()); + valid = false; + break; } } }