| Age | Commit message (Collapse) | Author |
|---|
|
SimpleLogger is not fully thread-safe, and all loggers that wrap it are
also not safe accordingly. this does not affect much, but in rare cases
it can cause interleaving of messages on stderr when used with the json
or raw log formats. the fix applied here is a bit of a hack, but fixing
this properly requires rearchitecting the logger infrastructure. nested
loggers are not the most natural abstraction here, and it is biting us.
Change-Id: Ifbf34fe1e85c60e73b59faee50e7411c7b5e7c12
|
|
(plus one reference to CppNix github)
Change-Id: Id8b3d2897f3b54e286861805cfd421adc4d5de47
|
|
we no longer need these since we're no longer using sinks to serialize things.
Change-Id: Iffb1a3eab33c83f611c88fa4e8beaa8d5ffa079b
|
|
this is cursed. deeply and profoundly cursed. under NO CIRCUMSTANCES
must protocol serializer helpers be applied to temporaries! doing so
will inevitably cause dangling references and cause the entire thing
to crash. we need to do this even so to get rid of boost coroutines,
and likewise to encapsulate the serializers we suffer today at least
a little bit to allow a gradual migration to an actual IPC protocol.
(this isn't a problem that's unique to generators. c++ coroutines in
general cannot safely take references to arbitrary temporaries since
c++ does not have a lifetime system that can make this safe. -sigh-)
Change-Id: I2921ba451e04d86798752d140885d3c5cc08e146
|
|
upcast_goal was only ever needed to break circular includes, but the
same solution that gave us upcast_goal also lets us fully remove it:
just upcast goals without a wrapper function, but only in .cc files.
Change-Id: I9c71654b2535121459ba7dcfd6c5da5606904032
|
|
this much more closely mimics what is actually happening: we're reading
data from somewhere else, actively, rather than passively waiting. with
the data flow matching the underlying system interactions better we can
remove a few sinkToSource calls that merely exists to undo the mismatch
caused by not treating subprocess output as a data source to begin with
Change-Id: If4abfc2f8398fb5e88c9b91a8bdefd5504bb2d11
|
|
this will let us also return a source for the program output later,
which will in turn make sinkToSource unnecessary for program output
processing. this may also reopen a path for provigin program input,
but that still needs a proper async io framework to avoid problems.
Change-Id: Iaf93f47db99c38cfaf134bd60ed6a804d7ddf688
|
|
This comes quite often when the available job slots on all remote
builders are exhausted and this is pretty spammy.
This isn't really an issue, but expected behavior.
A better way to display this is a nom-like approach where all scheduled
builds are shown in a tree and pending builds are being marked as such
IMHO.
Change-Id: I6bc14e6054f84e3eb0768127b490e263d8cdcf89
|
|
This happened during a PathSubstitutionGoal of a .drv file:
substitution of '/tmp/jade/nix-test/ca/eval-store/store/1lj7lsq5y0f25mfbnq6d3zd0bw5ay33n-dependencies-input-2.drv'
What happened here is that since PathSubstitutionGoal is not a
DerivationGoal, in production builds, the UB was not caught, since it
would early-exit from failing a dynamic_cast to DerivationGoal * on the
very next line, but before the null reference was ever used.
This was nonetheless UB. The fix should be to just rearrange the two
lines; I don't think there is a further bug there, since *substituting a
.drv* **necessarily** means you cannot have the representation of
the derivation as would be necessary for drv to not be null there.
Test failure:
++(eval-store.sh:12) _RR_TRACE_DIR=/home/jade/.local/share/rr rr record -- nix build -f dependencies.nix --eval-store /tmp/jade/nix-test/ca/eval-store/eval-store -o /tmp/jade/nix-test/ca/eval-store/result
don't know how to build these paths:
/tmp/jade/nix-test/ca/eval-store/store/6y51mf0p57ggipgab6hdjabbvplzsicq-dependencies-top.drv
copying 1 paths...
copying path '/tmp/jade/nix-test/ca/eval-store/store/8027afyvqb87y1sf5xhdkqsflqn1ziy8-dependencies.builder0.sh' to 'local'...
copying 1 paths...
copying path '/tmp/jade/nix-test/ca/eval-store/store/7r5pqyncvfgrryf9gzy1z56z3xigi61x-builder-dependencies-input-0.sh' to 'local'...
copying 1 paths...
copying path '/tmp/jade/nix-test/ca/eval-store/store/nhmgm87zlqy3ks96dxrn7l37b72azi99-builder-dependencies-input-1.sh' to 'local'...
copying 1 paths...
copying path '/tmp/jade/nix-test/ca/eval-store/store/nq4qa2j6y8ajqazlfq6h46ck637my1n6-builder-dependencies-input-2.sh' to 'local'...
copying 1 paths...
copying path '/tmp/jade/nix-test/ca/eval-store/store/6vh0vna9l5afck01y7iaks3hm9ikwqyj-builder-fod-input.sh' to 'local'...
building '/tmp/jade/nix-test/ca/eval-store/store/gy91pqymf2nc5v7ld1bad94xpwxdi25s-dependencies-input-0.drv'...
building '/tmp/jade/nix-test/ca/eval-store/store/w7wlkjx97ivmnrymkac5av3nyp94hzvq-dependencies-input-1.drv'...
../src/libstore/build/derivation-goal.cc:1556:22: runtime error: reference binding to null pointer of type 'Derivation'
0 0x734ba59a6886 in nix::DerivationGoal::waiteeDone(std::shared_ptr<nix::Goal>, nix::Goal::ExitCode) /home/jade/lix/lix2/build/src/libstore/build/derivation-goal.cc:1556:12
1 0x734ba59c0962 in nix::Goal::amDone(nix::Goal::ExitCode, std::optional<nix::Error>) /home/jade/lix/lix2/build/src/libstore/build/goal.cc:95:25
2 0x734ba5a1c44a in nix::PathSubstitutionGoal::done(nix::Goal::ExitCode, nix::BuildResult::Status, std::optional<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>>) /home/jade/lix/lix2/build/src/libstore/build/substitution-goal.cc:38:5
3 0x734ba5a1b454 in nix::PathSubstitutionGoal::init() /home/jade/lix/lix2/build/src/libstore/build/substitution-goal.cc:56:9
4 0x734ba5a2a6c6 in nix::Worker::run(std::set<std::shared_ptr<nix::Goal>, nix::CompareGoalPtrs, std::allocator<std::shared_ptr<nix::Goal>>> const&) /home/jade/lix/lix2/build/src/libstore/build/worker.cc:320:23
5 0x734ba59b93d8 in nix::Store::buildPathsWithResults(std::vector<nix::DerivedPath, std::allocator<nix::DerivedPath>> const&, nix::BuildMode, std::shared_ptr<nix::Store>) /home/jade/lix/lix2/build/src/libstore/build/entry-points.cc:60:12
6 0x734ba663c107 in nix::Installable::build2(nix::ref<nix::Store>, nix::ref<nix::Store>, nix::Realise, std::vector<nix::ref<nix::Installable>, std::allocator<nix::ref<nix::Installable>>> const&, nix::BuildMode) /home/jade/lix/lix2/build/src/libcmd/installables.cc:637:36
Change-Id: Id0e651e480bebf6356733b01bc639e9bb59c7bd0
|
|
Change-Id: Ic1f68e6af658e94ef7922841dd3ad4c69551ef56
|
|
Move the identical static `chmod_` functions in libstore to
libutil. the function is called `chmodPath` instead of `chmod`
as otherwise it will shadow the standard library chmod in the nix
namespace, which is somewhat confusing.
Change-Id: I7b5ce379c6c602e3d3a1bbc49dbb70b1ae8f7bad
|
|
it's no longer used. it really shouldn't have existed this long since it
was just a mashup of both std::promise and std::packaged_task in a shape
that makes composition unnecessarily difficult. all but a single case of
Callback pattern calls were fully synchronous anyway, and even this sole
outlier was by far not important enough to justify the extra complexity.
Change-Id: I208aec4572bf2501cdbd0f331f27d505fca3a62f
|
|
Either the contents of `line` could cause format errors, or this usage
is Technically safe. However, I trust nothing, especially with
boost::format.
Change-Id: I07933b20bde3b305a6e5d61c2a7bab6ecb042ad9
|
|
This does involve making a large number of destructors able to throw,
because we had to change it high in the class hierarchy. Oh well.
Change-Id: Ib62d3d6895b755f20322bb8acc9bf43daf0174b2
|
|
This was achieved by running maintainers/buildtime_report.sh on the
build directory of a meson build, then asking "why the heck is json
eating our build times", and strategically moving the json using bits
out of widely included headers.
It turns out that putting literally any metrics whatsoever into the
build had immediate and predictable results.
Results are 1382.5s frontend time -> 1175.4s frontend time, back end
time approximately invariant.
Related: https://git.lix.systems/lix-project/lix/issues/159
Change-Id: I7edea95c8536203325c8bb4dae5f32d727a21b2d
|
|
add a reset() method to close the wrapped fd instead of assigning magic
constants. also make the from-fd constructor explicit so you can't
accidentally assign the *wrong* magic constant, or even an unrelated
integer that also just happens to be an fd by pure chance.
Change-Id: I51311b0f6e040240886b5103d39d1794a6acc325
|
|
Cleanup `fmt.hh`
(cherry picked from commit 47a1dbb4b8e7913cbb9b4d604728b912e76e4ca0)
Change-Id: Id076a45cb39652f437fe3f8bda10c310a9894777
|
|
Include phase reporting in log file for ssh-ng builds
(cherry picked from commit b1e7d7cad625095656fff05ac4aedeb12135110a)
Change-Id: I4076669b0ba160412f7c628ca9113f9abbc8c303
|
|
runPostBuildHook(): Be less chatty
(cherry picked from commit 28a988bdde7b20abff9000162015500c44a5812b)
Change-Id: Ibb49b71bf631d04d08970d719f091221b15c5036
|
|
Rename `yellowtxt` -> `magentatxt`
(cherry picked from commit fb78a99e04206e7b1df84a362bb87d3300b41855)
Change-Id: I9ade553d9f499e6713aeff3463c9a653a880a051
|
|
obsidiansystems/floating-content-addressing-derivations-eval-store
Fix building CA derivations with and eval store
(cherry picked from commit dfc0cee7024a082d90a4f68296f55a82dfd52126)
Change-Id: I28feb5a36d4fe75f0ed3e3e2db6eb56b67d0f371
|
|
Give `Derivation::tryResolve` an `evalStore` argument
(cherry picked from commit 36ca6adc60511dc822870f2df43c0a578e481925)
Change-Id: If76b185a01ffa982e4c49cf333a9b5fbf9edebfe
|
|
Unlock output paths when a derivation is already built
(cherry picked from commit 7ba4e073e8622ca86b52e03d68476e80250ab62f)
Change-Id: I9de077679290d5141a610ac43d99d3a43acff87c
|
|
Fix/remove some bad std::moves
(cherry picked from commit 8c049a9f044569ebda70231709f6f15d3073894a)
Change-Id: I720273378d2506a13883acee28abd096d099b0d4
|
|
Factor out bits of the worker protocol to use elsewhere
(cherry picked from commit 4b1a97338f517f45e6169d3d8845c5caa5724e97)
Change-Id: If93afa0f8b1cf9b0e705b34fa71e6fd708752758
|
|
This reverts commit 5e3986f59cb58f48186a49dcec7aa317b4787522. This
un-implements RFC 92 but fixes the critical bug #9052 which many people
are hitting. This is a decent stop-gap until a minimal reproduction of
that bug is found and a proper fix can be made.
Mostly fixed #9052, but I would like to leave that issue open until we
have a regression test, so I can then properly fix the bug (unbreaking
RFC 92) later.
(cherry picked from commit 8440afbed756254784d9fea3eaab06649dffd390)
|
|
We use the same nested map representation we used for goals, again in
order to save space. We might someday want to combine with `inputDrvs`,
by doing `V = bool` instead of `V = std::set<OutputName>`, but we are
not doing that yet for sake of a smaller diff.
The ATerm format for Derivations also needs to be extended, in addition
to the in-memory format. To accomodate this, we added a new basic
versioning scheme, so old versions of Nix will get nice errors. (And
going forward, if the ATerm format changes again the errors will be even
better.)
`parsedStrings`, an internal function used as part of parsing
derivations in A-Term format, used to consume the final `]` but expect
the initial `[` to already be consumed. This made for what looked like
unbalanced brackets at callsites, which was confusing. Now it consumes
both which is hopefully less confusing.
As part of testing, we also created a unit test for the A-Term format for
regular non-experimental derivations too.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Apply suggestions from code review
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
|
|
To avoid dealing with an optional `drvPath` (because we might not know
it yet) everywhere, make an `CreateDerivationAndRealiseGoal`. This goal
just builds/substitutes the derivation file, and then kicks of a build
for that obtained derivation; in other words it does the chaining of
goals when the drv file is missing (as can already be the case) or
computed (new case).
This also means the `getDerivation` state can be removed from
`DerivationGoal`, which makes the `BasicDerivation` / in memory case and
`Derivation` / drv file file case closer together.
The map type is factored out for clarity, and because we will soon hvae
a second use for it (`Derivation` itself).
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
|
|
We're about to split up `DerivationGoal` a bit. At that point
`makeDerivationGoal` will mean something more specific than it does
today. (Perhaps a future rename will make this clearer.)
On the other hand, the more public `Worker::makeGoal` function will
continue to work exactly as before. So by moving some call sites to use
that instead, we preemptively avoid issues in the next step.
|
|
Types converted:
- `NixStringContextElem`
- `OutputsSpec`
- `ExtendedOutputsSpec`
- `DerivationOutput`
- `DerivationType`
Existing ones mostly conforming the pattern cleaned up:
- `ContentAddressMethod`
- `ContentAddressWithReferences`
The `DerivationGoal::derivationType` field had a bogus initialization,
now caught, so I made it `std::optional`. I think #8829 can make it
non-optional again because it will ensure we always have the derivation
when we construct a `DerivationGoal`.
See that issue (#7479) for details on the general goal.
`git grep 'Raw::Raw'` indicates the two types I didn't yet convert
`DerivedPath` and `BuiltPath` (and their `Single` variants) . This is
because @roberth and I (can't find issue right now...) plan on reworking
them somewhat, so I didn't want to churn them more just yet.
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>
|
|
We want to be able to write down `foo.drv^bar.drv^baz`:
`foo.drv^bar.drv` is the dynamic derivation (since it is itself a
derivation output, `bar.drv` from `foo.drv`).
To that end, we create `Single{Derivation,BuiltPath}` types, that are
very similar except instead of having multiple outputs (in a set or
map), they have a single one. This is for everything to the left of the
rightmost `^`.
`NixStringContextElem` has an analogous change, and now can reuse
`SingleDerivedPath` at the top level. In fact, if we ever get rid of
`DrvDeep`, `NixStringContextElem` could be replaced with
`SingleDerivedPath` entirely!
Important note: some JSON formats have changed.
We already can *produce* dynamic derivations, but we can't refer to them
directly. Today, we can merely express building or example at the top
imperatively over time by building `foo.drv^bar.drv`, and then with a
second nix invocation doing `<result-from-first>^baz`, but this is not
declarative. The ethos of Nix of being able to write down the full plan
everything you want to do, and then execute than plan with a single
command, and for that we need the new inductive form of these types.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
|
|
Pass this around instead of `Source &` and `Sink &` directly. This will
give us something to put the protocol version on once the time comes.
To do this ergonomically, we need to expose `RemoteStore::Connection`,
so do that too. Give it some more API docs while we are at it.
|
|
See API docs on that struct for why. The pasing as as template argument
doesn't yet happen in that commit, but will instead happen in later
commit.
Also make `WorkerOp` (now `Op`) and enum struct. This led us to catch
that two operations were not handled!
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
|
|
This is generally a fine practice: Putting implementations in headers
makes them harder to read and slows compilation. Unfortunately it is
necessary for templates, but we can ameliorate that by putting them in a
separate header. Only files which need to instantiate those templates
will need to include the header with the implementation; the rest can
just include the declaration.
This is now documenting in the contributing guide.
Also, it just happens that these polymorphic serializers are the
protocol agnostic ones. (Worker and serve protocol have the same logic
for these container types.) This means by doing this general template
cleanup, we are also getting a head start on better indicating which
code is protocol-specific and which code is shared between protocols.
|
|
This is the more typically way to do [Argument-dependent
lookup](https://en.cppreference.com/w/cpp/language/adl)-leveraging
generic serializers in C++. It makes the relationship between the `read`
and `write` methods more clear and rigorous, and also looks more
familiar to users coming from other languages that do not have C++'s
libertine ad-hoc overloading.
I am returning to this because during the review in
https://github.com/NixOS/nix/pull/6223, it came up as something that
would make the code easier to read --- easier today hopefully already,
but definitely easier if we were have multiple codified protocols with
code sharing between them as that PR seeks to accomplish.
If I recall correctly, the main criticism of this the first time around
(in 2020) was that having to specify the type when writing, e.g.
`WorkerProto<MyType>::write`, was too verbose and cumbersome. This is
now addressed with the `workerProtoWrite` wrapper function.
This method is also the way `nlohmann::json`, which we have used for a
number of years now, does its serializers, for what its worth.
This reverts commit 45a0ed82f089158a79c8c25ef844c55e4a74fc35. That
commit in turn reverted 9ab07e99f527d1fa3adfa02839da477a1528d64b.
|
|
Derivations can output "text-hashed" data
|
|
|
|
|
|
In other words, use a plain `ContentAddress` not
`ContentAddressWithReferences` for `DerivationOutput::CAFixed`.
Supporting fixed output derivations with (fixed) references would be a
cool feature, but it is out of scope at this moment.
|
|
In many cases we are dealing with a collection of realisations, they are
all outputs of the same derivation. In that case, we don't need
"derivation hashes modulos" to be part of our map key, because the
output names alone will be unique. Those hashes are still part of the
realisation proper, so we aren't loosing any information, we're just
"normalizing our schema" by narrowing the "primary key".
Besides making our data model a bit "tighter" this allows us to avoid a
double `for` loop in `DerivationGoal::waiteeDone`. The inner `for` loop
was previously just to select the output we cared about without knowing
its hash. Now we can just select the output by name directly.
Note that neither protocol is changed as part of this: we are still
transferring `DrvOutputs` over the wire for `BuildResult`s. I would only
consider revising this once #6223 is merged, and we can mention protocol
versions inside factored-out serialization logic. Until then it is
better not change anything because it would come a the cost of code
reuse.
|
|
If my memory is correct, @edolstra objected to modifying `wantedOutputs`
upon falling back to doing a build (as we did before), because we should
only modify it in response to new requests --- *actual* wants --- and
not because we are "incidentally" building all the outptus beyond what
may have been requested.
That's a fair point, and the alternative is to replace the boolean soup
with proper enums: Instead of modifying `wantedOuputs` som more, we'll
modify `needsRestart` to indicate we are passed the need.
|
|
In https://github.com/NixOS/nix/pull/6311#discussion_r834863823, I
realized since derivation goals' wanted outputs can "grow" due to
overlapping dependencies (See `DerivationGoal::addWantedOutputs`, called
by `Worker::makeDerivationGoalCommon`), the previous bug fix had an
unfortunate side effect of causing more pointless rebuilds.
In paticular, we have this situation:
1. Goal made from `DerivedPath::Built { foo, {a} }`.
2. Goal gives on on substituting, starts building.
3. Goal made from `DerivedPath::Built { foo, {b} }`, in fact is just
modified original goal.
4. Though the goal had gotten as far as building, so all outputs were
going to be produced, `addWantedOutputs` no longer knows that and so
the goal is flagged to be restarted.
This might sound far-fetched with input-addressed drvs, where we usually
basically have all our goals "planned out" before we start doing
anything, but with CA derivation goals and especially RFC 92, where *drv
resolution* means goals are created after some building is completed, it
is more likely to happen.
So the first thing to do was restore the clearing of `wantedOutputs` we
used to do, and then filter the outputs in `buildPathsWithResults` to
only get the ones we care about.
But fix also has its own side effect in that the `DerivedPath` in the
`BuildResult` in `DerivationGoal` cannot be trusted; it is merely the
*first* `DerivedPath` for which this goal was originally created.
To remedy this, I made `BuildResult` be like it was before, and instead
made `KeyedBuildResult` be a subclass wit the path. Only
`buildPathsWithResults` returns `KeyedBuildResult`s, everything else
just becomes like it was before, where the "key" is unambiguous from
context.
I think separating the "primary key" field(s) from the other fields is
good practical in general anyways. (I would like to do the same thing
for `ValidPathInfo`.) Among other things, it allows constructions like
`std::map<Key, ThingWithKey>` where doesn't contain duplicate keys and
just precludes the possibility of those duplicate keys being out of
sync.
We might leverage the above someday to overload `buildPathsWithResults`
to take a *set* of return a *map* per the above.
-----
Unfortunately, we need to avoid C++20 strictness on designated
initializers.
(BTW
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2287r1.html
this offers some new syntax for this use-case. Hopefully this will be
adopted and we can eventually use it.)
No having that yet, maybe it would be better to not make
`KeyedBuildResult` a subclass to just avoid this.
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
|
|
This fixes the issue that `nix-build`, without experimental feature
'nix-command' enabled, recommends the experimental CLI `nix log` to view
build logs. Now it'll recommend the stable `nix-store -l` CLI instead.
Fixes https://github.com/NixOS/nix/issues/8118
|
|
This is needed in subsequent commits to allow the settings and CLI args
infrastructure itself to read this setting.
|
|
|
|
Additionally get rid of the evil time we made an empty
`OutputSpec::Names()`.
|
|
This should be a non-empty set, and so we don't want people doing this
by accident. We remove the zero-0 constructor with a little inheritance
trickery.
|
|
`DerivedPath::Built` and `DerivationGoal` were previously using a
regular set with the convention that the empty set means all outputs.
But it is easy to forget about this rule when processing those sets.
Using `OutputSpec` forces us to get it right.
|
|
libstore/derivation-goal: Elaborate a TODO for performance concern
|
|
This makes 'nix develop' set the Linux personality in the same way
that the actual build does, allowing a command like 'nix develop
nix#devShells.i686-linux.default' on x86_64-linux to work correctly.
|
