Instead of passing them around separately, or doing finicky logic in a
try-catch block to recover them, just make `BuildError` always contain a
status, and make it the thrower's responsibility to set it. This is much
more simple and explicit.
Once that change is done, split the `done` functions of `DerivationGoal`
and `DerivationBuildingGoal` into separate success and failure
functions, which ends up being easier to understand and hardly any
duplication.
Also, change the handling of failures in resolved cases to use
`BuildResult::DependencyFailed` and a new message. This is because the
underlying derivation will also get its message printed --- which is
good, because in general the resolved derivation is not unique. One dyn
drv test had to be updated, but CA (and dyn drv) is experimental, so I
do not mind.
Finally, delete `SubstError` because it is unused.
The commit says it was added for CA testing --- manual I assume, since
there is no use of this in the test suite. I don't think we need it any
more, and I am not sure whether it was ever supposed to have made it to
`master` either.
This reverts commit 2eec2f765a.
We currently just use this during the build of a derivation, but there is no
reason we wouldn't want to use it elsewhere, e.g. to check the outputs
of someone else's build after the fact.
Moreover, I like pulling things out of `DerivationBuilder` that are
simple and don't need access to all that state. While
`DerivationBuilder` is unix-only, this refactor also make the code more
portable "for free".
The header is private, at Eelco's request.
With the migration to /nix/var/nix/builds we now have failing builds
when the derivation name is too long.
This change removes the derivation name from the temporary build to have
a predictable prefix length:
Also see: https://github.com/NixOS/infra/pull/764
for context.
This allows us to replace some very hacky and not correct string
concatentation in `HttpBinaryCacheStore`. It will especially be useful
with #13752, when today's hacks started to cause problems in practice,
not just theory.
Also make `fixGitURL` returned a `ParsedURL`.
I need this for some `ParseURL` improvements, but I figure this is
better to send as its own PR.
I changed the tests willy-nilly to sometimes use
`std::list<std::string_view>` instead of `Strings` (which is
`std::list<std::string>`).
Co-Authored-By: Sergei Zimmerman <sergei@zimmerman.foo>
It is suppposed to be "post build" not "during the build" after all. Its
location now matches that for the hook case (see elsewhere in
`DerivationdBuildingGoal`).
It was in a try-catch before, and now it isn't, but I believe that it is
impossible for it to throw `BuildError`, which is sufficient for this
code motion to be correct.
Update src/libutil/windows/current-process.cc
Prefer `nullptr` over `NULL`
Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
Update src/libutil/unix/current-process.cc
Prefer C++ type casts
Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
Update src/libutil/windows/current-process.cc
Prefer C++ type casts
Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
Update src/libutil/unix/current-process.cc
Don't allocate exception
Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo>
It would have been nice to use std::views::enumerate here, but
it uses a signed difference type for the value_type:
> value_type = std::tuple<difference_type, ranges::range_value_t<Base>>
zip + iota has the same semantics as the code used to have, so there's
no behavior change here.
This is a nicer separation of concerns --- `DerivationBuilder` just
mounts the extra paths you tell it too, and the outside world is
responsible for making sure those extra paths make sense.
Since the closure only depends on global settings, and not
per-derivation information, we also have the option of moving this up
further and caching it across all local builds. (I only just realized
this after having done this refactor. I am not doing that change at this
time, however.)
Now, `DerivationBuilder` only concerns itself with `finalEnv` and
`extraFiles`, in straightforward unconditional code. All the fancy
desugaring logic is consolidated in `DerivationBuildingGoal`.
We should better share the pulled-out logic with `nix-shell`/`nix
develop`, which would fill in some missing features, arguably fixing
bugs.
I think this is a better separation of concerns. `DerivationBuilder`
doesn't need to to the final, query-heavy details about how these things
are constructed. It just operates on the level of "simple, stupid" files
and environment variables.
As much as I prefer rewriting the parsed rather than unparsed JSON for
elegance, this gets in the way of the separation of concerns that I am
trying to do.
As a practical matter, any rewriting that this did will also be done by
the second round of rewriting that remains below, so removing this code
should have no effect.
This is needed to rearrange include order, but I also think it is a good
thing anyways, as we seek to reduce the use of global settings variables
over time.
This avoids problems with older versions of Nix that don't put the
caches in WAL mode. That's generally not a problem, until you do something like
nix build --print-out-paths ... | cachix
which deadlocks because cachix tries to switch the caches to truncate
mode, which requires exclusive access. But the first process cannot
make progress because the cachix process isn't reading from the pipe.
With "truncate" mode, if we try to write to the database while another
process has an active write transaction, we'll block until the other
transaction finishes. This is a problem for the evaluation cache in
particular, since it uses long-running transactions.
WAL mode does not have this issue: it just returns "busy" right away,
so Nix will print
error (ignored): SQLite database '/home/eelco/.cache/nix/eval-cache-v5/...' is busy
and stop trying to write to the evaluation cache. (This was the
intended/original behaviour, see AttrDb::doSQLite().)
This systematizes the way our s3:// URLs are parsed in filetransfer.cc.
Yoinked out and refactored out of [1].
[1]: https://github.com/NixOS/nix/pull/13752
Co-authored-by: Bernardo Meurer Costa <beme@anthropic.com>
Compilers in nixpkgs have caught up and major distros
should also have recent enough compilers. It would be
nice to have newer features like more full featured
ranges and deducing this.
