From 00c2e2c8f317ca366ea8b56096651131e0206729 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 21 Dec 2020 10:01:36 +0100 Subject: [PATCH 01/60] Fix with ca derivations Allow building (floating) content-addressed derivations with Hydra. There's some UI artifacts here and there, but everything should work fine otherwise --- src/hydra-eval-jobs/hydra-eval-jobs.cc | 9 +- src/hydra-queue-runner/build-remote.cc | 66 +++++++--- src/hydra-queue-runner/build-result.cc | 15 ++- src/hydra-queue-runner/build-result.hh | 2 +- src/hydra-queue-runner/builder.cc | 8 +- src/hydra-queue-runner/hydra-queue-runner.cc | 20 ++- src/hydra-queue-runner/queue-monitor.cc | 124 +++++++++---------- src/hydra-queue-runner/state.hh | 2 +- src/lib/Hydra/Controller/Build.pm | 4 +- src/sql/hydra.sql | 4 +- 10 files changed, 150 insertions(+), 104 deletions(-) diff --git a/src/hydra-eval-jobs/hydra-eval-jobs.cc b/src/hydra-eval-jobs/hydra-eval-jobs.cc index acffe1d18..969d2391e 100644 --- a/src/hydra-eval-jobs/hydra-eval-jobs.cc +++ b/src/hydra-eval-jobs/hydra-eval-jobs.cc @@ -209,8 +209,13 @@ static void worker( } nlohmann::json out; - for (auto & j : outputs) - out[j.first] = j.second; + if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + for (auto & j : outputs) + out[j.first] = ""; + } else { + for (auto & j : outputs) + out[j.first] = j.second; + } job["outputs"] = std::move(out); reply["job"] = std::move(job); diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 9f789978f..8c583022e 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -213,7 +213,7 @@ void State::buildRemote(ref destStore, unsigned int remoteVersion; try { - to << SERVE_MAGIC_1 << 0x204; + to << SERVE_MAGIC_1 << 0x205; to.flush(); unsigned int magic = readInt(from); @@ -244,22 +244,28 @@ void State::buildRemote(ref destStore, updateStep(ssSendingInputs); StorePathSet inputs; - BasicDerivation basicDrv(*step->drv); - - for (auto & p : step->drv->inputSrcs) - inputs.insert(p); - - for (auto & input : step->drv->inputDrvs) { - auto drv2 = localStore->readDerivation(input.first); - for (auto & name : input.second) { - if (auto i = get(drv2.outputs, name)) { - auto outPath = i->path(*localStore, drv2.name, name); - inputs.insert(*outPath); - basicDrv.inputSrcs.insert(*outPath); + BasicDerivation basicDrv; + if (auto maybeBasicDrv = step->drv->tryResolve(*destStore)) + basicDrv = *maybeBasicDrv; + else { + basicDrv = BasicDerivation(*step->drv); + for (auto & input : step->drv->inputDrvs) { + auto drv2 = localStore->readDerivation(input.first); + auto hashes = staticOutputHashes(*localStore, drv2); + for (auto & name : input.second) { + if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + auto inputRealisation = destStore->queryRealisation(DrvOutput{hashes.at(name), name}); + assert(inputRealisation); + inputs.insert(inputRealisation->outPath); + basicDrv.inputSrcs.insert(inputRealisation->outPath); } + } } } + for (auto & p : step->drv->inputSrcs) + inputs.insert(p); + /* Ensure that the inputs exist in the destination store. This is a no-op for regular stores, but for the binary cache store, this will copy the inputs to the binary cache from the local @@ -398,6 +404,27 @@ void State::buildRemote(ref destStore, result.logFile = ""; } + DrvOutputs builtOutputs; + if (GET_PROTOCOL_MINOR(remoteVersion) >= 6) { + builtOutputs = worker_proto::read(*localStore, from, Phantom{}); + } else { + assert( + step->drv->type() != DerivationType::CAFloating && + step->drv->type() != DerivationType::DeferredInputAddressed + ); + auto outputMap = localStore->queryPartialDerivationOutputMap(step->drvPath); + auto outputHashes = staticOutputHashes(*localStore, *step->drv); + for (auto & [outputName, outputPath] : outputMap) + if (outputPath) { + auto outputHash = outputHashes.at(outputName); + auto drvOutput = DrvOutput{outputHash, outputName}; + builtOutputs.insert({drvOutput, Realisation{drvOutput, *outputPath}}); + } + } + StorePathSet outputs; + for (auto & [_, realisation] : builtOutputs) + outputs.insert(realisation.outPath); + /* Copy the output paths. */ if (!machine->isLocalhost() || localStore != std::shared_ptr(destStore)) { updateStep(ssReceivingOutputs); @@ -406,12 +433,6 @@ void State::buildRemote(ref 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); - } - /* Get info about each output path. */ std::map infos; size_t totalNarSize = 0; @@ -481,6 +502,13 @@ void State::buildRemote(ref destStore, result.overhead += std::chrono::duration_cast(now2 - now1).count(); } + /* Register the outputs of the newly built drv */ + if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + for (auto & [_, realisation] : builtOutputs) { + localStore->registerDrvOutput(realisation); + } + } + /* Shut down the connection. */ child.to = -1; child.pid.wait(); diff --git a/src/hydra-queue-runner/build-result.cc b/src/hydra-queue-runner/build-result.cc index f69bf0dfb..004ca2b7f 100644 --- a/src/hydra-queue-runner/build-result.cc +++ b/src/hydra-queue-runner/build-result.cc @@ -11,18 +11,17 @@ using namespace nix; BuildOutput getBuildOutput( nix::ref store, NarMemberDatas & narMembers, - const Derivation & drv) + const StorePath & drvPath) { BuildOutput res; /* Compute the closure size. */ StorePathSet outputs; StorePathSet closure; - for (auto & i : drv.outputsAndOptPaths(*store)) - if (i.second.second) { - store->computeFSClosure(*i.second.second, closure); - outputs.insert(*i.second.second); - } + for (auto& [outputName, outputPath] : store->queryDerivationOutputMap(drvPath)) { + store->computeFSClosure(outputPath, closure); + outputs.insert(outputPath); + } for (auto & path : closure) { auto info = store->queryPathInfo(path); res.closureSize += info->narSize; @@ -107,9 +106,9 @@ 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] : store->queryPartialDerivationOutputMap(drvPath)) { BuildProduct product; - auto outPath = output.path(*store, drv.name, name); + auto outPath = output; product.path = store->printStorePath(*outPath); product.type = "nix-build"; product.subtype = name == "out" ? "" : name; diff --git a/src/hydra-queue-runner/build-result.hh b/src/hydra-queue-runner/build-result.hh index a3f71ae96..cf2469749 100644 --- a/src/hydra-queue-runner/build-result.hh +++ b/src/hydra-queue-runner/build-result.hh @@ -42,4 +42,4 @@ struct BuildOutput BuildOutput getBuildOutput( nix::ref store, NarMemberDatas & narMembers, - const nix::Derivation & drv); + const nix::StorePath & drvPath); diff --git a/src/hydra-queue-runner/builder.cc b/src/hydra-queue-runner/builder.cc index 89aa7d15d..d56e624d8 100644 --- a/src/hydra-queue-runner/builder.cc +++ b/src/hydra-queue-runner/builder.cc @@ -221,7 +221,7 @@ State::StepResult State::doBuildStep(nix::ref destStore, if (result.stepStatus == bsSuccess) { updateStep(ssPostProcessing); - res = getBuildOutput(destStore, narMembers, *step->drv); + res = getBuildOutput(destStore, narMembers, step->drvPath); } } @@ -275,9 +275,9 @@ State::StepResult State::doBuildStep(nix::ref destStore, assert(stepNr); - for (auto & i : step->drv->outputsAndOptPaths(*localStore)) { - if (i.second.second) - addRoot(*i.second.second); + for (auto & i : localStore->queryPartialDerivationOutputMap(step->drvPath)) { + if (i.second) + addRoot(*i.second); } /* 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 62eb572c8..223102aa1 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -257,10 +257,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) - 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))); + for (auto& [name, output] : localStore->queryPartialDerivationOutputMap(step->drvPath)) + txn.exec_params0 + ("insert into BuildStepOutputs (build, stepnr, name, path) values ($1, $2, $3, $4)", + buildId, stepNr, name, output ? localStore->printStorePath(*output) : ""); if (status == bsBusy) txn.exec(fmt("notify step_started, '%d\t%d'", buildId, stepNr)); @@ -297,6 +297,18 @@ 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()); + // If we've finished building, all the paths should be known + for (auto& [name, output] : localStore->queryDerivationOutputMap(drvPath)) + 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 49caf8e35..c4fb81cee 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -186,15 +186,14 @@ bool State::getQueuedBuilds(Connection & conn, if (!res[0].is_null()) propagatedFrom = res[0].as(); if (!propagatedFrom) { - for (auto & i : ex.step->drv->outputsAndOptPaths(*localStore)) { - if (i.second.second) { - auto res = txn.exec_params - ("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", - localStore->printStorePath(*i.second.second)); - if (!res[0][0].is_null()) { - propagatedFrom = res[0][0].as(); - break; - } + for (auto & i : localStore->queryPartialDerivationOutputMap(ex.step->drvPath)) { + 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); + if (!res[0][0].is_null()) { + propagatedFrom = res[0][0].as(); + break; } } } @@ -230,12 +229,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, drv); + BuildOutput res = getBuildOutputCached(conn, destStore, build->drvPath); - for (auto & i : drv.outputsAndOptPaths(*localStore)) - if (i.second.second) - addRoot(*i.second.second); + for (auto & i : localStore->queryDerivationOutputMap(build->drvPath)) + addRoot(i.second); { auto mc = startDbUpdate(); @@ -468,26 +465,29 @@ Step::ptr State::createStep(ref destStore, throw PreviousFailure{step}; /* Are all outputs valid? */ + auto outputHashes = staticOutputHashes(*localStore, *(step->drv)); bool valid = true; - DerivationOutputs missing; - for (auto & i : step->drv->outputs) - if (!destStore->isValidPath(*i.second.path(*localStore, step->drv->name, i.first))) { - valid = false; - missing.insert_or_assign(i.first, i.second); - } + std::map> missing; + for (auto & [outputName, maybeOutputPath] : destStore->queryPartialDerivationOutputMap(step->drvPath)) { + if (!(maybeOutputPath && destStore->isValidPath(*maybeOutputPath))) { + valid = false; + missing.insert({{outputHashes.at(outputName), 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) { - auto path = i.second.path(*localStore, step->drv->name, i.first); - if (/* localStore != destStore && */ localStore->isValidPath(*path)) + for (auto & [i, maybePath] : missing) { + if ((maybePath && localStore->isValidPath(*maybePath)) || + (settings.isExperimentalFeatureEnabled("ca-derivations") && localStore->queryRealisation(i))) avail++; - else if (useSubstitutes) { + else if (useSubstitutes && maybePath) { + // TODO: Make work with CA derivations SubstitutablePathInfos infos; - localStore->querySubstitutablePathInfos({{*path, {}}}, infos); + localStore->querySubstitutablePathInfos({{*maybePath, {}}}, infos); if (infos.size() == 1) avail++; } @@ -495,44 +495,44 @@ Step::ptr State::createStep(ref destStore, if (missing.size() == avail) { valid = true; - for (auto & i : missing) { - auto path = i.second.path(*localStore, step->drv->name, i.first); - - try { - time_t startTime = time(0); + for (auto & [i, path] : missing) { + if (path) { + try { + time_t startTime = time(0); + + 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. + } - 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. - } + StorePathSet closure; + localStore->computeFSClosure({*path}, closure); + copyPaths(*localStore, *destStore, closure, NoRepair, CheckSigs, NoSubstitute); - StorePathSet closure; - localStore->computeFSClosure({*path}, closure); - copyPaths(*localStore, *destStore, closure, NoRepair, CheckSigs, NoSubstitute); + time_t stopTime = time(0); - time_t stopTime = time(0); + { + auto mc = startDbUpdate(); + pqxx::work txn(conn); + createSubstitutionStep(txn, startTime, stopTime, build, drvPath, "out", *path); + txn.commit(); + } - { - auto mc = startDbUpdate(); - pqxx::work txn(conn); - createSubstitutionStep(txn, startTime, stopTime, build, drvPath, "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; } - - } catch (Error & e) { - printError("while copying/substituting output ‘%s’ of ‘%s’: %s", - localStore->printStorePath(*path), - localStore->printStorePath(drvPath), - e.what()); - valid = false; - break; } } } @@ -627,17 +627,17 @@ void State::processJobsetSharesChange(Connection & conn) } -BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref destStore, const nix::Derivation & drv) +BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref destStore, const nix::StorePath & drvPath) { { pqxx::work txn(conn); - for (auto & [name, output] : drv.outputsAndOptPaths(*localStore)) { + for (auto & [name, output] : localStore->queryDerivationOutputMap(drvPath)) { 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(); @@ -691,5 +691,5 @@ BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref } NarMemberDatas narMembers; - return getBuildOutput(destStore, narMembers, drv); + return getBuildOutput(destStore, narMembers, drvPath); } diff --git a/src/hydra-queue-runner/state.hh b/src/hydra-queue-runner/state.hh index 1eed5a846..39ea1d9e4 100644 --- a/src/hydra-queue-runner/state.hh +++ b/src/hydra-queue-runner/state.hh @@ -476,7 +476,7 @@ private: void processQueueChange(Connection & conn); BuildOutput getBuildOutputCached(Connection & conn, nix::ref destStore, - const nix::Derivation & drv); + const nix::StorePath & drvPath); Step::ptr createStep(nix::ref store, Connection & conn, Build::ptr build, const nix::StorePath & drvPath, diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm index bde1030e2..023a77ec0 100644 --- a/src/lib/Hydra/Controller/Build.pm +++ b/src/lib/Hydra/Controller/Build.pm @@ -66,9 +66,11 @@ sub build_GET { $c->stash->{template} = 'build.tt'; $c->stash->{isLocalStore} = isLocalStore(); + # XXX: If ca-derivations is enabled then this will always return false + # because `$_->path` will be empty $c->stash->{available} = $c->stash->{isLocalStore} - ? all { isValidPath($_->path) } $build->buildoutputs->all + ? all { $_->path && isValidPath($_->path) } $build->buildoutputs->all : 1; $c->stash->{drvAvailable} = isValidPath $build->drvpath; diff --git a/src/sql/hydra.sql b/src/sql/hydra.sql index 7762e133d..978980ac9 100644 --- a/src/sql/hydra.sql +++ b/src/sql/hydra.sql @@ -245,7 +245,7 @@ create trigger BuildBumped after update on Builds for each row create table BuildOutputs ( build integer not null, name text not null, - path text not null, + path text, primary key (build, name), foreign key (build) references Builds(id) on delete cascade ); @@ -301,7 +301,7 @@ create table BuildStepOutputs ( build integer not null, stepnr integer not null, name text not null, - path text not null, + path text, primary key (build, stepnr, name), foreign key (build) references Builds(id) on delete cascade, foreign key (build, stepnr) references BuildSteps(build, stepnr) on delete cascade From 030d72e60194a78f49698b3dbac293bc43ae1102 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 18 Feb 2021 15:43:49 +0100 Subject: [PATCH 02/60] Add some tests for the content-addressed derivations --- t/content-addressed.pl | 31 +++++++++++++++++++++++++++++++ t/jobs/content-addressed.nix | 28 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 t/content-addressed.pl create mode 100644 t/jobs/content-addressed.nix diff --git a/t/content-addressed.pl b/t/content-addressed.pl new file mode 100644 index 000000000..828476183 --- /dev/null +++ b/t/content-addressed.pl @@ -0,0 +1,31 @@ +use feature 'unicode_strings'; +use strict; +use Setup; + +my %ctx = test_init(); + +require Hydra::Schema; +require Hydra::Model::DB; + +use Test2::V0; + +my $db = Hydra::Model::DB->new; +hydra_setup($db); + +my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"}); + +$jobset = createBaseJobset("content-addressed", "content-addressed.nix"); + +ok(evalSucceeds($jobset), "Evaluating jobs/content-addressed.nix should exit with return code 0"); +ok(nrQueuedBuildsForJobset($jobset) == 3 , "Evaluating jobs/content-addressed.nix should result in 3 builds"); + +for my $build (queuedBuildsForJobset($jobset)) { + ok(runBuild($build), "Build '".$build->job."' from jobs/content-addressed.nix should exit with code 0"); + my $newbuild = $db->resultset('Builds')->find($build->id); + my $expected = $build->job eq "fails" ? 1 : $build->job =~ /with_failed/ ? 6 : 0; + ok($newbuild->finished == 1 && $newbuild->buildstatus == $expected, "Build '".$build->job."' from jobs/content-addressed.nix should have buildstatus $expected"); +} + + +done_testing; + diff --git a/t/jobs/content-addressed.nix b/t/jobs/content-addressed.nix new file mode 100644 index 000000000..b2a2681a3 --- /dev/null +++ b/t/jobs/content-addressed.nix @@ -0,0 +1,28 @@ +let cfg = import ./config.nix; in +let + mkDerivation = args: cfg.mkDerivation ({ + __contentAddressed = true; + outputHashMode = "recursive"; + outputHashAlgo = "sha256"; + } // args); +in +{ + empty_dir = + mkDerivation { + name = "empty-dir"; + builder = ./empty-dir-builder.sh; + }; + + fails = + mkDerivation { + name = "fails"; + builder = ./fail.sh; + }; + + succeed_with_failed = + mkDerivation { + name = "succeed-with-failed"; + builder = ./succeed-with-failed.sh; + }; +} + From 111abeb57416ce1749862852cabae62b2257e994 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 12 Apr 2021 13:33:04 +0200 Subject: [PATCH 03/60] Properly register the unresolved derivation This is something that Nix should do internally as part of the build loop, but since we're partially reimplementing it, we need to also reimplement this logic --- src/hydra-queue-runner/build-remote.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 8c583022e..aece2772e 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -245,6 +245,7 @@ void State::buildRemote(ref destStore, StorePathSet inputs; BasicDerivation basicDrv; + auto outputHashes = staticOutputHashes(*localStore, *step->drv); if (auto maybeBasicDrv = step->drv->tryResolve(*destStore)) basicDrv = *maybeBasicDrv; else { @@ -413,7 +414,6 @@ void State::buildRemote(ref destStore, step->drv->type() != DerivationType::DeferredInputAddressed ); auto outputMap = localStore->queryPartialDerivationOutputMap(step->drvPath); - auto outputHashes = staticOutputHashes(*localStore, *step->drv); for (auto & [outputName, outputPath] : outputMap) if (outputPath) { auto outputHash = outputHashes.at(outputName); @@ -504,8 +504,15 @@ void State::buildRemote(ref destStore, /* Register the outputs of the newly built drv */ if (settings.isExperimentalFeatureEnabled("ca-derivations")) { - for (auto & [_, realisation] : builtOutputs) { + for (auto & [outputId, realisation] : builtOutputs) { + // Register the resolved drv output localStore->registerDrvOutput(realisation); + + // Also register the unresolved one + auto unresolvedRealisation = realisation; + unresolvedRealisation.signatures.clear(); + unresolvedRealisation.id.drvHash = outputHashes.at(outputId.outputName); + localStore->registerDrvOutput(unresolvedRealisation); } } From 22a638e5be534ff82f13ad4d9fe54a31c0e84df9 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 20 Apr 2021 15:16:34 +0200 Subject: [PATCH 04/60] Fix a localStore/destStore confusion This hadn't been caught anywhere until now as all the tests run with `destStore==localStore` --- src/hydra-queue-runner/build-remote.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index aece2772e..cff46152c 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -246,7 +246,7 @@ void State::buildRemote(ref destStore, StorePathSet inputs; BasicDerivation basicDrv; auto outputHashes = staticOutputHashes(*localStore, *step->drv); - if (auto maybeBasicDrv = step->drv->tryResolve(*destStore)) + if (auto maybeBasicDrv = step->drv->tryResolve(*localStore)) basicDrv = *maybeBasicDrv; else { basicDrv = BasicDerivation(*step->drv); @@ -255,7 +255,7 @@ void State::buildRemote(ref destStore, auto hashes = staticOutputHashes(*localStore, drv2); for (auto & name : input.second) { if (settings.isExperimentalFeatureEnabled("ca-derivations")) { - auto inputRealisation = destStore->queryRealisation(DrvOutput{hashes.at(name), name}); + auto inputRealisation = localStore->queryRealisation(DrvOutput{hashes.at(name), name}); assert(inputRealisation); inputs.insert(inputRealisation->outPath); basicDrv.inputSrcs.insert(inputRealisation->outPath); From ddfece591fd13dc835590c47d242a27802214b42 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 20 Apr 2021 17:24:39 +0200 Subject: [PATCH 05/60] Check for previous builds using the drvPath rather than the output paths Otherwise ca-derivations are always assumed to be the same because their output path is empty --- src/script/hydra-eval-jobset | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/script/hydra-eval-jobset b/src/script/hydra-eval-jobset index de437ecdb..992830631 100755 --- a/src/script/hydra-eval-jobset +++ b/src/script/hydra-eval-jobset @@ -442,7 +442,7 @@ sub checkBuild { # the eval), but they give a factor 1000 speedup on # the Nixpkgs jobset with PostgreSQL. { jobset_id => $jobset->get_column('id'), job => $jobName, - name => $firstOutputName, path => $firstOutputPath }, + name => $firstOutputName, drvPath => $drvPath }, { rows => 1, columns => ['id', 'finished'], join => ['buildoutputs'] }); if (defined $prevBuild) { #print STDERR " already scheduled/built as build ", $prevBuild->id, "\n"; From fad88390bac5dd551d1c0f832fa3160c470bb764 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 20 Apr 2021 18:51:09 +0200 Subject: [PATCH 06/60] Don't call queryPartialDerivationOutputMap on the dest store Unless it's also the local store, this will likely fail because this call requires having the derivation handy, which the dest store generally won't have --- src/hydra-queue-runner/queue-monitor.cc | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index c4fb81cee..aceaf9da4 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -468,12 +468,18 @@ Step::ptr State::createStep(ref destStore, auto outputHashes = staticOutputHashes(*localStore, *(step->drv)); bool valid = true; std::map> missing; - for (auto & [outputName, maybeOutputPath] : destStore->queryPartialDerivationOutputMap(step->drvPath)) { - if (!(maybeOutputPath && destStore->isValidPath(*maybeOutputPath))) { + for (auto [outputName, outputHash] : outputHashes) { + if (! destStore->queryRealisation(DrvOutput{outputHash, outputName})) { valid = false; - missing.insert({{outputHashes.at(outputName), outputName}, maybeOutputPath}); + missing.insert({{outputHash, outputName}, std::nullopt}); } } + /* for (auto & [outputName, maybeOutputPath] : destStore->queryPartialDerivationOutputMap(step->drvPath)) { */ + /* if (!(maybeOutputPath && destStore->isValidPath(*maybeOutputPath))) { */ + /* valid = false; */ + /* missing.insert({{outputHashes.at(outputName), outputName}, maybeOutputPath}); */ + /* } */ + /* } */ /* Try to copy the missing paths from the local store or from substitutes. */ From 410d7a0b86ebd4ae2b61a43383412e2472167841 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 22 Apr 2021 11:16:20 +0200 Subject: [PATCH 07/60] getBuildOutput: Don't query the dest store for the outputs The dest store doesn't need to have the derivation, so rather first ask the local store for all the built outputs, and directly pass these to `getBuildOutput` --- src/hydra-queue-runner/build-result.cc | 11 +++++------ src/hydra-queue-runner/build-result.hh | 2 +- src/hydra-queue-runner/builder.cc | 2 +- src/hydra-queue-runner/queue-monitor.cc | 7 +++++-- 4 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/hydra-queue-runner/build-result.cc b/src/hydra-queue-runner/build-result.cc index 004ca2b7f..e670abc63 100644 --- a/src/hydra-queue-runner/build-result.cc +++ b/src/hydra-queue-runner/build-result.cc @@ -11,14 +11,14 @@ using namespace nix; BuildOutput getBuildOutput( nix::ref store, NarMemberDatas & narMembers, - const StorePath & drvPath) + const OutputPathMap derivationOutputs) { BuildOutput res; /* Compute the closure size. */ StorePathSet outputs; StorePathSet closure; - for (auto& [outputName, outputPath] : store->queryDerivationOutputMap(drvPath)) { + for (auto& [_, outputPath] : derivationOutputs) { store->computeFSClosure(outputPath, closure); outputs.insert(outputPath); } @@ -106,13 +106,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] : store->queryPartialDerivationOutputMap(drvPath)) { + for (auto& [name, output] : derivationOutputs) { BuildProduct product; - auto outPath = output; - product.path = store->printStorePath(*outPath); + product.path = store->printStorePath(output); 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()); diff --git a/src/hydra-queue-runner/build-result.hh b/src/hydra-queue-runner/build-result.hh index cf2469749..cba22ca38 100644 --- a/src/hydra-queue-runner/build-result.hh +++ b/src/hydra-queue-runner/build-result.hh @@ -42,4 +42,4 @@ struct BuildOutput BuildOutput getBuildOutput( nix::ref store, NarMemberDatas & narMembers, - const nix::StorePath & drvPath); + const nix::OutputPathMap derivationOutputs); diff --git a/src/hydra-queue-runner/builder.cc b/src/hydra-queue-runner/builder.cc index d56e624d8..1526a2428 100644 --- a/src/hydra-queue-runner/builder.cc +++ b/src/hydra-queue-runner/builder.cc @@ -221,7 +221,7 @@ State::StepResult State::doBuildStep(nix::ref destStore, if (result.stepStatus == bsSuccess) { updateStep(ssPostProcessing); - res = getBuildOutput(destStore, narMembers, step->drvPath); + res = getBuildOutput(destStore, narMembers, localStore->queryDerivationOutputMap(step->drvPath)); } } diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index aceaf9da4..1cf24093c 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -635,10 +635,13 @@ void State::processJobsetSharesChange(Connection & conn) BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref destStore, const nix::StorePath & drvPath) { + + auto derivationOutputs = localStore->queryDerivationOutputMap(drvPath); + { pqxx::work txn(conn); - for (auto & [name, output] : localStore->queryDerivationOutputMap(drvPath)) { + 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 " @@ -697,5 +700,5 @@ BuildOutput State::getBuildOutputCached(Connection & conn, nix::ref } NarMemberDatas narMembers; - return getBuildOutput(destStore, narMembers, drvPath); + return getBuildOutput(destStore, narMembers, derivationOutputs); } From 68d18b2e7fc23236d4ab5a6cf491c14681519db2 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 22 Apr 2021 13:28:02 +0200 Subject: [PATCH 08/60] Correctly handle the non-ca case when building Fix a bug introduced by bb12e9bce0fc5a11b5e804cbab9a43876576722a --- src/hydra-queue-runner/queue-monitor.cc | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 1cf24093c..02b637806 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -468,18 +468,23 @@ Step::ptr State::createStep(ref destStore, auto outputHashes = staticOutputHashes(*localStore, *(step->drv)); bool valid = true; std::map> missing; - for (auto [outputName, outputHash] : outputHashes) { - if (! destStore->queryRealisation(DrvOutput{outputHash, outputName})) { - valid = false; - missing.insert({{outputHash, outputName}, std::nullopt}); + if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + for (auto [outputName, outputHash] : outputHashes) { + if (! destStore->queryRealisation(DrvOutput{outputHash, outputName})) { + valid = false; + missing.insert({{outputHash, outputName}, std::nullopt}); + } + } + } else { + for (auto & [outputName, maybeOutputPath] : step->drv->outputsAndOptPaths(*destStore)) { + // If we're not CA, all the output paths should be known + assert(maybeOutputPath.second); + if (!destStore->isValidPath(*maybeOutputPath.second)) { + valid = false; + missing.insert({{outputHashes.at(outputName), outputName}, maybeOutputPath.second}); + } } } - /* for (auto & [outputName, maybeOutputPath] : destStore->queryPartialDerivationOutputMap(step->drvPath)) { */ - /* if (!(maybeOutputPath && destStore->isValidPath(*maybeOutputPath))) { */ - /* valid = false; */ - /* missing.insert({{outputHashes.at(outputName), outputName}, maybeOutputPath}); */ - /* } */ - /* } */ /* Try to copy the missing paths from the local store or from substitutes. */ From af8f070790fcec30e9d807b2892a98468d5e1a7a Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 22 Apr 2021 15:45:05 +0200 Subject: [PATCH 09/60] Allow gc-ing the destination store Ensure that if a path is present locally but not in the destination store, then we properly copy it --- src/hydra-queue-runner/queue-monitor.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 02b637806..3f5cc1344 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -493,8 +493,10 @@ Step::ptr State::createStep(ref destStore, size_t avail = 0; for (auto & [i, maybePath] : missing) { if ((maybePath && localStore->isValidPath(*maybePath)) || - (settings.isExperimentalFeatureEnabled("ca-derivations") && localStore->queryRealisation(i))) + (settings.isExperimentalFeatureEnabled("ca-derivations") && localStore->queryRealisation(i))) { + maybePath = localStore->queryRealisation(i)->outPath; avail++; + } else if (useSubstitutes && maybePath) { // TODO: Make work with CA derivations SubstitutablePathInfos infos; From 70402e5c9f0f34ad51193975b9f8f852da58fe54 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 12:42:23 +0200 Subject: [PATCH 10/60] Actually use the `content-addressed` test Looks like it has never been used because the file was ill-named --- t/{content-addressed.pl => content-addressed.t} | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) rename t/{content-addressed.pl => content-addressed.t} (84%) diff --git a/t/content-addressed.pl b/t/content-addressed.t similarity index 84% rename from t/content-addressed.pl rename to t/content-addressed.t index 828476183..8a70193f4 100644 --- a/t/content-addressed.pl +++ b/t/content-addressed.t @@ -2,7 +2,11 @@ use strict; use Setup; -my %ctx = test_init(); +my %ctx = test_init( + nix_config => qq| + experimental-features = ca-derivations + |, +); require Hydra::Schema; require Hydra::Model::DB; @@ -14,7 +18,7 @@ my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"}); -$jobset = createBaseJobset("content-addressed", "content-addressed.nix"); +my $jobset = createBaseJobset("content-addressed", "content-addressed.nix", $ctx{jobsdir}); ok(evalSucceeds($jobset), "Evaluating jobs/content-addressed.nix should exit with return code 0"); ok(nrQueuedBuildsForJobset($jobset) == 3 , "Evaluating jobs/content-addressed.nix should result in 3 builds"); From d1e41cf406e8b1bbe306cd6786e9b1a51011dd82 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 14:27:53 +0200 Subject: [PATCH 11/60] Update the ca test Make it follow the same APIs and conventions as the rest of the testsuite --- t/content-addressed.t | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/t/content-addressed.t b/t/content-addressed.t index 8a70193f4..2d61b474b 100644 --- a/t/content-addressed.t +++ b/t/content-addressed.t @@ -21,13 +21,14 @@ my $project = $db->resultset('Projects')->create({name => "tests", displayname = my $jobset = createBaseJobset("content-addressed", "content-addressed.nix", $ctx{jobsdir}); ok(evalSucceeds($jobset), "Evaluating jobs/content-addressed.nix should exit with return code 0"); -ok(nrQueuedBuildsForJobset($jobset) == 3 , "Evaluating jobs/content-addressed.nix should result in 3 builds"); +is(nrQueuedBuildsForJobset($jobset), 3, "Evaluating jobs/content-addressed.nix should result in 3 builds"); for my $build (queuedBuildsForJobset($jobset)) { ok(runBuild($build), "Build '".$build->job."' from jobs/content-addressed.nix should exit with code 0"); my $newbuild = $db->resultset('Builds')->find($build->id); + is($newbuild->finished, 1, "Build '".$build->job."' from jobs/content-addressed.nix should be finished."); my $expected = $build->job eq "fails" ? 1 : $build->job =~ /with_failed/ ? 6 : 0; - ok($newbuild->finished == 1 && $newbuild->buildstatus == $expected, "Build '".$build->job."' from jobs/content-addressed.nix should have buildstatus $expected"); + is($newbuild->buildstatus, $expected, "Build '".$build->job."' from jobs/content-addressed.nix should have buildstatus $expected."); } From c5a092e79053874bc79130b6d1213d663c090191 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 14:30:41 +0200 Subject: [PATCH 12/60] Clarify what happens with the out paths in the db Only ca derivations might have an empty `path` field (because this one will only be known at a later stage) --- src/lib/Hydra/Controller/Build.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm index 023a77ec0..6771603c7 100644 --- a/src/lib/Hydra/Controller/Build.pm +++ b/src/lib/Hydra/Controller/Build.pm @@ -66,8 +66,8 @@ sub build_GET { $c->stash->{template} = 'build.tt'; $c->stash->{isLocalStore} = isLocalStore(); - # XXX: If ca-derivations is enabled then this will always return false - # because `$_->path` will be empty + # XXX: If the derivation is content-addressed then this will always return + # false because `$_->path` will be empty $c->stash->{available} = $c->stash->{isLocalStore} ? all { $_->path && isValidPath($_->path) } $build->buildoutputs->all From d2434247c2e18495e3bffa5626f9d4c2a472ac0d Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 14:36:39 +0200 Subject: [PATCH 13/60] Move mkDerivation-for-ca to config.nix.in That way it can be reused more generically --- t/jobs/config.nix.in | 5 +++++ t/jobs/content-addressed.nix | 13 +++---------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/t/jobs/config.nix.in b/t/jobs/config.nix.in index 51b6c06fc..41776341c 100644 --- a/t/jobs/config.nix.in +++ b/t/jobs/config.nix.in @@ -6,4 +6,9 @@ rec { system = builtins.currentSystem; PATH = path; } // args); + mkContentAddressedDerivation = args: mkDerivation ({ + __contentAddressed = true; + outputHashMode = "recursive"; + outputHashAlgo = "sha256"; + } // args); } diff --git a/t/jobs/content-addressed.nix b/t/jobs/content-addressed.nix index b2a2681a3..4ce8dc8b1 100644 --- a/t/jobs/content-addressed.nix +++ b/t/jobs/content-addressed.nix @@ -1,26 +1,19 @@ let cfg = import ./config.nix; in -let - mkDerivation = args: cfg.mkDerivation ({ - __contentAddressed = true; - outputHashMode = "recursive"; - outputHashAlgo = "sha256"; - } // args); -in { empty_dir = - mkDerivation { + cfg.mkContentAddressedDerivation { name = "empty-dir"; builder = ./empty-dir-builder.sh; }; fails = - mkDerivation { + cfg.mkContentAddressedDerivation { name = "fails"; builder = ./fail.sh; }; succeed_with_failed = - mkDerivation { + cfg.mkContentAddressedDerivation { name = "succeed-with-failed"; builder = ./succeed-with-failed.sh; }; From 4a6a342dbfe2472df108c3e4352ccdea654b4af2 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 16:05:21 +0200 Subject: [PATCH 14/60] eval-jobs: Don't strip the output path for ia derivations This causes the `Output store paths` field to always appear empty (regardless of whether the drv is CA or not), which isn't good --- src/hydra-eval-jobs/hydra-eval-jobs.cc | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/hydra-eval-jobs/hydra-eval-jobs.cc b/src/hydra-eval-jobs/hydra-eval-jobs.cc index 969d2391e..d98ad88d1 100644 --- a/src/hydra-eval-jobs/hydra-eval-jobs.cc +++ b/src/hydra-eval-jobs/hydra-eval-jobs.cc @@ -209,13 +209,15 @@ static void worker( } nlohmann::json out; - if (settings.isExperimentalFeatureEnabled("ca-derivations")) { - for (auto & j : outputs) + for (auto & j : outputs) + if (localStore->maybeParseStorePath(j.second)) + out[j.first] = j.second; + else + // If this isn't a real store path, it means that it's a + // placeholder. + // Replace it by an empty string for the time being until + // we build the derivation and can give this a proper value out[j.first] = ""; - } else { - for (auto & j : outputs) - out[j.first] = j.second; - } job["outputs"] = std::move(out); reply["job"] = std::move(job); From 89a55b90eb648d8e9b39cf577f8467e7e0021ee4 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 17:23:31 +0200 Subject: [PATCH 15/60] Add the output paths of CA derivations to the database Once the build is done, update the `BuildOutputs` table to add the output paths of the derivation --- src/hydra-queue-runner/build-result.cc | 3 ++- src/hydra-queue-runner/build-result.hh | 2 ++ src/hydra-queue-runner/hydra-queue-runner.cc | 9 +++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/hydra-queue-runner/build-result.cc b/src/hydra-queue-runner/build-result.cc index e670abc63..e17d5904a 100644 --- a/src/hydra-queue-runner/build-result.cc +++ b/src/hydra-queue-runner/build-result.cc @@ -18,9 +18,10 @@ BuildOutput getBuildOutput( /* Compute the closure size. */ StorePathSet outputs; StorePathSet closure; - for (auto& [_, outputPath] : derivationOutputs) { + for (auto& [outputName, outputPath] : derivationOutputs) { store->computeFSClosure(outputPath, closure); outputs.insert(outputPath); + res.outputs.insert({outputName, outputPath}); } for (auto & path : closure) { auto info = store->queryPathInfo(path); diff --git a/src/hydra-queue-runner/build-result.hh b/src/hydra-queue-runner/build-result.hh index cba22ca38..7d47f67ce 100644 --- a/src/hydra-queue-runner/build-result.hh +++ b/src/hydra-queue-runner/build-result.hh @@ -36,6 +36,8 @@ struct BuildOutput std::list products; + std::map outputs; + std::map metrics; }; diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 223102aa1..3bb8ba7ea 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -414,6 +414,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; From 34aaa0ba5a0ce89610e591a1c8adda8f4fdf389c Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 20:13:45 +0200 Subject: [PATCH 16/60] Add more ca tests - Test that the `build` page loads correctly - Test that after a succesful build, all the corresponding outputs have their `path` correctly set --- t/content-addressed.t | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/t/content-addressed.t b/t/content-addressed.t index 2d61b474b..144430e32 100644 --- a/t/content-addressed.t +++ b/t/content-addressed.t @@ -11,7 +11,12 @@ my %ctx = test_init( require Hydra::Schema; require Hydra::Model::DB; +use JSON; + +use HTTP::Request::Common; use Test2::V0; +require Catalyst::Test; +Catalyst::Test->import('Hydra'); my $db = Hydra::Model::DB->new; hydra_setup($db); @@ -29,6 +34,24 @@ for my $build (queuedBuildsForJobset($jobset)) { is($newbuild->finished, 1, "Build '".$build->job."' from jobs/content-addressed.nix should be finished."); my $expected = $build->job eq "fails" ? 1 : $build->job =~ /with_failed/ ? 6 : 0; is($newbuild->buildstatus, $expected, "Build '".$build->job."' from jobs/content-addressed.nix should have buildstatus $expected."); + + my $response = request("/build/".$build->id); + ok($response->is_success, "The 'build' page for build '".$build->job."' should load properly"); + + if ($newbuild->buildstatus == 0) { + my $buildOutputs = $newbuild->buildoutputs; + for my $output ($newbuild->buildoutputs) { + # XXX: This hardcodes /nix/store/. + # It's fine because in practice the nix store for the tests will be of + # the form `/some/thing/nix/store/`, but it would be cleaner if there + # was a way to query Nix for its store dir? + like( + $output->path, qr|/nix/store/|, + "Output '".$output->name."' of build '".$build->job."' should be a valid store path" + ); + } + } + } From 2a2aa72f5423c3fba8197695ff659bf70607b9ff Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 20:19:07 +0200 Subject: [PATCH 17/60] Test evaluating a ca drv without the experimental feature --- ...t-addressed-without-experimental-feature.t | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 t/content-addressed-without-experimental-feature.t diff --git a/t/content-addressed-without-experimental-feature.t b/t/content-addressed-without-experimental-feature.t new file mode 100644 index 000000000..60b4ef4a6 --- /dev/null +++ b/t/content-addressed-without-experimental-feature.t @@ -0,0 +1,27 @@ +use feature 'unicode_strings'; +use strict; +use Setup; + +my %ctx = test_init(); + +require Hydra::Schema; +require Hydra::Model::DB; + +use JSON; + +use HTTP::Request::Common; +use Test2::V0; +require Catalyst::Test; +Catalyst::Test->import('Hydra'); + +my $db = Hydra::Model::DB->new; +hydra_setup($db); + +my $project = $db->resultset('Projects')->create({name => "tests", displayname => "", owner => "root"}); + +my $jobset = createBaseJobset("content-addressed", "content-addressed.nix", $ctx{jobsdir}); + +ok(evalSucceeds($jobset), "Evaluating jobs/content-addressed.nix without the experimental feature should exit with return code 0"); +is(nrQueuedBuildsForJobset($jobset), 0, "Evaluating jobs/content-addressed.nix without the experimental Nix feature should result in 0 build"); + +done_testing; From a07b1dbb22b1522e6c962bf5e8f07c2a6c72d019 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 20:21:02 +0200 Subject: [PATCH 18/60] Move the ca tests to a subdirectory --- t/{content-addressed.t => content-addressed/basic.t} | 2 +- .../without-experimental-feature.t} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename t/{content-addressed.t => content-addressed/basic.t} (99%) rename t/{content-addressed-without-experimental-feature.t => content-addressed/without-experimental-feature.t} (97%) diff --git a/t/content-addressed.t b/t/content-addressed/basic.t similarity index 99% rename from t/content-addressed.t rename to t/content-addressed/basic.t index 144430e32..76eccce86 100644 --- a/t/content-addressed.t +++ b/t/content-addressed/basic.t @@ -11,7 +11,7 @@ my %ctx = test_init( require Hydra::Schema; require Hydra::Model::DB; -use JSON; +use JSON::MaybeXS; use HTTP::Request::Common; use Test2::V0; diff --git a/t/content-addressed-without-experimental-feature.t b/t/content-addressed/without-experimental-feature.t similarity index 97% rename from t/content-addressed-without-experimental-feature.t rename to t/content-addressed/without-experimental-feature.t index 60b4ef4a6..0c8c84f8c 100644 --- a/t/content-addressed-without-experimental-feature.t +++ b/t/content-addressed/without-experimental-feature.t @@ -7,7 +7,7 @@ my %ctx = test_init(); require Hydra::Schema; require Hydra::Model::DB; -use JSON; +use JSON::MaybeXS; use HTTP::Request::Common; use Test2::V0; From 3d64cfb1d07f12f0367a2c308305869041d4e375 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 20:44:39 +0200 Subject: [PATCH 19/60] Fix a potential queue-runner crash The previous logic was (probably) crashing hydra-queue-runner if 1. The `ca-derivation` feature wasn't enabled 2. An output of the current step was present on the local store but not on the dest store The reason for that is that the code was trying to call `localStore->queryRealisation` which only works if `ca-derivations` is enabled --- src/hydra-queue-runner/queue-monitor.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 3f5cc1344..8fef2e6a9 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -492,12 +492,12 @@ Step::ptr State::createStep(ref destStore, size_t avail = 0; for (auto & [i, maybePath] : missing) { - if ((maybePath && localStore->isValidPath(*maybePath)) || - (settings.isExperimentalFeatureEnabled("ca-derivations") && localStore->queryRealisation(i))) { + if ((maybePath && localStore->isValidPath(*maybePath))) + avail++; + else if (settings.isExperimentalFeatureEnabled("ca-derivations") && localStore->queryRealisation(i)) { maybePath = localStore->queryRealisation(i)->outPath; avail++; - } - else if (useSubstitutes && maybePath) { + } else if (useSubstitutes && maybePath) { // TODO: Make work with CA derivations SubstitutablePathInfos infos; localStore->querySubstitutablePathInfos({{*maybePath, {}}}, infos); From 9f2b576f7129c4e8d4cb3b8eba5e3d54b8282de5 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 21:04:16 +0200 Subject: [PATCH 20/60] Prevent enabling ca-derivations from breaking substitution Make it so that hydra still correctly shortcuts input-addressed derivations, even when the `ca-derivations` experimental feature is enabled. (The substitution of content-addressed derivations doesn't work properly in hydra yet) --- src/hydra-queue-runner/queue-monitor.cc | 21 ++++++++++----------- t/queue-runner/notifications.t | 2 +- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 8fef2e6a9..09d9f591a 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -468,20 +468,19 @@ Step::ptr State::createStep(ref destStore, auto outputHashes = staticOutputHashes(*localStore, *(step->drv)); bool valid = true; std::map> missing; - if (settings.isExperimentalFeatureEnabled("ca-derivations")) { - for (auto [outputName, outputHash] : outputHashes) { - if (! destStore->queryRealisation(DrvOutput{outputHash, outputName})) { + for (auto &[outputName, maybeOutputPath] : + step->drv->outputsAndOptPaths(*destStore)) { + auto outputHash = outputHashes.at(outputName); + if (maybeOutputPath.second) { + if (!destStore->isValidPath(*maybeOutputPath.second)) { valid = false; - missing.insert({{outputHash, outputName}, std::nullopt}); + missing.insert({{outputHash, outputName}, maybeOutputPath.second}); } - } - } else { - for (auto & [outputName, maybeOutputPath] : step->drv->outputsAndOptPaths(*destStore)) { - // If we're not CA, all the output paths should be known - assert(maybeOutputPath.second); - if (!destStore->isValidPath(*maybeOutputPath.second)) { + } else { + settings.requireExperimentalFeature("ca-derivations"); + if (!destStore->queryRealisation(DrvOutput{outputHash, outputName})) { valid = false; - missing.insert({{outputHashes.at(outputName), outputName}, maybeOutputPath.second}); + missing.insert({{outputHash, outputName}, std::nullopt}); } } } diff --git a/t/queue-runner/notifications.t b/t/queue-runner/notifications.t index b35b2b2ff..0332061db 100644 --- a/t/queue-runner/notifications.t +++ b/t/queue-runner/notifications.t @@ -8,7 +8,7 @@ my $binarycachedir = File::Temp->newdir(); my $ctx = test_context( nix_config => qq| - experimental-features = nix-command + experimental-features = nix-command ca-derivations substituters = file://${binarycachedir}?trusted=1 |, hydra_config => q| From a79afe0d5bc5828935f1ee9aca0c66b10a1fc01a Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Apr 2021 21:15:20 +0200 Subject: [PATCH 21/60] Document the state of ca derivations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not much to document, but at least it’s there :) --- doc/manual/src/projects.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/manual/src/projects.md b/doc/manual/src/projects.md index 95174f1bc..c35d0ce33 100644 --- a/doc/manual/src/projects.md +++ b/doc/manual/src/projects.md @@ -399,3 +399,10 @@ analogous: | `String value` | `gitea_status_repo` | *Name of the `Git checkout` input* | | `String value` | `gitea_http_url` | *Public URL of `gitea`*, optional | +Content-addressed derivations +----------------------------- + +Hydra can to a certain extent use the [`ca-derivations` experimental Nix feature](https://github.com/NixOS/rfcs/pull/62). +To use it, make sure that the Nix version you use is at least as recent as the one used in hydra's flake. + +Be warned that this support is still highly experimental, and anything beyond the basic functionality might be broken at that point. From f676dc8dc28aae6e647996df2bbb25b883c396ff Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 3 May 2021 20:54:37 +0200 Subject: [PATCH 22/60] Also register the ca outputs on the dest store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise it’s a bit tricky to use it as a binary cache --- src/hydra-queue-runner/build-remote.cc | 2 ++ t/content-addressed/basic.t | 1 + t/lib/HydraTestContext.pm | 6 +++++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index cff46152c..08185d65a 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -507,12 +507,14 @@ void State::buildRemote(ref destStore, for (auto & [outputId, realisation] : 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(outputId.outputName); localStore->registerDrvOutput(unresolvedRealisation); + destStore->registerDrvOutput(unresolvedRealisation); } } diff --git a/t/content-addressed/basic.t b/t/content-addressed/basic.t index 76eccce86..2f7d4e881 100644 --- a/t/content-addressed/basic.t +++ b/t/content-addressed/basic.t @@ -54,6 +54,7 @@ for my $build (queuedBuildsForJobset($jobset)) { } +isnt(<$ctx{deststoredir}/realisations/*>, "", "The destination store should have the realisations of the built derivations registered"); done_testing; diff --git a/t/lib/HydraTestContext.pm b/t/lib/HydraTestContext.pm index 9f9db4f2f..c83b20ef3 100644 --- a/t/lib/HydraTestContext.pm +++ b/t/lib/HydraTestContext.pm @@ -35,6 +35,8 @@ use CliRunners; sub new { my ($class, %opts) = @_; + my $deststoredir; + my $dir = File::Temp->newdir(); $ENV{'HYDRA_DATA'} = "$dir/hydra-data"; @@ -48,6 +50,7 @@ sub new { my $hydra_config = $opts{'hydra_config'} || ""; if ($opts{'use_external_destination_store'} // 1) { + $deststoredir = "$dir/nix/dest-store"; $hydra_config = "store_uri = file:$dir/nix/dest-store\n" . $hydra_config; } @@ -70,7 +73,8 @@ sub new { db_handle => $pgsql, tmpdir => $dir, testdir => abs_path(dirname(__FILE__) . "/.."), - jobsdir => abs_path(dirname(__FILE__) . "/../jobs") + jobsdir => abs_path(dirname(__FILE__) . "/../jobs"), + deststoredir => $deststoredir, }; return bless $self, $class; From 79e5e736e6cd80aff8832754d338eda8525c1655 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 31 May 2021 15:30:31 +0200 Subject: [PATCH 23/60] Fix the resolving of derivations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Only call `tryResolve` when we now that we can do it safely - Make sure that we always copy *all* the inputs of the derivation This is missing a nice test, but it’s also a bit of a pain to write, so it’ll wait a bit. --- src/hydra-queue-runner/build-remote.cc | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 08185d65a..972f3dd00 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -243,12 +243,20 @@ void State::buildRemote(ref destStore, outputs of the input derivations. */ updateStep(ssSendingInputs); - StorePathSet inputs; BasicDerivation basicDrv; auto outputHashes = staticOutputHashes(*localStore, *step->drv); - if (auto maybeBasicDrv = step->drv->tryResolve(*localStore)) + if ((derivationIsCA(step->drv->type()) + || step->drv->type() == DerivationType::DeferredInputAddressed)) { + auto maybeBasicDrv = step->drv->tryResolve(*localStore); + if (!maybeBasicDrv) + throw Error( + "the derivation '%s' can’t be resolved. It’s probably " + "missing some outputs", + localStore->printStorePath(step->drvPath)); basicDrv = *maybeBasicDrv; - else { + } else { + // If the derivation is a real `InputAddressed` derivation, we must + // resolve it manually to keep the original output paths basicDrv = BasicDerivation(*step->drv); for (auto & input : step->drv->inputDrvs) { auto drv2 = localStore->readDerivation(input.first); @@ -257,16 +265,12 @@ void State::buildRemote(ref destStore, if (settings.isExperimentalFeatureEnabled("ca-derivations")) { auto inputRealisation = localStore->queryRealisation(DrvOutput{hashes.at(name), name}); assert(inputRealisation); - inputs.insert(inputRealisation->outPath); basicDrv.inputSrcs.insert(inputRealisation->outPath); } } } } - for (auto & p : step->drv->inputSrcs) - inputs.insert(p); - /* Ensure that the inputs exist in the destination store. This is a no-op for regular stores, but for the binary cache store, this will copy the inputs to the binary cache from the local @@ -287,7 +291,7 @@ void State::buildRemote(ref destStore, auto now1 = std::chrono::steady_clock::now(); - copyClosureTo(machine->state->sendLock, destStore, from, to, inputs, true); + copyClosureTo(machine->state->sendLock, destStore, from, to, basicDrv.inputSrcs, true); auto now2 = std::chrono::steady_clock::now(); From 529c28f874784915e854c0781b62ea5cb1f0e878 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 28 Jul 2021 11:36:58 +0200 Subject: [PATCH 24/60] Make non-ca-depending-on-ca derivations work Fix a small mistake that caused an assertion failure in such a case --- src/hydra-queue-runner/build-remote.cc | 3 +-- t/content-addressed/basic.t | 2 +- t/jobs/content-addressed.nix | 9 ++++++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 972f3dd00..6d73cf4b7 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -245,8 +245,7 @@ void State::buildRemote(ref destStore, BasicDerivation basicDrv; auto outputHashes = staticOutputHashes(*localStore, *step->drv); - if ((derivationIsCA(step->drv->type()) - || step->drv->type() == DerivationType::DeferredInputAddressed)) { + if (!derivationHasKnownOutputPaths(step->drv->type())) { auto maybeBasicDrv = step->drv->tryResolve(*localStore); if (!maybeBasicDrv) throw Error( diff --git a/t/content-addressed/basic.t b/t/content-addressed/basic.t index 2f7d4e881..28cce0df8 100644 --- a/t/content-addressed/basic.t +++ b/t/content-addressed/basic.t @@ -26,7 +26,7 @@ my $project = $db->resultset('Projects')->create({name => "tests", displayname = my $jobset = createBaseJobset("content-addressed", "content-addressed.nix", $ctx{jobsdir}); ok(evalSucceeds($jobset), "Evaluating jobs/content-addressed.nix should exit with return code 0"); -is(nrQueuedBuildsForJobset($jobset), 3, "Evaluating jobs/content-addressed.nix should result in 3 builds"); +is(nrQueuedBuildsForJobset($jobset), 4, "Evaluating jobs/content-addressed.nix should result in 4 builds"); for my $build (queuedBuildsForJobset($jobset)) { ok(runBuild($build), "Build '".$build->job."' from jobs/content-addressed.nix should exit with code 0"); diff --git a/t/jobs/content-addressed.nix b/t/jobs/content-addressed.nix index 4ce8dc8b1..785e917c2 100644 --- a/t/jobs/content-addressed.nix +++ b/t/jobs/content-addressed.nix @@ -1,5 +1,5 @@ let cfg = import ./config.nix; in -{ +rec { empty_dir = cfg.mkContentAddressedDerivation { name = "empty-dir"; @@ -17,5 +17,12 @@ let cfg = import ./config.nix; in name = "succeed-with-failed"; builder = ./succeed-with-failed.sh; }; + + nonCaDependingOnCA = + cfg.mkDerivation { + name = "non-ca-depending-on-ca"; + builder = ./empty-dir-builder.sh; + FOO = empty_dir; + }; } From c6005f48054ceff2ba2e00ec1110958099e33a36 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 29 Jul 2021 07:17:57 +0200 Subject: [PATCH 25/60] =?UTF-8?q?Correctly=20handle=20=E2=80=9Creal?= =?UTF-8?q?=E2=80=9D=20input-addressed=20derivations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s possible that their inputs don’t have a properly registered `realisation` (coming from an older Nix), so don’t assume that --- src/hydra-queue-runner/build-remote.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 6d73cf4b7..d86fc37f9 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -259,13 +259,10 @@ void State::buildRemote(ref destStore, basicDrv = BasicDerivation(*step->drv); for (auto & input : step->drv->inputDrvs) { auto drv2 = localStore->readDerivation(input.first); - auto hashes = staticOutputHashes(*localStore, drv2); + auto drv2Outputs = drv2.outputsAndOptPaths(*localStore); for (auto & name : input.second) { - if (settings.isExperimentalFeatureEnabled("ca-derivations")) { - auto inputRealisation = localStore->queryRealisation(DrvOutput{hashes.at(name), name}); - assert(inputRealisation); - basicDrv.inputSrcs.insert(inputRealisation->outPath); - } + auto inputPath = drv2Outputs.at(name); + basicDrv.inputSrcs.insert(*inputPath.second); } } } From 103607525dd1a9f834e66ad9c1032b7aff2b1d3e Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 18 Oct 2021 16:18:33 +0200 Subject: [PATCH 26/60] Make perlcritic happy --- t/content-addressed/basic.t | 1 + t/content-addressed/without-experimental-feature.t | 1 + 2 files changed, 2 insertions(+) diff --git a/t/content-addressed/basic.t b/t/content-addressed/basic.t index 28cce0df8..f19e5d6d4 100644 --- a/t/content-addressed/basic.t +++ b/t/content-addressed/basic.t @@ -1,5 +1,6 @@ use feature 'unicode_strings'; use strict; +use warnings; use Setup; my %ctx = test_init( diff --git a/t/content-addressed/without-experimental-feature.t b/t/content-addressed/without-experimental-feature.t index 0c8c84f8c..a37d138ec 100644 --- a/t/content-addressed/without-experimental-feature.t +++ b/t/content-addressed/without-experimental-feature.t @@ -1,5 +1,6 @@ use feature 'unicode_strings'; use strict; +use warnings; use Setup; my %ctx = test_init(); From a084efc7039a84c5409c71eb68706dfc3fdbd423 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 20 Oct 2021 10:28:05 +0200 Subject: [PATCH 27/60] Extract some parts of `State::buildRemote` into auxiliary functions Makes the whole thing marginally easier to read --- src/hydra-queue-runner/build-remote.cc | 110 +++++++++++++++---------- 1 file changed, 68 insertions(+), 42 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index d86fc37f9..6c647d91e 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -153,6 +153,71 @@ StorePaths reverseTopoSortPaths(const std::map & paths 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; + auto outputHashes = staticOutputHashes(store, drv); + if (!derivationHasKnownOutputPaths(drv.type())) { + 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 & input : drv.inputDrvs) { + auto drv2 = store.readDerivation(input.first); + auto drv2Outputs = drv2.outputsAndOptPaths(store); + for (auto & name : input.second) { + auto inputPath = drv2Outputs.at(name); + ret.inputSrcs.insert(*inputPath.second); + } + } + } + return ret; +} + +/** + * Get the newly built outputs, either from the remote if it supports it, or by + * introspecting the derivation if the remote is too old + */ +DrvOutputs getBuiltOutputs(Store & store, const int remoteVersion, FdSource & from, Derivation & drv) +{ + DrvOutputs builtOutputs; + if (GET_PROTOCOL_MINOR(remoteVersion) >= 6) { + builtOutputs + = worker_proto::read(store, from, Phantom {}); + } else { + // If the remote is too old to handle CA derivations, we can’t get this + // far anyways + assert(derivationHasKnownOutputPaths(drv.type())); + DerivationOutputsAndOptPaths drvOutputs + = drv.outputsAndOptPaths(store); + auto outputHashes = staticOutputHashes(store, 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 }; + builtOutputs.insert( + { drvOutput, Realisation { drvOutput, *outputPath } }); + } + } + return builtOutputs; +} void State::buildRemote(ref destStore, Machine::ptr machine, Step::ptr step, @@ -243,29 +308,7 @@ void State::buildRemote(ref destStore, outputs of the input derivations. */ updateStep(ssSendingInputs); - BasicDerivation basicDrv; - auto outputHashes = staticOutputHashes(*localStore, *step->drv); - if (!derivationHasKnownOutputPaths(step->drv->type())) { - auto maybeBasicDrv = step->drv->tryResolve(*localStore); - if (!maybeBasicDrv) - throw Error( - "the derivation '%s' can’t be resolved. It’s probably " - "missing some outputs", - localStore->printStorePath(step->drvPath)); - basicDrv = *maybeBasicDrv; - } else { - // If the derivation is a real `InputAddressed` derivation, we must - // resolve it manually to keep the original output paths - basicDrv = BasicDerivation(*step->drv); - for (auto & input : step->drv->inputDrvs) { - auto drv2 = localStore->readDerivation(input.first); - auto drv2Outputs = drv2.outputsAndOptPaths(*localStore); - for (auto & name : input.second) { - auto inputPath = drv2Outputs.at(name); - basicDrv.inputSrcs.insert(*inputPath.second); - } - } - } + BasicDerivation basicDrv = inlineInputDerivations(*localStore, *step->drv, step->drvPath); /* Ensure that the inputs exist in the destination store. This is a no-op for regular stores, but for the binary cache store, @@ -345,9 +388,6 @@ void State::buildRemote(ref destStore, result.stopTime = stop; } } - if (GET_PROTOCOL_MINOR(remoteVersion) >= 6) { - worker_proto::read(*localStore, from, Phantom {}); - } switch ((BuildResult::Status) res) { case BuildResult::Built: result.stepStatus = bsSuccess; @@ -405,22 +445,7 @@ void State::buildRemote(ref destStore, result.logFile = ""; } - DrvOutputs builtOutputs; - if (GET_PROTOCOL_MINOR(remoteVersion) >= 6) { - builtOutputs = worker_proto::read(*localStore, from, Phantom{}); - } else { - assert( - step->drv->type() != DerivationType::CAFloating && - step->drv->type() != DerivationType::DeferredInputAddressed - ); - auto outputMap = localStore->queryPartialDerivationOutputMap(step->drvPath); - for (auto & [outputName, outputPath] : outputMap) - if (outputPath) { - auto outputHash = outputHashes.at(outputName); - auto drvOutput = DrvOutput{outputHash, outputName}; - builtOutputs.insert({drvOutput, Realisation{drvOutput, *outputPath}}); - } - } + auto builtOutputs = getBuiltOutputs(*localStore, remoteVersion, from, *step->drv); StorePathSet outputs; for (auto & [_, realisation] : builtOutputs) outputs.insert(realisation.outPath); @@ -504,6 +529,7 @@ void State::buildRemote(ref destStore, /* Register the outputs of the newly built drv */ if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + auto outputHashes = staticOutputHashes(*localStore, *step->drv); for (auto & [outputId, realisation] : builtOutputs) { // Register the resolved drv output localStore->registerDrvOutput(realisation); From 46ae574648bc4cb39c5c38da19bc7b02e69642ff Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 19 Jan 2022 16:44:26 +0100 Subject: [PATCH 28/60] Fix build with Nix 2.6pre --- src/hydra-queue-runner/build-remote.cc | 2 +- src/hydra-queue-runner/queue-monitor.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 6c647d91e..e7495f33c 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -528,7 +528,7 @@ void State::buildRemote(ref destStore, } /* Register the outputs of the newly built drv */ - if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { auto outputHashes = staticOutputHashes(*localStore, *step->drv); for (auto & [outputId, realisation] : builtOutputs) { // Register the resolved drv output diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 09d9f591a..c039a18a2 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -477,7 +477,7 @@ Step::ptr State::createStep(ref destStore, missing.insert({{outputHash, outputName}, maybeOutputPath.second}); } } else { - settings.requireExperimentalFeature("ca-derivations"); + settings.requireExperimentalFeature(Xp::CaDerivations); if (!destStore->queryRealisation(DrvOutput{outputHash, outputName})) { valid = false; missing.insert({{outputHash, outputName}, std::nullopt}); @@ -493,7 +493,7 @@ Step::ptr State::createStep(ref destStore, for (auto & [i, maybePath] : missing) { if ((maybePath && localStore->isValidPath(*maybePath))) avail++; - else if (settings.isExperimentalFeatureEnabled("ca-derivations") && localStore->queryRealisation(i)) { + else if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && localStore->queryRealisation(i)) { maybePath = localStore->queryRealisation(i)->outPath; avail++; } else if (useSubstitutes && maybePath) { From cdff8067285336abbe4750ae15d0a7dcdda5fcb5 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 20 Jan 2022 06:35:43 +0100 Subject: [PATCH 29/60] Flake: Update --- flake.lock | 19 +++++++++---------- t/jobs/empty-dir-builder.sh | 2 ++ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/flake.lock b/flake.lock index fa71ceb5b..30519f6a7 100644 --- a/flake.lock +++ b/flake.lock @@ -3,16 +3,15 @@ "lowdown-src": { "flake": false, "locked": { - "lastModified": 1617481909, - "narHash": "sha256-SqnfOFuLuVRRNeVJr1yeEPJue/qWoCp5N6o5Kr///p4=", + "lastModified": 1633514407, + "narHash": "sha256-Dw32tiMjdK9t3ETl5fzGrutQTzh2rufgZV4A/BbxuD4=", "owner": "kristapsdz", "repo": "lowdown", - "rev": "148f9b2f586c41b7e36e73009db43ea68c7a1a4d", + "rev": "d2c2b44ff6c27b936ec27358a2653caaef8f73b8", "type": "github" }, "original": { "owner": "kristapsdz", - "ref": "VERSION_0_8_4", "repo": "lowdown", "type": "github" } @@ -23,11 +22,11 @@ "nixpkgs": "nixpkgs" }, "locked": { - "lastModified": 1628586117, - "narHash": "sha256-8hS4xy7fq3z9XZIMYm4sQi9SzhcYqEJfdbwgDePoWuc=", + "lastModified": 1642583127, + "narHash": "sha256-WyCL2SDApuIjQngO0UozOBI/iDUUoDW1QEQ/MUUu/Ec=", "owner": "NixOS", "repo": "nix", - "rev": "a6ba313a0aac3b6e2fef434cb42d190a0849238e", + "rev": "bc443511eb65420b51d10708e25427fe50de37a8", "type": "github" }, "original": { @@ -37,11 +36,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1624862269, - "narHash": "sha256-JFcsh2+7QtfKdJFoPibLFPLgIW6Ycnv8Bts9a7RYme0=", + "lastModified": 1632864508, + "narHash": "sha256-d127FIvGR41XbVRDPVvozUPQ/uRHbHwvfyKHwEt5xFM=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "f77036342e2b690c61c97202bf48f2ce13acc022", + "rev": "82891b5e2c2359d7e58d08849e4c89511ab94234", "type": "github" }, "original": { diff --git a/t/jobs/empty-dir-builder.sh b/t/jobs/empty-dir-builder.sh index addc7ef6b..dfd695a0e 100755 --- a/t/jobs/empty-dir-builder.sh +++ b/t/jobs/empty-dir-builder.sh @@ -1,3 +1,5 @@ #! /bin/sh +echo foo + mkdir $out From 8b34b2fe2e8ddb953775db5dd410e79bc5fd436f Mon Sep 17 00:00:00 2001 From: Andrea Ciceri Date: Sat, 25 Jun 2022 17:20:48 +0200 Subject: [PATCH 30/60] Don't query outputs in case of CA derivations --- src/hydra-eval-jobs/hydra-eval-jobs.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hydra-eval-jobs/hydra-eval-jobs.cc b/src/hydra-eval-jobs/hydra-eval-jobs.cc index f997b02b0..357b291dd 100644 --- a/src/hydra-eval-jobs/hydra-eval-jobs.cc +++ b/src/hydra-eval-jobs/hydra-eval-jobs.cc @@ -174,7 +174,7 @@ static void worker( if (auto drv = getDerivation(state, *v, false)) { - DrvInfo::Outputs outputs = drv->queryOutputs(); + DrvInfo::Outputs outputs = drv->queryOutputs(!settings.isExperimentalFeatureEnabled(Xp::CaDerivations)); if (drv->querySystem() == "unknown") throw EvalError("derivation must have a 'system' attribute"); From d8639ac4083d7e21a45e5ef3f438d4a0b3fc81cf Mon Sep 17 00:00:00 2001 From: Andrea Ciceri Date: Sat, 25 Jun 2022 17:45:02 +0200 Subject: [PATCH 31/60] Using new libstore method and fixes due to CA derivations --- src/hydra-queue-runner/build-remote.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index c180a9106..fbe8495ef 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -185,7 +185,7 @@ BasicDerivation inlineInputDerivations(Store & store, Derivation & drv, const St { BasicDerivation ret; auto outputHashes = staticOutputHashes(store, drv); - if (!derivationHasKnownOutputPaths(drv.type())) { + if (!drv.type().hasKnownOutputPaths()) { auto maybeBasicDrv = drv.tryResolve(store); if (!maybeBasicDrv) throw Error( @@ -222,7 +222,7 @@ DrvOutputs getBuiltOutputs(Store & store, const int remoteVersion, FdSource & fr } else { // If the remote is too old to handle CA derivations, we can’t get this // far anyways - assert(derivationHasKnownOutputPaths(drv.type())); + assert(drv.type().hasKnownOutputPaths()); DerivationOutputsAndOptPaths drvOutputs = drv.outputsAndOptPaths(store); auto outputHashes = staticOutputHashes(store, drv); @@ -354,10 +354,10 @@ void State::buildRemote(ref destStore, /* Copy the input closure. */ if (machine->isLocalhost()) { StorePathSet closure; - destStore->computeFSClosure(inputs, closure); + destStore->computeFSClosure(step->drv->inputSrcs, closure); copyPaths(*destStore, *localStore, closure, NoRepair, NoCheckSigs, NoSubstitute); } else { - copyClosureTo(machine->state->sendLock, *destStore, from, to, inputs, true); + copyClosureTo(machine->state->sendLock, *destStore, from, to, step->drv->inputSrcs, true); } auto now2 = std::chrono::steady_clock::now(); From de2fc58e0c6fa76e83b30224018cf4268fced772 Mon Sep 17 00:00:00 2001 From: Andrea Ciceri Date: Sat, 25 Jun 2022 17:45:44 +0200 Subject: [PATCH 32/60] `string` -> `std::string` --- src/hydra-queue-runner/hydra-queue-runner.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index b3e18dfea..339e23b13 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -363,7 +363,7 @@ void State::finishBuildStep(pqxx::work & txn, const RemoteResult & result, // 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()); + 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)) txn.exec_params0 From 0ef435e4ce604fa94f889aeb539ff6de9284bce0 Mon Sep 17 00:00:00 2001 From: Andrea Ciceri Date: Mon, 27 Jun 2022 15:35:34 +0200 Subject: [PATCH 33/60] Forgot to remove old comment in the merge --- src/hydra-queue-runner/queue-monitor.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 9292eed3f..bd7a44871 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -510,7 +510,6 @@ Step::ptr State::createStep(ref destStore, maybePath = localStore->queryRealisation(i)->outPath; avail++; } else if (useSubstitutes && maybePath) { - // TODO: Make work with CA derivations SubstitutablePathInfos infos; localStore->querySubstitutablePathInfos({{*maybePath, {}}}, infos); if (infos.size() == 1) From db89bbe6cf3e06b08ce2a0ca669958a42b7b6fe0 Mon Sep 17 00:00:00 2001 From: Andrea Ciceri Date: Mon, 27 Jun 2022 15:36:54 +0200 Subject: [PATCH 34/60] Fixed regression to #1126 --- src/hydra-queue-runner/build-remote.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index fbe8495ef..05ab67617 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -354,7 +354,7 @@ void State::buildRemote(ref destStore, /* Copy the input closure. */ if (machine->isLocalhost()) { StorePathSet closure; - destStore->computeFSClosure(step->drv->inputSrcs, closure); + destStore->computeFSClosure(basicDrv.inputSrcs, closure); copyPaths(*destStore, *localStore, closure, NoRepair, NoCheckSigs, NoSubstitute); } else { copyClosureTo(machine->state->sendLock, *destStore, from, to, step->drv->inputSrcs, true); From 8d52640cf1ee1ffb74b19216b77bfec7ad258d67 Mon Sep 17 00:00:00 2001 From: Andrea Ciceri Date: Thu, 30 Jun 2022 15:26:52 +0200 Subject: [PATCH 35/60] DB model changed Now `BuildStepOutputs` has a new boolean column `contentAddressed`. Also regenerated the relative Perl. --- src/lib/Hydra/Schema/Result/BuildOutputs.pm | 8 ++++---- src/lib/Hydra/Schema/Result/BuildStepOutputs.pm | 13 ++++++++++--- src/sql/hydra.sql | 15 ++++++++------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/lib/Hydra/Schema/Result/BuildOutputs.pm b/src/lib/Hydra/Schema/Result/BuildOutputs.pm index 9fc4f7c78..3997b4976 100644 --- a/src/lib/Hydra/Schema/Result/BuildOutputs.pm +++ b/src/lib/Hydra/Schema/Result/BuildOutputs.pm @@ -49,7 +49,7 @@ __PACKAGE__->table("buildoutputs"); =head2 path data_type: 'text' - is_nullable: 0 + is_nullable: 1 =cut @@ -59,7 +59,7 @@ __PACKAGE__->add_columns( "name", { data_type => "text", is_nullable => 0 }, "path", - { data_type => "text", is_nullable => 0 }, + { data_type => "text", is_nullable => 1 }, ); =head1 PRIMARY KEY @@ -94,8 +94,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2021-08-26 12:02:36 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:gU+kZ6A0ISKpaXGRGve8mg +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2022-06-30 12:02:32 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:Jsabm3YTcI7YvCuNdKP5Ng my %hint = ( columns => [ diff --git a/src/lib/Hydra/Schema/Result/BuildStepOutputs.pm b/src/lib/Hydra/Schema/Result/BuildStepOutputs.pm index 016a35fe2..42392190f 100644 --- a/src/lib/Hydra/Schema/Result/BuildStepOutputs.pm +++ b/src/lib/Hydra/Schema/Result/BuildStepOutputs.pm @@ -55,6 +55,11 @@ __PACKAGE__->table("buildstepoutputs"); =head2 path data_type: 'text' + is_nullable: 1 + +=head2 contentaddressed + + data_type: 'boolean' is_nullable: 0 =cut @@ -67,7 +72,9 @@ __PACKAGE__->add_columns( "name", { data_type => "text", is_nullable => 0 }, "path", - { data_type => "text", is_nullable => 0 }, + { data_type => "text", is_nullable => 1 }, + "contentaddressed", + { data_type => "boolean", is_nullable => 0 }, ); =head1 PRIMARY KEY @@ -119,8 +126,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2021-08-26 12:02:36 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:gxp8rOjpRVen4YbIjomHTw +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2022-06-30 12:02:32 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:Bad70CRTt7zb2GGuRoQ++Q # You can replace this text with custom code or comments, and it will be preserved on regeneration diff --git a/src/sql/hydra.sql b/src/sql/hydra.sql index e94579720..02159fe8e 100644 --- a/src/sql/hydra.sql +++ b/src/sql/hydra.sql @@ -300,13 +300,14 @@ create table BuildSteps ( create table BuildStepOutputs ( - build integer not null, - stepnr integer not null, - name text not null, - path text, - primary key (build, stepnr, name), - foreign key (build) references Builds(id) on delete cascade, - foreign key (build, stepnr) references BuildSteps(build, stepnr) on delete cascade + build integer not null, + stepnr integer not null, + name text not null, + path text, + contentAddressed boolean not null, + primary key (build, stepnr, name), + foreign key (build) references Builds(id) on delete cascade, + foreign key (build, stepnr) references BuildSteps(build, stepnr) on delete cascade ); From 954e41fbe26373a59e2cb21441c1d2fba352ea6f Mon Sep 17 00:00:00 2001 From: Andrea Ciceri Date: Thu, 30 Jun 2022 15:27:52 +0200 Subject: [PATCH 36/60] Keep track if step build outputs are CA or not --- src/hydra-queue-runner/hydra-queue-runner.cc | 11 ++++++----- src/hydra-queue-runner/queue-monitor.cc | 2 +- src/hydra-queue-runner/state.hh | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 339e23b13..7b3b39c37 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -320,8 +320,8 @@ unsigned int State::createBuildStep(pqxx::work & txn, time_t startTime, BuildID for (auto& [name, output] : localStore->queryPartialDerivationOutputMap(step->drvPath)) txn.exec_params0 - ("insert into BuildStepOutputs (build, stepnr, name, path) values ($1, $2, $3, $4)", - buildId, stepNr, name, output ? localStore->printStorePath(*output) : ""); + ("insert into BuildStepOutputs (build, stepnr, name, path, contentAddressed) values ($1, $2, $3, $4, $5)", + buildId, stepNr, name, output ? localStore->printStorePath(*output) : "", step->drv->type().isCA()); if (status == bsBusy) txn.exec(fmt("notify step_started, '%d\t%d'", buildId, stepNr)); @@ -374,7 +374,7 @@ void State::finishBuildStep(pqxx::work & txn, const RemoteResult & result, 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); @@ -393,9 +393,10 @@ int State::createSubstitutionStep(pqxx::work & txn, time_t startTime, time_t sto if (r.affected_rows() == 0) goto restart; txn.exec_params0 - ("insert into BuildStepOutputs (build, stepnr, name, path) values ($1, $2, $3, $4)", + ("insert into BuildStepOutputs (build, stepnr, name, path, contentAddressed) values ($1, $2, $3, $4, $5)", build->id, stepNr, outputName, - localStore->printStorePath(storePath)); + localStore->printStorePath(storePath), + drv.type().isCA()); return stepNr; } diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index bd7a44871..2c538b672 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -545,7 +545,7 @@ Step::ptr State::createStep(ref 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(); } diff --git a/src/hydra-queue-runner/state.hh b/src/hydra-queue-runner/state.hh index eac0e9a57..eaa117f18 100644 --- a/src/hydra-queue-runner/state.hh +++ b/src/hydra-queue-runner/state.hh @@ -483,7 +483,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); From 3fdc9e491dc7d33bc09ef4f33984f87c8f478aa2 Mon Sep 17 00:00:00 2001 From: Andrea Ciceri Date: Thu, 30 Jun 2022 15:28:22 +0200 Subject: [PATCH 37/60] Improved UI to correctly handle CA derivations --- src/lib/Hydra/Controller/Build.pm | 12 ++++++++++++ src/root/build.tt | 23 +++++++++++++++-------- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm index c85c720c0..2d68d0c5f 100644 --- a/src/lib/Hydra/Controller/Build.pm +++ b/src/lib/Hydra/Controller/Build.pm @@ -114,6 +114,18 @@ sub build_GET { $c->stash->{steps} = [$build->buildsteps->search({}, {order_by => "stepnr desc"})]; + $c->stash->{contentAddressed} = 0; + # Hydra marks single outputs as CA but currently in Nix only derivations + # can be CA (and *all* their outputs are CA). + # So the next check (which assumes that if a step's output is CA then + # all the other outptus and the whole derivation are CA) is safe. + foreach my $step (@{$c->stash->{steps}}) { + if ($step->buildstepoutputs->search({contentaddressed => 1})->count > 0) { + $c->stash->{contentAddressed} = 1; + last; + } + } + $c->stash->{binaryCachePublicUri} = $c->config->{binary_cache_public_uri}; } diff --git a/src/root/build.tt b/src/root/build.tt index 027ce3e4d..d262ed546 100644 --- a/src/root/build.tt +++ b/src/root/build.tt @@ -20,8 +20,13 @@ END; %] [% BLOCK renderOutputs %] - [% start=1; FOREACH output IN outputs %] - [% IF !start %],
[% END; start=0; output.path %] + [% start=1; FOREACH output IN step.buildstepoutputs %] + [% IF !start %],
[% END; start=0; %] + [% IF step.status != 0 && output.contentaddressed %] + [% output.name %] + [% ELSE %] + [% output.path %] + [% END %] [% END %] [% END %] @@ -40,9 +45,9 @@ END; [% step.stepnr %] [% IF step.type == 0 %] - Build of [% INCLUDE renderOutputs outputs=step.buildstepoutputs %] + Build of [% INCLUDE renderOutputs step=step %] [% ELSE %] - Substitution of [% INCLUDE renderOutputs outputs=step.buildstepoutputs %] + Substitution of [% INCLUDE renderOutputs step=step %] [% END %] @@ -381,10 +386,12 @@ END; Derivation store path: [% build.drvpath %] - - Output store paths: - [% INCLUDE renderOutputs outputs=build.buildoutputs %] - + [% IF !contentAddressed || step.status == 0 %] + + Output store paths: + [% INCLUDE renderOutputs step=step %] + + [% END %] [% chartsURL = c.uri_for('/job' build.project.name build.jobset.name build.job) _ "#tabs-charts" %] [% IF build.finished && build.closuresize %] From 391ada95ec2d3398a529b1ad29750a8bc88cd63f Mon Sep 17 00:00:00 2001 From: Andrea Ciceri Date: Sat, 2 Jul 2022 10:10:15 +0200 Subject: [PATCH 38/60] "Content addressed" field in builds details --- src/root/build.tt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/root/build.tt b/src/root/build.tt index d262ed546..6c180cdfe 100644 --- a/src/root/build.tt +++ b/src/root/build.tt @@ -385,6 +385,16 @@ END; Derivation store path: [% build.drvpath %] + + + Content addressed: + + [% IF contentAddressed %] + Yes + [% ELSE %] + No + [% END %] + [% IF !contentAddressed || step.status == 0 %] From d549a178ef8c1d390b85dd5a59ec767a705972c7 Mon Sep 17 00:00:00 2001 From: Alexander Sosedkin Date: Thu, 3 Aug 2023 01:52:55 +0200 Subject: [PATCH 39/60] Fix compilation against Nix 2.16 and 2.17: ca-derivations followup --- src/hydra-eval-jobs/hydra-eval-jobs.cc | 4 ++-- src/hydra-queue-runner/build-remote.cc | 8 ++++---- src/hydra-queue-runner/queue-monitor.cc | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/hydra-eval-jobs/hydra-eval-jobs.cc b/src/hydra-eval-jobs/hydra-eval-jobs.cc index f01b3d6e7..2366a1c4b 100644 --- a/src/hydra-eval-jobs/hydra-eval-jobs.cc +++ b/src/hydra-eval-jobs/hydra-eval-jobs.cc @@ -175,7 +175,7 @@ static void worker( if (auto drv = getDerivation(state, *v, false)) { - DrvInfo::Outputs outputs = drv->queryOutputs(!settings.isExperimentalFeatureEnabled(Xp::CaDerivations)); + DrvInfo::Outputs outputs = drv->queryOutputs(!experimentalFeatureSettings.isEnabled(Xp::CaDerivations)); if (drv->querySystem() == "unknown") throw EvalError("derivation must have a 'system' attribute"); @@ -236,7 +236,7 @@ static void worker( } nlohmann::json out; - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) + if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) for (auto & j : outputs) out[j.first] = ""; else diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index f0edc9266..2b9eed7c5 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -217,12 +217,12 @@ BasicDerivation inlineInputDerivations(Store & store, Derivation & drv, const St * Get the newly built outputs, either from the remote if it supports it, or by * introspecting the derivation if the remote is too old */ -DrvOutputs getBuiltOutputs(Store & store, const int remoteVersion, FdSource & from, Derivation & drv) +DrvOutputs getBuiltOutputs(Store & store, const int remoteVersion, WorkerProto::ReadConn & from, Derivation & drv) { DrvOutputs builtOutputs; if (GET_PROTOCOL_MINOR(remoteVersion) >= 6) { builtOutputs - = worker_proto::read(store, from, Phantom {}); + = WorkerProto::Serialise::read(store, from); } else { // If the remote is too old to handle CA derivations, we can’t get this // far anyways @@ -479,7 +479,7 @@ void State::buildRemote(ref destStore, result.logFile = ""; } - auto builtOutputs = getBuiltOutputs(*localStore, remoteVersion, from, *step->drv); + auto builtOutputs = getBuiltOutputs(*localStore, remoteVersion, rconn, *step->drv); StorePathSet outputs; for (auto & [_, realisation] : builtOutputs) outputs.insert(realisation.outPath); @@ -562,7 +562,7 @@ void State::buildRemote(ref destStore, } /* Register the outputs of the newly built drv */ - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { + if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { auto outputHashes = staticOutputHashes(*localStore, *step->drv); for (auto & [outputId, realisation] : builtOutputs) { // Register the resolved drv output diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index dd038c5a1..565c97374 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -490,7 +490,7 @@ Step::ptr State::createStep(ref destStore, missing.insert({{outputHash, outputName}, maybeOutputPath.second}); } } else { - settings.requireExperimentalFeature(Xp::CaDerivations); + experimentalFeatureSettings.require(Xp::CaDerivations); if (!destStore->queryRealisation(DrvOutput{outputHash, outputName})) { valid = false; missing.insert({{outputHash, outputName}, std::nullopt}); @@ -506,7 +506,7 @@ Step::ptr State::createStep(ref destStore, for (auto & [i, maybePath] : missing) { if ((maybePath && localStore->isValidPath(*maybePath))) avail++; - else if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && localStore->queryRealisation(i)) { + else if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && localStore->queryRealisation(i)) { maybePath = localStore->queryRealisation(i)->outPath; avail++; } else if (useSubstitutes && maybePath) { From d1c1eaa269950eabe76ca861d6db9be15b545db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charlotte=20=F0=9F=A6=9D=20Delenk?= Date: Sat, 2 Jul 2022 08:27:19 +0100 Subject: [PATCH 40/60] Copy the derivation input closure to the remote builder and the derivation output to the local store --- src/hydra-queue-runner/build-remote.cc | 42 ++++++++++++++------------ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 2b9eed7c5..1c530a4cb 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -364,6 +364,7 @@ void State::buildRemote(ref destStore, copyPaths(*destStore, *localStore, closure, NoRepair, NoCheckSigs, NoSubstitute); } else { copyClosureTo(machine->state->sendLock, *destStore, from, to, step->drv->inputSrcs, true); + copyClosureTo(machine->state->sendLock, *destStore, from, to, basicDrv.inputSrcs, true); } auto now2 = std::chrono::steady_clock::now(); @@ -534,26 +535,27 @@ void State::buildRemote(ref destStore, for (auto & path : pathsSorted) { auto & info = infos.find(path)->second; - /* 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. */ - to << ServeProto::Command::DumpStorePath << localStore->printStorePath(path); - to.flush(); - - TeeSource tee(from, sink); - extractNarData(tee, localStore->printStorePath(path), narMembers); - }); - - destStore->addToStore(info, *source2, NoRepair, NoCheckSigs); + for (auto & store : {&*destStore, &*localStore}) { + /* 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. */ + to << ServeProto::Command::DumpStorePath << localStore->printStorePath(path); + to.flush(); + + TeeSource tee(from, sink); + extractNarData(tee, localStore->printStorePath(path), narMembers); + }); + store->addToStore(info, *source2, NoRepair, NoCheckSigs); + } } auto now2 = std::chrono::steady_clock::now(); From b4dd816ddec5add431ebdcc96866d976d90a911b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charlotte=20=F0=9F=A6=9D=20Delenk?= Date: Tue, 30 May 2023 07:54:09 +0100 Subject: [PATCH 41/60] require buildoutputs to be present before displaying it --- src/lib/Hydra/Controller/Build.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm index 2cd3abcec..c9c19a3b7 100644 --- a/src/lib/Hydra/Controller/Build.pm +++ b/src/lib/Hydra/Controller/Build.pm @@ -86,7 +86,7 @@ sub build_GET { : 1; $c->stash->{drvAvailable} = isValidPath $build->drvpath; - if ($build->finished && $build->iscachedbuild) { + if ($build->finished && $build->iscachedbuild && defined(($build->buildoutputs)[0]->path)) { my $path = ($build->buildoutputs)[0]->path or die; my $cachedBuildStep = findBuildStepByOutPath($self, $c, $path); if (defined $cachedBuildStep) { From c257abeb7cd1ce27424803aaf971ca1cfbedecae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charlotte=20=F0=9F=A6=9D=20Delenk?= Date: Tue, 30 May 2023 08:29:43 +0100 Subject: [PATCH 42/60] make it work --- src/lib/Hydra/Controller/Build.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/Hydra/Controller/Build.pm b/src/lib/Hydra/Controller/Build.pm index c9c19a3b7..b7ef4cdd7 100644 --- a/src/lib/Hydra/Controller/Build.pm +++ b/src/lib/Hydra/Controller/Build.pm @@ -86,8 +86,8 @@ sub build_GET { : 1; $c->stash->{drvAvailable} = isValidPath $build->drvpath; - if ($build->finished && $build->iscachedbuild && defined(($build->buildoutputs)[0]->path)) { - my $path = ($build->buildoutputs)[0]->path or die; + if ($build->finished && $build->iscachedbuild) { + my $path = ($build->buildoutputs)[0]->path or ""; my $cachedBuildStep = findBuildStepByOutPath($self, $c, $path); if (defined $cachedBuildStep) { $c->stash->{cachedBuild} = $cachedBuildStep->build; From 9e528b2faee8a2bcdb25290ff4fcbc0423c9fbd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sandro=20J=C3=A4ckel?= Date: Mon, 4 Dec 2023 15:30:11 +0100 Subject: [PATCH 43/60] Add realisations to binary cache for ca-derivations --- src/lib/Hydra/Controller/Root.pm | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/lib/Hydra/Controller/Root.pm b/src/lib/Hydra/Controller/Root.pm index c6843d296..548cfac34 100644 --- a/src/lib/Hydra/Controller/Root.pm +++ b/src/lib/Hydra/Controller/Root.pm @@ -18,6 +18,8 @@ use Net::Prometheus; use Types::Standard qw/StrMatch/; use constant NARINFO_REGEX => qr{^([a-z0-9]{32})\.narinfo$}; +# e.g.: https://hydra.example.com/realisations/sha256:a62128132508a3a32eef651d6467695944763602f226ac630543e947d9feb140!out.doi +use constant REALISATIONS_REGEX => qr{^(sha256:[a-z0-9]{64}![a-z]+)\.doi$}; # Put this controller at top-level. __PACKAGE__->config->{namespace} = ''; @@ -355,6 +357,33 @@ sub nix_cache_info :Path('nix-cache-info') :Args(0) { } +sub realisations :Path('realisations') :Args(StrMatch[REALISATIONS_REGEX]) { + my ($self, $c, $realisation) = @_; + + if (!isLocalStore) { + notFound($c, "There is no binary cache here."); + } + + else { + my ($rawDrvOutput) = $realisation =~ REALISATIONS_REGEX; + my $rawRealisation = queryRawRealisation($rawDrvOutput); + + if (!$rawRealisation) { + $c->response->status(404); + $c->response->content_type('text/plain'); + $c->stash->{plain}->{data} = "does not exist\n"; + $c->forward('Hydra::View::Plain'); + setCacheHeaders($c, 60 * 60); + return; + } + + $c->response->content_type('text/plain'); + $c->stash->{plain}->{data} = $rawRealisation; + $c->forward('Hydra::View::Plain'); + } +} + + sub narinfo :Path :Args(StrMatch[NARINFO_REGEX]) { my ($self, $c, $narinfo) = @_; From 040d734f8c5de96c3473b7c1b89b9a181a1d45ab Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 4 Dec 2023 13:07:04 -0500 Subject: [PATCH 44/60] Restore accidentally deleted blank line --- src/hydra-queue-runner/build-remote.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 9b5fa5fcf..fe8fbe947 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -536,6 +536,7 @@ void RemoteResult::updateWithBuildResult(const nix::BuildResult & buildResult) } + void State::buildRemote(ref destStore, Machine::ptr machine, Step::ptr step, const BuildOptions & buildOptions, From 5357d8823b6aa55b821f7957ae4d7328ce8c6c80 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 4 Dec 2023 14:18:45 -0500 Subject: [PATCH 45/60] Remove stray whitespace in test --- t/content-addressed/basic.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/content-addressed/basic.t b/t/content-addressed/basic.t index f19e5d6d4..fc38c24d3 100644 --- a/t/content-addressed/basic.t +++ b/t/content-addressed/basic.t @@ -26,7 +26,7 @@ my $project = $db->resultset('Projects')->create({name => "tests", displayname = my $jobset = createBaseJobset("content-addressed", "content-addressed.nix", $ctx{jobsdir}); -ok(evalSucceeds($jobset), "Evaluating jobs/content-addressed.nix should exit with return code 0"); +ok(evalSucceeds($jobset), "Evaluating jobs/content-addressed.nix should exit with return code 0"); is(nrQueuedBuildsForJobset($jobset), 4, "Evaluating jobs/content-addressed.nix should result in 4 builds"); for my $build (queuedBuildsForJobset($jobset)) { From 3b3050cfddf6f30971d01a8fb93f1ea635f8e94f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 4 Dec 2023 14:45:29 -0500 Subject: [PATCH 46/60] Forgot to remove this duplicated code --- src/hydra-queue-runner/build-remote.cc | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index fe8fbe947..7c82e9327 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -265,16 +265,6 @@ static BasicDerivation sendInputs( { BasicDerivation basicDrv = inlineInputDerivations(localStore, *step.drv, step.drvPath); - for (const auto & [drvPath, node] : step.drv->inputDrvs.map) { - auto drv2 = localStore.readDerivation(drvPath); - for (auto & name : node.value) { - if (auto i = get(drv2.outputs, name)) { - auto outPath = i->path(localStore, drv2.name, name); - basicDrv.inputSrcs.insert(*outPath); - } - } - } - /* Ensure that the inputs exist in the destination store. This is a no-op for regular stores, but for the binary cache store, this will copy the inputs to the binary cache from the local From 972f995391d9dda788b1c8d1e81a7be5c563ebed Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 4 Dec 2023 15:28:44 -0500 Subject: [PATCH 47/60] Style, indenting --- src/hydra-eval-jobs/hydra-eval-jobs.cc | 17 ++++++----- src/hydra-queue-runner/build-remote.cc | 26 ++++++++--------- src/hydra-queue-runner/build-result.cc | 2 +- src/hydra-queue-runner/hydra-queue-runner.cc | 30 ++++++++++---------- src/hydra-queue-runner/queue-monitor.cc | 29 +++++++++---------- 5 files changed, 53 insertions(+), 51 deletions(-) diff --git a/src/hydra-eval-jobs/hydra-eval-jobs.cc b/src/hydra-eval-jobs/hydra-eval-jobs.cc index 65864ca8c..d007189d6 100644 --- a/src/hydra-eval-jobs/hydra-eval-jobs.cc +++ b/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(!experimentalFeatureSettings.isEnabled(Xp::CaDerivations)); + // 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,11 @@ static void worker( } nlohmann::json out; - if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) - for (auto & j : outputs) - out[j.first] = ""; - else - for (auto & j : outputs) - out[j.first] = state.store->printStorePath(*j.second); + for (auto & [outputName, optOutputPath] : outputs) + if (optOutputPath) + out[outputName] = state.store->printStorePath(*optOutputPath); + else + out[outputName] = ""; job["outputs"] = std::move(out); reply["job"] = std::move(job); } diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 7c82e9327..26f6d63fc 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -676,19 +676,19 @@ void State::buildRemote(ref destStore, /* 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 - 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); - } + 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); + } } /* Shut down the connection. */ diff --git a/src/hydra-queue-runner/build-result.cc b/src/hydra-queue-runner/build-result.cc index 702cb4d0c..ffdc37b7d 100644 --- a/src/hydra-queue-runner/build-result.cc +++ b/src/hydra-queue-runner/build-result.cc @@ -107,7 +107,7 @@ 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] : derivationOutputs) { + for (auto & [name, output] : derivationOutputs) { BuildProduct product; product.path = store->printStorePath(output); product.type = "nix-build"; diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index c8b352d2e..9ff744d62 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -354,15 +354,15 @@ void State::finishBuildStep(pqxx::work & txn, const RemoteResult & result, 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()); - // If we've finished building, all the paths should be known - for (auto& [name, output] : localStore->queryDerivationOutputMap(drvPath)) - txn.exec_params0 - ("update BuildStepOutputs set path = $4 where build = $1 and stepnr = $2 and name = $3", - buildId, stepNr, name, localStore->printStorePath(output)); + // 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()); + // If we've finished building, all the paths should be known + for (auto& [name, output] : localStore->queryDerivationOutputMap(drvPath)) + txn.exec_params0 + ("update BuildStepOutputs set path = $4 where build = $1 and stepnr = $2 and name = $3", + buildId, stepNr, name, localStore->printStorePath(output)); } } @@ -471,12 +471,12 @@ void State::markSucceededBuild(pqxx::work & txn, Build::ptr build, 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 + ("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); diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 8c8fe1698..cee5b6cc6 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -238,7 +238,7 @@ bool State::getQueuedBuilds(Connection & conn, BuildOutput res = getBuildOutputCached(conn, destStore, build->drvPath); for (auto & i : localStore->queryDerivationOutputMap(build->drvPath)) - addRoot(i.second); + addRoot(i.second); { auto mc = startDbUpdate(); @@ -481,21 +481,20 @@ 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)) { - 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}); + for (auto &[outputName, maybeOutputPath] : step->drv->outputsAndOptPaths(*destStore)) { + 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}); + } } - } } /* Try to copy the missing paths from the local store or from From 513f5847e9e5203530570e24ad73f653e97a8029 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 4 Dec 2023 16:07:20 -0500 Subject: [PATCH 48/60] Two more style nits --- src/hydra-queue-runner/hydra-queue-runner.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 9ff744d62..0307c9a7d 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] : localStore->queryPartialDerivationOutputMap(step->drvPath)) txn.exec_params0 ("insert into BuildStepOutputs (build, stepnr, name, path, contentAddressed) values ($1, $2, $3, $4, $5)", buildId, stepNr, name, output ? localStore->printStorePath(*output) : "", step->drv->type().isCA()); @@ -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] : localStore->queryDerivationOutputMap(drvPath)) txn.exec_params0 ("update BuildStepOutputs set path = $4 where build = $1 and stepnr = $2 and name = $3", buildId, stepNr, name, localStore->printStorePath(output)); From 30c0619a21aa038fd0ef960bc6b3d17cf34d761f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 4 Dec 2023 18:06:52 -0500 Subject: [PATCH 49/60] Fix indentation --- src/hydra-queue-runner/hydra-queue-runner.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 0307c9a7d..041bf3d2e 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -313,9 +313,9 @@ 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)) - txn.exec_params0 - ("insert into BuildStepOutputs (build, stepnr, name, path, contentAddressed) values ($1, $2, $3, $4, $5)", - buildId, stepNr, name, output ? localStore->printStorePath(*output) : "", step->drv->type().isCA()); + txn.exec_params0 + ("insert into BuildStepOutputs (build, stepnr, name, path, contentAddressed) values ($1, $2, $3, $4, $5)", + buildId, stepNr, name, output ? localStore->printStorePath(*output) : "", step->drv->type().isCA()); if (status == bsBusy) txn.exec(fmt("notify step_started, '%d\t%d'", buildId, stepNr)); From 049f608ba24208e502d6b9488f65a4738d189407 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 7 Dec 2023 15:07:26 -0500 Subject: [PATCH 50/60] Revert "Do not copy for both stores for now" As the revert commit's description says, we only want to remove this in the preperatory branch, for https://github.com/NixOS/hydra/pull/1316. In the main branch, for https://github.com/NixOS/hydra/pull/875, the change shoudl be put back. This revert puts it back. This reverts commit 2ee0068fdca7d9fb8bef23b3671a86b8f0fcff07. --- src/hydra-queue-runner/build-remote.cc | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 774842442..1d0bcc35b 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -425,6 +425,20 @@ static void copyPathFromRemote( const ValidPathInfo & info ) { + // Why both stores? @thufschmitt says: + // + // > I think it's an easy (and terribly inefficient 😬) way of + // making sure that `localStore.queryRealisations` will succeed + // (which we IIRC we need later to get back some metadata about the + // path to put it in the db). + // > + // > To be honest, we shouldn't do that but instead carry the needed + // metadata in memory until the point where we need it (but that can + // come later once we're confident that this is at least correct) + // + // TODO make the above change to avoid copying excess data back and + // forth. + for (auto * store : {&destStore, &localStore}) { /* Receive the NAR from the remote and add it to the destination store. Meanwhile, extract all the info from the NAR that getBuildOutput() needs. */ @@ -444,7 +458,8 @@ static void copyPathFromRemote( extractNarData(tee, localStore.printStorePath(info.path), narMembers); }); - destStore.addToStore(info, *source2, NoRepair, NoCheckSigs); + store->addToStore(info, *source2, NoRepair, NoCheckSigs); + } } static void copyPathsFromRemote( From b7557840f497caa3f976934887590b80be28bfa9 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 10 Dec 2023 17:31:16 -0500 Subject: [PATCH 51/60] Tighten logic in build success case Rather than maybe set a root, assert than we have an output path. Even in the CA case, we should always know by this point. --- src/hydra-queue-runner/builder.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/hydra-queue-runner/builder.cc b/src/hydra-queue-runner/builder.cc index 429839419..5cdf2fc26 100644 --- a/src/hydra-queue-runner/builder.cc +++ b/src/hydra-queue-runner/builder.cc @@ -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] : localStore->queryPartialDerivationOutputMap(step->drvPath)) { + 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 From 7128f396618a7ca59ef30e5f76e5384eed19f932 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 10 Dec 2023 17:32:01 -0500 Subject: [PATCH 52/60] Clean up for loop slightly --- src/hydra-queue-runner/queue-monitor.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 049179326..6357e68e2 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, _] : localStore->queryPartialDerivationOutputMap(ex.step->drvPath)) { 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; From ea0f14d5c78a9d726a4313101fa53fc263673ff7 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 10 Dec 2023 17:51:17 -0500 Subject: [PATCH 53/60] Simplify build-remote logic a little bit We had an old vs new special case were we did not need one. --- src/hydra-queue-runner/build-remote.cc | 51 ++++++++------------------ 1 file changed, 16 insertions(+), 35 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index 0970e2116..c7fc58125 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(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, From 9c6c5ecdbb11232474eb3535c4a75df77d97240a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 10 Dec 2023 18:36:58 -0500 Subject: [PATCH 54/60] Clean up `queue-monitor.cc` a bit It is hopefully a bit more robust now, and actually closer to master. --- src/hydra-queue-runner/queue-monitor.cc | 93 ++++++++++++++----------- 1 file changed, 53 insertions(+), 40 deletions(-) diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index 6357e68e2..866d63f22 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -503,14 +503,24 @@ Step::ptr State::createStep(ref destStore, size_t avail = 0; for (auto & [i, maybePath] : missing) { - if ((maybePath && localStore->isValidPath(*maybePath))) - avail++; - else if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && localStore->queryRealisation(i)) { - maybePath = localStore->queryRealisation(i)->outPath; + // 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 (useSubstitutes && maybePath) { + else if (useSubstitutes) { SubstitutablePathInfos infos; - localStore->querySubstitutablePathInfos({{*maybePath, {}}}, infos); + localStore->querySubstitutablePathInfos({{path, {}}}, infos); if (infos.size() == 1) avail++; } @@ -518,44 +528,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); - - 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); + 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; + + try { + time_t startTime = time(0); + + 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. + } - time_t stopTime = time(0); + copyClosure(*localStore, *destStore, + StorePathSet { path }, + NoRepair, CheckSigs, NoSubstitute); - { - auto mc = startDbUpdate(); - pqxx::work txn(conn); - createSubstitutionStep(txn, startTime, stopTime, build, drvPath, *(step->drv), "out", *path); - txn.commit(); - } + time_t stopTime = time(0); - } catch (Error & e) { - printError("while copying/substituting output ‘%s’ of ‘%s’: %s", - localStore->printStorePath(*path), - localStore->printStorePath(drvPath), - e.what()); - valid = false; - break; + { + 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; } } } From 2359db89a7252e39920fa021b57af99b10a5bcfb Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 11 Dec 2023 12:38:47 -0500 Subject: [PATCH 55/60] flake.lock: Update to include unrelease 2.19 backports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need better `evalStore` support. Flake lock file updates: • Updated input 'nix': 'github:NixOS/nix/50f8f1c8bc019a4c0fd098b9ac674b94cfc6af0d' (2023-11-27) → 'github:NixOS/nix/ae451e2247b18be6bd36b9d85e41b632e774f40b' (2023-12-11) --- flake.lock | 8 ++++---- flake.nix | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/flake.lock b/flake.lock index 2871e70a1..e7f9224cb 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 5a5aef553..8280e076a 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 }: From 0b3653216315670b7d4342bbc7d9b6e2516a0998 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 10 Dec 2023 16:31:29 -0500 Subject: [PATCH 56/60] Use `destStore` realisations to resolve CA derivations In particuilar, make all - `tryResolve` - `queryerivationOutputMap` - `queryPartialDerivationOutputMap` look in dest store. The local store will just be used for the derivations themselves. Progress towards not keeping outputs and realisations in the local store at all. --- src/hydra-queue-runner/build-remote.cc | 4 +++- src/hydra-queue-runner/builder.cc | 4 ++-- src/hydra-queue-runner/hydra-queue-runner.cc | 4 ++-- src/hydra-queue-runner/queue-monitor.cc | 22 +++++++------------- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index c7fc58125..a911972d0 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -236,7 +236,7 @@ static BasicDerivation sendInputs( to do that, however, but we would not use it here.) */ BasicDerivation basicDrv = ({ - auto maybeBasicDrv = step.drv->tryResolve(localStore); + auto maybeBasicDrv = step.drv->tryResolve(destStore, &localStore); if (!maybeBasicDrv) throw Error( "the derivation '%s' can’t be resolved. It’s probably " @@ -332,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; diff --git a/src/hydra-queue-runner/builder.cc b/src/hydra-queue-runner/builder.cc index 5cdf2fc26..b1b58c05f 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,7 +277,7 @@ State::StepResult State::doBuildStep(nix::ref destStore, assert(stepNr); - for (auto & [outputName, optOutputPath] : localStore->queryPartialDerivationOutputMap(step->drvPath)) { + 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", diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 041bf3d2e..8025fc123 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, contentAddressed) values ($1, $2, $3, $4, $5)", buildId, stepNr, name, output ? localStore->printStorePath(*output) : "", step->drv->type().isCA()); @@ -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 866d63f22..a4d9b4dce 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -192,7 +192,7 @@ bool State::getQueuedBuilds(Connection & conn, if (!res[0].is_null()) propagatedFrom = res[0].as(); if (!propagatedFrom) { - for (auto & [outputName, _] : 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), @@ -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 From 17c8d51c37b3a6edf8af2ca92dc0a8f6c8a77e1b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 10 Dec 2023 16:38:03 -0500 Subject: [PATCH 57/60] Don't write things to the local store that don't belong No outputs, no resolutions --- src/hydra-queue-runner/build-remote.cc | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/hydra-queue-runner/build-remote.cc b/src/hydra-queue-runner/build-remote.cc index a911972d0..65f9f3fb9 100644 --- a/src/hydra-queue-runner/build-remote.cc +++ b/src/hydra-queue-runner/build-remote.cc @@ -397,20 +397,6 @@ static void copyPathFromRemote( const ValidPathInfo & info ) { - // Why both stores? @thufschmitt says: - // - // > I think it's an easy (and terribly inefficient 😬) way of - // making sure that `localStore.queryRealisations` will succeed - // (which we IIRC we need later to get back some metadata about the - // path to put it in the db). - // > - // > To be honest, we shouldn't do that but instead carry the needed - // metadata in memory until the point where we need it (but that can - // come later once we're confident that this is at least correct) - // - // TODO make the above change to avoid copying excess data back and - // forth. - for (auto * store : {&destStore, &localStore}) { /* Receive the NAR from the remote and add it to the destination store. Meanwhile, extract all the info from the NAR that getBuildOutput() needs. */ @@ -430,8 +416,7 @@ static void copyPathFromRemote( extractNarData(tee, localStore.printStorePath(info.path), narMembers); }); - store->addToStore(info, *source2, NoRepair, NoCheckSigs); - } + destStore.addToStore(info, *source2, NoRepair, NoCheckSigs); } static void copyPathsFromRemote( @@ -663,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); } } From 7d7a2a3836e0d3fcc25613bdc7c9787b6353b04d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 10 Dec 2023 17:03:13 -0500 Subject: [PATCH 58/60] Beef of content addressing test --- t/content-addressed/basic.t | 2 +- t/jobs/content-addressed.nix | 9 ++++++++- t/jobs/dir-with-file-builder.sh | 7 +++++++ 3 files changed, 16 insertions(+), 2 deletions(-) create mode 100755 t/jobs/dir-with-file-builder.sh diff --git a/t/content-addressed/basic.t b/t/content-addressed/basic.t index fc38c24d3..6597e727e 100644 --- a/t/content-addressed/basic.t +++ b/t/content-addressed/basic.t @@ -27,7 +27,7 @@ my $project = $db->resultset('Projects')->create({name => "tests", displayname = my $jobset = createBaseJobset("content-addressed", "content-addressed.nix", $ctx{jobsdir}); ok(evalSucceeds($jobset), "Evaluating jobs/content-addressed.nix should exit with return code 0"); -is(nrQueuedBuildsForJobset($jobset), 4, "Evaluating jobs/content-addressed.nix should result in 4 builds"); +is(nrQueuedBuildsForJobset($jobset), 5, "Evaluating jobs/content-addressed.nix should result in 4 builds"); for my $build (queuedBuildsForJobset($jobset)) { ok(runBuild($build), "Build '".$build->job."' from jobs/content-addressed.nix should exit with code 0"); diff --git a/t/jobs/content-addressed.nix b/t/jobs/content-addressed.nix index 785e917c2..65496df5e 100644 --- a/t/jobs/content-addressed.nix +++ b/t/jobs/content-addressed.nix @@ -18,10 +18,17 @@ rec { builder = ./succeed-with-failed.sh; }; + caDependingOnCA = + cfg.mkContentAddressedDerivation { + name = "ca-depending-on-ca"; + builder = ./dir-with-file-builder.sh; + FOO = empty_dir; + }; + nonCaDependingOnCA = cfg.mkDerivation { name = "non-ca-depending-on-ca"; - builder = ./empty-dir-builder.sh; + builder = ./dir-with-file-builder.sh; FOO = empty_dir; }; } diff --git a/t/jobs/dir-with-file-builder.sh b/t/jobs/dir-with-file-builder.sh new file mode 100755 index 000000000..e51c65585 --- /dev/null +++ b/t/jobs/dir-with-file-builder.sh @@ -0,0 +1,7 @@ +#! /bin/sh + +# Workaround for https://github.com/NixOS/nix/pull/6051 +echo "some output" + +mkdir $out +echo foo > $out/a-file From bfe3567571c1b3a50449c0d13ef6b438526408de Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 11 Dec 2023 13:03:35 -0500 Subject: [PATCH 59/60] Revert "Revert query -- those columns don't exist yet!" They do now! This reverts commit a6b6c5a5392a7e705825e8dac7cb818a1f9944fe. --- src/hydra-queue-runner/queue-monitor.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/hydra-queue-runner/queue-monitor.cc b/src/hydra-queue-runner/queue-monitor.cc index fc3442447..a4d9b4dce 100644 --- a/src/hydra-queue-runner/queue-monitor.cc +++ b/src/hydra-queue-runner/queue-monitor.cc @@ -192,12 +192,11 @@ bool State::getQueuedBuilds(Connection & conn, if (!res[0].is_null()) propagatedFrom = res[0].as(); if (!propagatedFrom) { - for (auto & [outputName, optOutputPath] : destStore->queryPartialDerivationOutputMap(ex.step->drvPath, &*localStore)) { - // ca-derivations not actually supported yet - assert(optOutputPath); + 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 path = $1 and startTime != 0 and stopTime != 0 and status = 1", - localStore->printStorePath(*optOutputPath)); + ("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), + outputName); if (!res[0][0].is_null()) { propagatedFrom = res[0][0].as(); break; From 2b1d27792f4c8108538de78f05cdc1c2c9cf70b0 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 26 Jan 2024 11:28:32 -0500 Subject: [PATCH 60/60] Fix mistake in merge --- src/hydra-queue-runner/hydra-queue-runner.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index d9d160c61..da392d61c 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -335,11 +335,12 @@ unsigned int State::createBuildStep(pqxx::work & txn, time_t startTime, BuildID for (auto & [name, output] : getDestStore()->queryPartialDerivationOutputMap(step->drvPath, &*localStore)) txn.exec_params0 - ("insert into BuildStepOutputs (build, stepnr, name, path) values ($1, $2, $3, $4)", + ("insert into BuildStepOutputs (build, stepnr, name, path, contentAddressed) values ($1, $2, $3, $4, $5)", buildId, stepNr, name, output ? std::optional { localStore->printStorePath(*output)} - : std::nullopt); + : std::nullopt, + step->drv->type().isCA()); if (status == bsBusy) txn.exec(fmt("notify step_started, '%d\t%d'", buildId, stepNr));