From f21fd6b7e653230c01b454f486cc51d5cc999667 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 10 Mar 2025 19:16:11 +0100 Subject: [PATCH] libstore: fix substitution failures on racy builds if two derivations have different outputs of the same other derivation as inputs there's a race window in which wanted output tracking broke: - start building depender-a - schedule dependency^a for depender-a - substitute dependency^a - dependency runs path validity checks, yields - start building depender-b - depender-b adds b to the wanted set of dependency - dependency resumes from validity checks with allValid - depender-b is unblocked, but dependency^b is not in the store Change-Id: Id4dba517bbbdbae96bf430d2c68f63eccd3b99dd --- lix/libstore/build/derivation-goal.cc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lix/libstore/build/derivation-goal.cc b/lix/libstore/build/derivation-goal.cc index 105e4f4ad..8023281eb 100644 --- a/lix/libstore/build/derivation-goal.cc +++ b/lix/libstore/build/derivation-goal.cc @@ -362,6 +362,13 @@ try { auto [allValid, validOutputs] = TRY_AWAIT(checkPathValidity()); + // recheck needRestart. more wanted outputs may have been added during the + // path validity check, and we do not want to treat !allValid as an error. + if (!allValid && needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) { + needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed; + co_return TRY_AWAIT(haveDerivation()); + } + if (buildMode == bmNormal && allValid) { co_return done(BuildResult::Substituted, std::move(validOutputs)); } @@ -509,6 +516,10 @@ try { } if (dependencies.empty()) { + // NOTE assertPathValidity *can* fail if wanted outputs are added while + // it is running. repair mode cannot work correctly if the goal was not + // created with all outputs wanted in the first place though, so we can + // ignore this possiblity and assume that all failures are real errors. co_return done(BuildResult::AlreadyValid, TRY_AWAIT(assertPathValidity())); } @@ -1718,7 +1729,7 @@ try { worker.store.printStorePath(drvPath), concatStringsSep(", ", quoteStrings(wantedOutputsLeft))); - bool allValid = true; + bool allValid = needRestart != NeedRestartForMoreOutputs::OutputsAddedDoNeed; for (auto & [_, status] : initialOutputs) { if (!status.wanted) continue; if (!status.known || !status.known->isValid()) {