[Backport 2.33-maintenance] Fix concurrent builder failure empty message bugs #15068
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add helpers to the base Goal class that set buildResult and call amDone, ensuring buildResult is always populated when a goal terminates. Derived class helpers now call the base class versions. This reorders operations: previously buildResult was set before bookkeeping (counter resets, worker stats), now it's set after. This is safe because the bookkeeping code (mcExpectedBuilds.reset(), worker.doneBuilds++, worker.updateProgress(), etc.) only accesses worker counters, not buildResult. (cherry picked from commit cb2ade2)
DerivationTrampolineGoal is the top-level goal whose buildResult is returned by buildPathsWithResults. When it failed without setting buildResult.inner, buildPathsWithResults would return failures with empty errorMsg, producing error messages like: error: failed to build attribute 'checks.x86_64-linux.foo', build of '/nix/store/...drv^*' failed: (note the empty message after "failed:") Use the new doneFailure helper to ensure buildResult is populated with meaningful error information. (cherry picked from commit 25eb07a)
When keepGoing=false and a build fails, other goals are cancelled. Previously, these cancelled goals were reported in the "build of ... failed" error message alongside actual failures. This was misleading since cancelled goals didn't actually fail - they were never tried. Update the test to expect only the actual failure (hash mismatch) to be reported, not the cancelled goals. (cherry picked from commit 3fd85c7)
When !keepGoing and a goal fails, other goals are cancelled and
remain with exitCode == ecBusy. These cancelled goals have a default
BuildResult::Failure{} with empty errorMsg.
Previously, buildPathsWithResults would return these cancelled goals,
and throwBuildErrors would report them as failures. When only one such
cancelled goal was present, it would throw an error with an empty
message like:
error: build of '/nix/store/...drv^*' failed:
Now we skip goals with ecBusy since their state is indeterminate.
Cancelled goals could be reported, but this keeps the output relevant.
Other indeterminate goal states were already not being reported, for
instance: derivations that weren't started for being blocked on a
concurrency limit, or blocked on a currently building dependency.
(cherry picked from commit 68f549d)
Change "cannot build missing derivation" to "failed to obtain derivation of" since the path (e.g. '...drv^out') is a derivation output, not a derivation. The message could be improved further to resolve ambiguity when multiple outputOf links are involved, but for now we err on the side of brevity since this message is already merged into larger error messages with other context from the Worker and CLI. (cherry picked from commit 3c3ceb1)
internal-nix-ci
bot
requested review from
Ericson2314 and
edolstra
as code owners
github-actions
bot
added
the
with-tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Automatic backport to
2.33-maintenance, triggered by a label in #14972.