Make --keep-going work with drv that don't have a system #12168
Conversation
RossComputerGuy
requested review from
Ericson2314 and
edolstra
as code owners
github-actions
bot
added
with-tests
RossComputerGuy
force-pushed
the
fix/keep-going-not-going
branch
2 times, most recently
from
42567d4 to
d534b27
Compare
|
The basic idea seems sound here :) |
|
Heh yeah, I'm trying to diagnose why the tests fail. It seems like this PR is cursed with the same problem as the original attempt. |
src/libstore/build-result.hh
Outdated
| @@ -36,6 +36,7 @@ struct BuildResult | |||
| NotDeterministic, | |||
| ResolvesToAlreadyValid, | |||
| NoSubstituters, | |||
| NoBuilders, | |||
There was a problem hiding this comment.
| NoBuilders, | |
| NoCompatibleBuilder, |
There was a problem hiding this comment.
It may be worth adding special handling to Hydra for this build result (see https://github.com/NixOS/hydra/blob/250668a19fa4d8ff9a6176ee6c44ca3003adedf1/src/hydra-queue-runner/build-remote.cc#L360-L413) to get a nicer build status in the UI than "aborted".
There was a problem hiding this comment.
I made the change. Yeah, having Hydra support this in the future would be nice. It could say something like "no compatible build machines".
src/libstore/store-api.hh
Outdated
| @@ -61,6 +61,7 @@ MakeError(InvalidPath, Error); | |||
| MakeError(Unsupported, Error); | |||
| MakeError(SubstituteGone, Error); | |||
| MakeError(SubstituterDisabled, Error); | |||
| MakeError(BadSystem, Error); | |||
There was a problem hiding this comment.
| MakeError(BadSystem, Error); | |
| MakeError(NoCompatibleBuilder, Error); |
There was a problem hiding this comment.
I made the change.
| # Hack to get the scheduler to do what we want: The `good` derivation can | ||
| # only be built after `delay_good` (which takes a long time to build) while | ||
| # the others don't have any dependency. | ||
| # This means that if we build this with parallelism (`-j2`), then we can be | ||
| # reasonably sure that the failing derivations will be scheduled _before_ the | ||
| # `good` one (and so we can check that `--keep-going` works fine) |
There was a problem hiding this comment.
This could be synchronized by waiting for a file to appear in a mutable directory in extra-sandbox-paths. The test would have to monitor the CLI's log before creating it.
(doing it in the failing derivation would only decrease the length of the race condition, but not make it reliable)
Something along the lines of
nix build --extra-sandbox-paths "$TEST_ROOT/sync" 2>&1 \
| while read ln; do
echo "nix build: $ln"
if echo "$ln" | grepQuiet ...; then
touch "$TEST_ROOT/sync/failing-drv-failed"
fi
doneThere was a problem hiding this comment.
Looks like that might work? I plugged it in. It doesn't break the tests on my machine.
|
|
||
| clearStore | ||
|
|
||
| # XXX: These invocations of nix-build should always return 100 according to the manpage, but often return 1 |
There was a problem hiding this comment.
Can you create an issue for this?
There was a problem hiding this comment.
Ok, @iFreilicht is the one who discovered the error. Probably could use more details before creating the issue?
There was a problem hiding this comment.
Well, that's all I found out, I don't have more details for you 😬 But it basically happened during testing, so you should be able to reproduce that issue and can then create one with a proper log. This might also be why the tests are failing right now.
There was a problem hiding this comment.
Oh, alright. I did check the blame and it looks like @Ericson2314 did write some of the worker stuff around where I'm touching. Maybe John has some ideas into getting this to pass.
RossComputerGuy
force-pushed
the
fix/keep-going-not-going
branch
from
9e574a8 to
f04f0bf
Compare
RossComputerGuy
force-pushed
the
fix/keep-going-not-going
branch
from
f04f0bf to
5e27b27
Compare
|
@RossComputerGuy Any idea why the test is failing? |
|
No, I tried debugging it via gdb but breakpoints don't trigger with debug symbols. It also looks like the testing instructions in the documentation is out of date. |
Motivation
Fixes #8321, this resurrects #8775 and changes things a little differently.
Context
This adds a new build result of
NoBuilderswhich implies that the derivation cannot be build because there are no builders. While a new error failure calledBadSystemis added to imply the system configuration is bad or incompatible. These have similar meaning but imply different things which could make sense for future PR's. Currently, only a bad system error occurs when there are no builders which matches a derivation.Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.