The simplification here is due to a long-standing bug, but it is not
worth fixing the bug at this time. Instead we've finally written up an
issue for the bug, and referenced the issue number in the code.
In the local building case, there is many things which can through
`BuildError`, but in the hook case there is just this one. We can
therefore simplify the code by "cinching" down the logic just to the
spot the error is thrown.
There is other code outside `libstore/build` which also uses
`BuildError`, but I believe those cases are mistakes. The point of
`BuildError` is the narrow technical use-cases of "errors which should
not be fatal with `--keep-going`". Using it outside the
building/scheduling code doesn't really make sense in that regard. It
seems likely that those usages were instead merely because "oh, this
error has something to do with building, so I guess `BuildError` is
better than `Error`".
It is quite likely that I myself used `BuildError` incorrectly as
described above :).
The variables are only set by CGroup mechanisms in `killSandbox` in the
local build. In the build hook case, these variables will not be set, so
there is nothing to do.
We can move this method from `LocalStore` to `Store` --- even if we only
want the actual builder to sign things in many cases, there is no reason
to try to enforce this policy by spurious moving the method to a
subclass.
Now, we might technically sign class, but CA derivations is
experimental, and @Ericson2314 is going to revisit all this stuff with
issue #11896 anyways.
- `chrootParentDir` can be a local variable instead of a class variable.
- `getChildStatus` can be inlined. Again, we have the `assert(!hook);`
in the local building case, which makes for a simpler thing inlined.
Thanks to the previous commit, we can inline all these small callbacks.
In the build-hook case, they were empty, and now they disappear
entirely.
While `LocalDerivationGoal` can be used in the hook case (we use it
based on whether we have a local store, not based on whether we are
using the build hook, a decision which comes later), the previous
commit's inline moved the code into a spot where we know we are cleaning
up after local building, *not* after running the build hook. This allows
for much more simplification.
The basic idea is that while we have duplicated this function, we now
have one call-site in the local build case, and one call site in the
build hook case. This unlocks big opportunities to specialize each copy,
since they really shouldn't be doing the same things. By the time we are
are done, there should not be much duplication left.
See #12628 for further info.
This variable was once used to communicate already acquired store path
locks between Nix and the build hook, but this hasn't been the case
since 9bcb4d2dd9. So let's get rid of
it.
It was first introduced in 19e0ce2c03
In Nix we only register the crash handler in main instead of initNix,
because library user may want to use their own crash handler.
Sample output:
Mar 12 08:38:06 eve nix[2303762]: Nix crashed. This is a bug. Please report this at https://github.com/NixOS/nix/issues with the following information included:
Mar 12 08:38:06 eve nix[2303762]: Exception: nix::SysError: error: writing to file: Resource temporarily unavailable
Mar 12 08:38:06 eve nix[2303762]: Stack trace:
Mar 12 08:38:06 eve nix[2303762]: 0# 0x000000000076876A in nix
1# 0x00007FDA40E9F20A in /nix/store/2lhklm5aizx30qbw49acnrrzkj9lbmij-gcc-14-20241116-lib/lib/libstdc++.so.6
2# std::unexpected() in /nix/store/2lhklm5aizx30qbw49acnrrzkj9lbmij-gcc-14-20241116-lib/lib/libstdc++.so.6
3# 0x00007FDA40E9F487 in /nix/store/2lhklm5aizx30qbw49acnrrzkj9lbmij-gcc-14-20241116-lib/lib/libstdc++.so.6
4# nix::writeFull(int, std::basic_string_view<char, std::char_traits<char> >, bool) in /home/joerg/git/nix/inst/lib/libnixutil.so
5# nix::writeLine(int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) in /home/joerg/git/nix/inst/lib/libnixutil.so
6# nix::JSONLogger::write(nlohmann::json_abi_v3_11_3::basic_json<std::map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void> const&) in /home/joerg/git/nix/inst/lib/libnixutil.so
7# nix::JSONLogger::logEI(nix::ErrorInfo const&) in /home/joerg/git/nix/inst/lib/libnixutil.so
8# nix::Logger::logEI(nix::Verbosity, nix::ErrorInfo) in nix
9# nix::handleExceptions(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<void ()>) in /home/joerg/git/nix/inst/lib/libnixmain.so
10# 0x000000000087A563 in nix
11# 0x00007FDA40BD41FE in /nix/store/6q2mknq81cyscjmkv72fpcsvan56qhmg-glibc-2.40-66/lib/libc.so.6
12# __libc_start_main in /nix/store/6q2mknq81cyscjmkv72fpcsvan56qhmg-glibc-2.40-66/lib/libc.so.6
13# 0x00000000006F4DF5 in nix
Co-authored-by: eldritch horrors <pennae@lix.systems>
Doing this makes catching non-obvious bugs easier. GHA CI workload is
already a concern and there isn't much benefit in running the tests with
and without sanitizers at the same time, so UBSAN is enabled for default
checks.
This change doesn't affect production builds in any way, but is rather a
step in the direction of improving automated testing during development.
Relates to #10969.
This fixes a few of the property tests, now that the property tests
are actually generating arbitrary data - some of that data now
requiring experimental features to function properly.
Here we're switching to combinators instead of dereference operator.
It turns out the dereference operator was being executed upon test
setup, meaning that we were only using a only single value for each of
the executions of the property tests! Really not good.
And on Windows, we instead get:
operator* is not allowed in this context
ff6af6fc68/src/gen/detail/GenerationHandler.cpp (L16C31-L16C71)
Now a few of the property tests fail, because we're generating cases
which haven't been exercised before.
I refactored the way that input resolution works in `DerivationGoal`. To
be honest, it is probably unclear to the reader whether this new way is
better or worse. I suppose *intrinsic* motivation, I can say that
- the more structured use of `inputGoal` (a local variable) is better
than the shotgrun approach with `inputDrvOutputs`
- A virtual `waiteeDone` was a hack, and now it's gone.
However, the *real* motivation of this is not the above things, but that
it is needed for my mammoth refactor fixing #11897 and #11928.
It is nice that this step could come first, rather than making that
refactor even bigger.
The bug reappeared after all, and the fix introduced a different bug. I
just reverted on 2.27 first, in #12576, but upon further introspection
and discussion with @roberth, with preparing for and travelling to
Planet Nix I will not be able to fix it on `master` soon enough for a
revert to not be warranted here in the meantime also.
This reverts commit c98525235f.
