Commit graph

418 commits

Author SHA1 Message Date
John Ericson
70f8b96c11 Factor out UnkeyedValidPathInfo and test
This makes the path info serialisers ideomatic again, which allows me to
test them.
2023-10-20 15:21:04 -04:00
John Ericson
596bd469cc Move ValidPathInfo serialization code to worker-protocol.{cc.hh}
It does not belong with the data type itself.

This also materializes the fact that `copyPath` does not do any version
negotiation just just hard-codes "16".

The non-standard interface of these serializers makes it harder to test,
but this is fixed in the next commit which then adds those tests.
2023-10-20 15:20:48 -04:00
John Ericson
ab822af0df Factor out serialization for BuildResult
Worker Protocol:

Note that the worker protocol already had a serialization for
`BuildResult`; this was added in
a4604f1928. It didn't have any versioning
support because at that time reusable seralizers were not away for the protocol
version. It could thus only be used for new messages also introduced in
that commit.

Now that we do support versioning in reusable serializers, we can expand
it to support all known versions and use it in many more places.

The exist test data becomes the version 1.29 tests: note that those
files' contents are unchanged. 1.28 and 1.27 tests are added to cover
the older code-paths.

The keyered build result test only has 1.29 because the keying was also
added in a4604f19284254ac98f19a13ff7c2216de7fe176; the older
serializations are always used unkeyed.

Serve Protocol:

Conversely, no attempt was made to factor out such a serializer for the
serve protocol, so our work there in this commit for that protocol
proceeds from scratch.
2023-10-20 15:19:28 -04:00
John Ericson
4372738efe Systematize the worker protocol derived path serialiser
It was some ad-hoc functions to account for versions, while the already
factored-out serializer just supported the latest version.

Now, we can fold that version-specific logic into the factored out one,
and so we do.
2023-10-20 15:19:28 -04:00
Yueh-Shun Li
e026f3e1ae treewide: Reference HashFormat members with scope
Base* -> HashFormat::Base*
2023-10-19 00:56:41 +08:00
John Ericson
80d7994f52 Special-case error message to add extra information
The Derivation parser and old ATerm unfortunately leaves few ways to get
nice errors when an old version of Nix encounters a new version of the
format. The most likely scenario for this to occur is with a new client
making a derivation that the old daemon it is communicating with cannot
understand.

The extensions we just created for dynamic derivation deps will add a
version field, solving the problem going forward, but there is still the
issue of what to do about old versions of Nix up to now.

The solution here is to carefully catch the bad error from the daemon
that is likely to indicate this problem, and add some extra context to
it.

There is another "Ugly backwards compatibility hack" in
`remote-store.cc` that also works by transforming an error.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
2023-09-07 10:39:37 -04:00
John Ericson
60b7121d2c Make the Derived Path family of types inductive for dynamic derivations
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>
2023-08-10 00:08:32 -04:00
John Ericson
60d8dd7aea Clean up store hierarchy with IndirectRootStore
See the API doc comments for details.
2023-07-24 09:19:44 -04:00
John Ericson
13269ba93b Make RemoteStore::ConnectionHandle part of class and expose
Will need to do subclass-specific implementations in the next commit.
This isn't because there will be multiple variations of the daemon
protocol (whew!) but because different clients pick and choose different
parts to use.
2023-07-24 09:17:01 -04:00
John Ericson
6bc98c7fba Give queryPartialDerivationOutputMap an evalStore parameter
This makes it more useful. In general, the derivation will be in one
store, and the realisation info is in another.

This also helps us avoid duplication. See how `resolveDerivedPath` is
now simpler because it uses `queryPartialDerivationOutputMap`. In #8369
we get more flavors of derived path, and need more code to resolve them
all, and this problem only gets worse.

The fact that we need a new method to deal with the multiple dispatch is
unfortunate, but this generally relates to the fact that `Store` is a
sub-par interface, too bulky/unwieldy and conflating separate concerns.
Solving that is out of scope of this PR.

This is part of the RFC 92 work. See tracking issue #6316
2023-07-20 15:59:52 -04:00
John Ericson
9f69b7dee9 Create worker_proto::{Read,Write}Conn
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.
2023-06-19 12:08:23 -04:00
John Ericson
95eae0c002 Put worker protocol items inside a WorkerProto struct
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>
2023-06-19 12:08:23 -04:00
John Ericson
469d06f9bc Split out worker protocol template definitions from declarations
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.
2023-06-19 11:45:59 -04:00
John Ericson
cb5052d98f Revert "Revert "Use template structs instead of phantoms""
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 45a0ed82f0. That
commit in turn reverted 9ab07e99f5.
2023-05-17 22:44:47 -04:00
John Ericson
904878d6d2 Move worker_proto defs out of remote-store.cc to own file
These items are not templates, and they declared in
`worker-protocol.hh`; therefore they should live in a
`worker-protocol.cc`.

Anything else needlessly diverges from convention. After all, it is not
like this code is only used in `remote-store.cc`; it is also used in
`daemon.cc`. There is no good reason to place it with the client
implementation or the server implementation when it used equally by
both.
2023-05-17 10:36:03 -04:00
John Ericson
e514b3939a Add name to some error messages 2023-05-09 13:24:53 -04:00
John Ericson
668377f217 TextHashMethod -> TextIngestionMethod, gate with XP feature
I suppose we can use `dynamic-derivations` for the few things we neeed.
2023-04-17 19:02:45 -04:00
John Ericson
f56c4a5bdf Merge remote-tracking branch 'upstream/master' into ca-drv-exotic 2023-04-17 18:10:12 -04:00
Robert Hensing
3f9589f17e
Merge pull request #6312 from obsidiansystems/keyed-build-result
Shuffle `BuildResult` data definition, make state machine clearer, introduce `SingleDrvOutputs`
2023-04-17 18:08:05 +02:00
John Ericson
e12efa3654 Merge remote-tracking branch 'upstream/master' into ca-drv-exotic 2023-04-17 10:16:57 -04:00
Robert Hensing
e641de085b
Merge pull request #3746 from obsidiansystems/path-info
Introduce `StoreReferences` and `ContentAddressWithReferences`
2023-04-17 15:49:48 +02:00
John Ericson
24866b71c4 Introduce SingleDrvOutputs
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.
2023-04-15 12:51:19 -04:00
John Ericson
37fca662b0 Make KeyedBuildResult, BuildResult like before, and fix bug another way
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>
2023-04-15 11:01:31 -04:00
John Ericson
ee420ac64e Legacy vs non-legacy to_string/parse for DerivedPath
As requested by @roberth, it is good to call out the specific instances
we care about, which is `!` for the RPC protocols, and `^` for humans.

This doesn't take advantage of parametricity as much, but since the
human and computer interfaces are good to decouple anyways (we don't
care if they drift further apart over time in the slightest) some
separation and slight duplication is fine.

Also, unit test both round trips.
2023-04-14 20:45:11 -04:00
John Ericson
fd21f9d76e Merge remote-tracking branch 'upstream/master' into path-info 2023-04-07 20:39:04 -04:00
matthewcroughan
9207f94582 Add Store::isTrustedClient()
This function returns true or false depending on whether the Nix client
is trusted or not. Mostly relevant when speaking to a remote store with
a daemon.

We include this information in `nix ping store` and `nix doctor`

Co-Authored-By: John Ericson <John.Ericson@Obsidian.Systems>
2023-04-06 19:59:57 -04:00
John Ericson
eeecfacb43 Merge branch 'path-info' into ca-drv-exotic 2023-04-01 16:40:32 -04:00
John Ericson
f7f44f7c96 Merge commit 'aa99005004bccc9be506a2a2f162f78bad4bcb41' into ca-drv-exotic 2023-04-01 15:15:32 -04:00
John Ericson
c51d554c93 Use "raw pattern" for content address types
We weren't because this ancient PR predated it!

This is actually a new version of the pattern which addresses some
issues identified in #7479.
2023-03-30 17:12:49 -04:00
John Ericson
296831f641 Move enabled experimental feature to libutil struct
This is needed in subsequent commits to allow the settings and CLI args
infrastructure itself to read this setting.
2023-03-20 11:05:22 -04:00
John Ericson
5abd643c6d Merge branch 'path-info' into ca-drv-exotic 2023-02-28 12:46:00 -05:00
Shea Levy
92edc38369
Don't send plugin-files to the daemon.
This is radically unsafe and the daemon has already loaded its plugins
anyway.

Fixes cachix/devenv#276
2023-02-01 20:05:56 -05:00
John Ericson
7c82213813 Merge branch 'path-info' into ca-drv-exotic 2023-01-14 17:09:58 -05:00
John Ericson
b3d91239ae Make ValidPathInfo have plain StorePathSet references like before
This change can wait for another PR.
2023-01-14 16:42:03 -05:00
John Ericson
056cc1c1b9 Merge remote-tracking branch 'upstream/master' into path-info 2023-01-14 14:27:28 -05:00
John Ericson
5ba6e5d0d9 Remove default constructor from OutputsSpec
This forces us to be explicit.

It also requires to rework how `from_json` works. A `JSON_IMPL` is added
to assist with this.
2023-01-11 19:08:19 -05:00
John Ericson
ce2f91d356 Split OutputsSpec and ExtendedOutputsSpec, use the former more
`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.
2023-01-11 18:57:18 -05:00
John Ericson
81727f85cb Merge branch 'path-info' into ca-drv-exotic 2023-01-06 15:45:34 -05:00
John Ericson
46e942ff9e Do big rename to clean up code
- `PathReferences` -> `References`

- `PathReferences<StorePath>` -> `StoreReference`

- `references` -> `others`

- `hasSelfReference` -> `self`

And get rid of silly subclassing
2023-01-06 15:36:05 -05:00
John Ericson
989b8065b4 Merge branch 'path-info' into ca-drv-exotic 2023-01-06 10:56:22 -05:00
John Ericson
e9fc1e4fdb Merge remote-tracking branch 'upstream/master' into path-info 2023-01-06 10:35:20 -05:00
Théophane Hufschmitt
8cac451fce Fix why-depends for CA derivations (again)
This has the same goal as b13fd4c58e81b2b2b0d72caa5ce80de861622610,but
achieves it in a different way in order to not break
`nix why-depends --derivation`.
2023-01-02 17:42:22 +01:00
Eelco Dolstra
703d863a48 Trivial changes from the lazy-trees branch 2022-12-07 14:06:34 +01:00
Eelco Dolstra
4c96761c2b Merge remote-tracking branch 'origin/master' into parallel-nix-copy 2022-08-22 15:29:10 +02:00
Eelco Dolstra
53e7b7e8ac Remove warnLargeDump()
This message was unhelpful (#1184) and probably misleading since
memory is O(1) in most cases now.
2022-08-17 11:32:01 +02:00
Théophane Hufschmitt
cb0553ecd0 Restore the "low-latency" ssh copying 2022-06-08 14:03:46 +02:00
Eelco Dolstra
6378f0bb32
RemoteStore::queryRealisationUncached(): Fix potential noexcept violation 2022-05-30 13:27:13 +02:00
Alain Zscheile
1385b20078
Get rid of most .at calls (#6393)
Use one of `get` or `getOr` instead which will either return a null-pointer (with a nicer error message) or a default value when the key is missing.
2022-05-04 07:44:32 +02:00
John Ericson
938650700f Merge branch 'path-info' into ca-drv-exotic 2022-03-10 16:20:01 +00:00
John Ericson
8ba089597f Merge remote-tracking branch 'upstream/master' into path-info 2022-03-10 15:48:14 +00:00