To construct various stores at runtime, we need to eliminate associated
types from the BlobService trait, and return Box<dyn …> instead of
specific types.
This also means we can't consume self in the close() method, so
everything we write to is put in an Option<>, and during the first close
we take from there.
Change-Id: Ia523b6ab2f2a5276f51cb5d17e81a5925bce69b6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8647
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Unfortunately, nixpkgs has at least one case[1] where the out environment
variable is shadowed -- though it doesn't cause a problem, since it's
shadowed with the correct value, odd as this may be!
[1]: c7c2984716/pkgs/development/python-modules/pybind11/default.nix (L19)
Change-Id: Ibf6790d2484dc9cce8e424feeb5886664d498dc3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8696
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
`tvix-store mount PATH` will mount the tvix-store to the given path.
Change-Id: Icb82a6b3cb8a22eec856c375a28ae5580403833f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8665
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This allows using a StorePath as a key in a hashmap.
Change-Id: Id3eed623da4e1fc44a970a3982c7caa21d2495c8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8666
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This brings in fuse (via the `fuser` crate), and adds pkg-config and
libfuse to the dev shell, so `cargo build` can link against it.
Change-Id: I0d11607490e27d946bdf92b0b9e45f9ab644ba74
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8664
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This testcase tests a missing blob fails the rendering, the comment has
been copied from elsewhere.
Change-Id: I48fa3fa454e12506590fa14a3591d156bafa8b5e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8722
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
The CLs did bitrot a bit, they're based on an older version of the
protocol, and it's unclear if they'll be a separate Go Binary, or just
another HTTP handler inside tvix-store itself, considering we now have
way more NAR juggling code than before.
Change-Id: I3632035cda8d75a8ff23b3132312f0f086d9e02f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8732
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This has moved to a `daemon` subcommand.
Change-Id: Iae9778d8a59e6bf84555119fabfd62db3917bb62
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8731
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
When comparing to C++ Nix, we notice that the thunking of default
expressions in function formals corresponds to their normal thunking,
e.g. literals are not thunked. This means that we can just invoke
compile() without much of a care and trust that it will sort it out
correctly.
If function formals blow up as a result of this, it likely indicates
that the expression is treated incorrectly by compile(), not
compile_param_pattern().
Change-Id: I64acbff2f251423eb72ce43e56a0603379305e1d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8704
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
C++ Nix forces and typechecks the passed argument even if it is not
necessary in order to compute the return value of the function. I
discovered this when I thought our formals miscompilation might be that
we are too strict, but doesn't look like it in this case.
Change-Id: Ifb3c92592293052c489d1e3ae8c7c54e4b6b4dc6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8701
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Before, the construction of a TwoByteWM would panic when no patterns
were provided, as in `tvix --expr 'builtins.toFile "snens" "soos"'`.
Change-Id: I25ed498c475523aec5baf8683b23059fadabb21c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8697
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This now creates different store client, depending on the cli
subcommand.
The `import` command will connect to the gRPC service, and the `daemon`
command will use the sled implementation.
It might make sense to define some URI syntax to make this configurable
by the user, via the CLI.
Change-Id: I72351fcf0e83a013b6aa67a90b64c108cbb01ffd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8619
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
We don't need to be able to clone these services in here.
Change-Id: Ifb69450f7ebdc8364cbf9cdfb6464f8560440e4c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8645
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
nix_oracle.rs now gives us the possibility to check this by stuffing the
expressions in a list. In fact, the incorrect behavior fixed in
- cl/8656
- cl/8655
- cl/8662
was discovered using this test suite.
Change-Id: Id0ab01ee6be0b875791214e0a72a2ac941c46c96
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8658
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This will be useful for comparing thunking behavior to C++ Nix. I
considered adding this capability to the tvix_tests/nix_tests
infrastructure, but as it would require changing the test file naming
scheme to do it in a clean way, I've postponed it–it's nice that our
tests are compatible with C++ Nix's test suite.
Change-Id: I60bcdd98ed25140e716f0858f8dc28f21ab957aa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8657
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
HasAttrs was weird because with longer attribute paths it would
sometimes not turn out to be a thunk. If it was a thunk, it'd usually
still do some eval strictly which we'll want to avoid.
Verified against C++ Nix using a new test suite introduced in a later
CL.
Change-Id: I6d047ccc68d046bb268462f170a3c4f3c5ddeffe
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8656
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Probably no real world code broken by this overzealous evaluation, but
let's be thorough!
Change-Id: Ib405a677182eab7940ace940c68e107573473a54
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8655
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Unary operator applications are thunked which can easily be observed by
nix-instantiate --eval -E '[ (!true) (-1) ]'
Unfortunately, there are few simple expressions where this makes a
difference in the end result. Thus it only cropped up when using nixpkgs
for cross compilation: Here we would compile the expression
!(stdenv.cc.isGNU or false)
to assemble python3Minimal's passthru attribute set (at least this seems
to be the most likely explanation from the backtraces I've studied).
This means that an unthunked
<stdenv.cc.isGNU or false>
OpForce
OpInvert
would be performed in order to assemble this attribute set, causing
stdenv.cc to be evaluated too early, causing an infinite recursion.
Resolves b/273.
It seems that having a test suite that doesn't use --strict and relies
on thunks rendered as <CODE> would be beneficial for catching such
issues. I've not been able to find a test case with --strict that
demonstrates the problem fixed in this CL.
Change-Id: I640a5827b963f5b9d0f86fa2142e75e3a6bbee78
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8654
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
This will prevent tvix from printing any warnings.
As a followup, we can also thread this parameter through into the
evaluator itself, to prevent warnings from being constructed in first
place.
Change-Id: I15381396f86573484bdd1a73d09034a665638e35
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8646
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
mapAttrs, map and genList call Nix functions provided by the caller and
store the result of applying them in a Nix data structure that does not
force all of its contents when forced itself. This means that when such
a builtin application is forced, the Nix function calls performed by the
builtin should not be forced: They may be forced later, but it is also
possible that they will never be forced, e.g. in
builtins.length (builtins.map (builtins.add 2) [ 1 2 3 ])
it is not necessary to compute a single application of builtins.add.
Since request_call_with immediately performs the function call
requested, Tvix would compute function applications unnecessarily before
this change. Because this was not followed by a request_force, the
impact of this was relatively low in Nix code (most functions return a
new thunk after being applied), but it was enough to cause a lot of
bogus builtins.trace applications when evaluating anything from
`lib.modules`. The newly added test includes many cases where Tvix
previously incorrectly applied a builtin, breaking a working expression.
To fix this we add a new helper to construct a Thunk performing a
function application at runtime from a function and argument given as
`Value`s. This mimics the compiler's compile_apply(), but does itself
not require a compiler, since the necessary Lambda can be constructed
independently.
I also looked into other builtins that call a Nix function to verify
that they don't exhibit such a problem:
- Many builtins immediately use the resulting value in a way that makes
it necessary to compute all the function calls they do as soon as
the outer builtin application is forced:
* all
* any
* filter
* groupBy
* partition
- concatMap needs to (shallowly) force the returned list for
concatenation.
- foldl' is strict in the application of `op` (I added a comment that
makes this explicit).
- genericClosure needs to (shallowly) force the resulting list and some
keys of the attribute sets inside.
Resolves b/272.
Change-Id: I1fa53f744bcffc035da84c1f97ed25d146830446
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8651
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Instead of instantiating it once in every loop iteration, put it in an
Arc, and clone that before passing it to the spawned task.
Change-Id: I5d9c838f27048726166fa50206d1edd5ed6849b5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8632
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This feature allows the compiler to detect situations where the
created thunk is useless and can be merged into the parent chunk
instead.
The only case where the compiler does this initially is when
statically optimising a select expression.
For example, previously the expression `builtins.length` compiled into
two thunks:
1. An "inner" thunk which contained an `OpConstant` that had the
optimised `length` builtin in it.
2. An "outer" thunk which contained an `OpConstant` to access the
inner thunk, and the trailing OpForce of the top-level program.
With this change, the inner thunk is skipped completely and the outer
chunk directly contains the `length` builtin access.
This can be applied in several situations, some easier than others,
and we will add them in as we go along.
Change-Id: Ie44521445fce1199f99b5b17712833faea9bc357
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7959
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This method extends the contents of one chunk with that of another,
effectively merging the thunks together.
This will be used for the upcoming "unthunking" functionality.
Change-Id: I6ad74232cd7f3eca198ed921e455205e00d76e6b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7958
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
With traverse_to not requiring a &mut anymore, we can drop the &mut self
in all these function signatures.
Change-Id: I22105376b625cb281c39e92d3206df8a6ce97a88
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8629
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
This operation is blocking, so it should be run inside a blocking tokio
task. Tokio panics if it detects a blocking operation inside a non-
blocking task, so cl/8619 would cause it to panic (as the GRPC clients
use spawn_blocking under the hood).
As spawn_blocking moves, and we can't clone `TvixStoreIO` (see cl/8614),
we create a new instance of TvixStoreIO inside each loop iteration.
Change-Id: I0c6548b3d4ac42d180d4c92314af8fd2b16510da
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8618
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This switches tvix-cli over from using `NixCompatIO` to `TvixStoreIO`.
For now, we simply instantiate in-memory services instead of getting
fancy with constructors inside tvix-store, but long-term, we might want
to support some URI syntax, to make this configurable at runtime.
nixpkgs eval tests might be fine (and fast!) with a purely in-memory
backend, but other usages might involve talking to a local tvix-store
over gRPC (using the gRPC client, either unix domain socket or even
further away remote), or running tvix-store in "embedded" mode (using
another client than the gRPC client).
Change-Id: I509afd3dc5ce3f2d52b0fb7067748fab820e26ab
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8572
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This allows closing a TvixStoreIO if it contains a SledPathInfoService.
Change-Id: Ife451eda331bafdb1af91f45a94cccd13f2f67c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8620
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
We already evaluate this the same as Nix does, so worth adding it into
CI.
Change-Id: I529faccac6e5e16e0bc985ab4c3e0cd07bb23293
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8624
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
This introduces a function that can be instantiated with an attribute
path to instantiate, as well as the expected path (normally interpolated
with the nix evaluator).
Check both pkgs.stdenv.drvPath and pkgs.stdenv.outPath to match.
Change-Id: Id667ed35fa159ff83fedb3017ef8d3271aa42695
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8606
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
We have code.namespace as a field, set to
tvix_store::directoryservice::sled, so there's no need to repeat the
name here.
Change-Id: Ic1aa8a2b24de439c6a189966bd773e9acf49d1e8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8623
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
This asks a remote tvix-store for the nar size and digest of a given
root node.
Change-Id: If9f916d9bfc5f8dc3166e2c6c1671c0f0124d1c1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8611
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This introduces a new struct, B3Digest, which internally holds a
Vec<u8>, but only allows construction with 32 bytes.
It also implements display, which will print the base64 representation.
This should reduce some boilerplate when parsing Vec<u8>.
Change-Id: Ia91aa40cb691916773abc8f93e6ed79a5fd34863
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8592
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
These two places didn't add the path from the context to the
ErrorKind, but simply relied on the
impl From<std::io::Error> for tvix_eval::ErrorKind, which doesn't add
the path.
Change-Id: Ifc7dbbe305d24242b0705de1dea34e8923e9d2cb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8603
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
We didn't return anything useful other than ErrorKind::IO anyways.
We can use io::ErrorKind::Unsupported for DummyIO.
Fixes b/271.
Change-Id: Icb231e9b38168e8b6fa473bfa405d160357b317f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8602
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
One test in here assumed the server was fast, but when very busy, a
computer running these tests might not be fast.
Treat both cases that can occur separately.
Change-Id: Iba168ad39f2458c4fb8873211df33beeaff7c6c1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8595
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Also, get rid of the explicit byte comparison here, but unwrap like in
the rest of the codebase. We'll deal with this in a generic manner in
b/267 and b/189.
Change-Id: Ie5f3d27ab35b7e88d67a2796c29cdd7bc7df71f3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8589
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This remained around from some previous time where this type was being
cloned around directly, but it's not anymore.
Change-Id: I2c61cf9cecb8e5d7d08644ed20642ee61357bc21
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8580
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This adds a wrapper type TvixIO<T: EvalIO>, which can wrap around an
arbitrary EvalIO implementation and perform actions needed for the
Tvix CLI (marking imported paths as known, and handling __corepkgs__).
Change-Id: I5fc1ca199b9f94b21a89103b84575e0f8f58dff9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8579
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This walks from a node further down until it reaches the requested path.
Change-Id: I2f9a15a8601db4d06c95d7b47cd6153264e203e3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8568
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This distinguishes it better from the EvalIO::import_path method.
Also update the docstring to explain what it does (and what it doesn't).
Change-Id: I32a8b2869fa67a894df28532b22bf170961a2abf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8578
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This was missed while renaming NixPath to StorePath.
Change-Id: Ibcc929c43b111e4370e8222c1dd86d403548367f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8577
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
We currently only support querying by the output hash digest.
This makes the interface a bit simpler.
Change-Id: I80b285373f1923e85cb0e404c4b15d51a7f259ef
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8570
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This allows decomposing a path consisting of a store path AND a suffix
into a StorePath, and a PathBuf containing the rest.
Change-Id: I81290e2fd804cdc9d1e88c71cb22c0fb882d7936
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8567
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Make it cleaner that StorePath only does encode the first path component
after the STORE_DIR prefix. Also, move some of the comments around a
bit, so it makes more sense what's using what.
Change-Id: Ibb57373a13526e30c58ad561ca50e1336b091d94
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8566
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
It's okay if these calls mutate some internal state inside an
implementation.
Change-Id: I12bb11bde0310778c3da1275696bf7de058863a3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8571
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Make it more explicit that we expect the from_string calls to fail here.
Change-Id: Ib3d46fc0850e364125e3548670ef301eeea2e45c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8565
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This connects to a (remote) tvix-store BlobService over gRPC.
Change-Id: If31f706738a5c3445886c117feca8b61f3203e9e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8552
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Whether chunking is involved or not, is an implementation detail of each
Blobstore. Consumers of a whole blob shouldn't need to worry about that.
It currently is not visible in the gRPC interface either. It
shouldn't bleed into everything.
Let the BlobService trait provide `open_read` and `open_write` methods,
which return handles providing io::Read or io::Write, and leave the
details up to the implementation.
This means, our custom BlobReader module can go away, and all the
chunking bits in there, too.
In the future, we might still want to add more chunking-aware syncing,
but as a syncing strategy some stores can expose, not as a fundamental
protocol component.
This currently needs "SyncReadIntoAsyncRead", taken and vendored in from
https://github.com/tokio-rs/tokio/pull/5669.
It provides a AsyncRead for a sync Read, which is necessary to connect
our (sync) BlobReader interface to a GRPC server implementation.
As an alternative, we could also make the BlobReader itself async, and
let consumers of the trait (EvalIO) deal with the async-ness, but this
is less of a change for now.
In terms of vendoring, I initially tried to move our tokio crate to
these commits, but ended up in version incompatibilities, so let's
vendor it in for now.
Change-Id: I5969ebbc4c0e1ceece47981be3b9e7cfb3f59ad0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8551
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
We already returned UnexpectedEof in case the reader stopped returning
bytes too early, but similarly we should also fail if there's still
bytes left to be read in the reader passed.
We normally use the NAR writer to produce new NAR files, so the readers
point to the blobs we actually want to render, and having some data left
in there should be an error.
If for some reason the reader points to more data than just the blob,
the `.take` method can be used to limit it to the (known) size.
Change-Id: I9e8fa0a6dd9c794492abb6dc9e55995e619cb3bb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8553
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Before there was code scattered about (e.g. text hashing module and
derivation output computation) constructing store paths from low level
building blocks --- there was some duplication and it was easy to make
nonsense store paths.
Now, we have roughly the same "safe-ish" ways of constructing them as
C++ Nix, and only those are exposed:
- Make text hashed content-addressed store paths
- Make other content-addressed store paths
- Make input-addressed fixed output hashes
Change-Id: I122a3ee0802b4f45ae386306b95b698991be89c8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8411
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
The logic validating connectivity of Directory nodes should be moved
to SimplePutter, and this use whatever DirectoryPutter the store comes
with.
Change-Id: Id68a86a96cc49ff73920017839788859ea9c5161
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8358
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This should allow import_path to communicate to a gRPC remote store,
that actually verifies the Directory nodes are interconnected.
Change-Id: Ic5d28c33518f50dedec15f1732d81579a3afaff1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8357
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This provides a handle to upload multiple proto::Directory as part of
the same closure.
Change-Id: I9213dde257a260c8622239918ea541064b270484
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8356
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
When building store paths we can just construct the thing.
Change-Id: Ife5d461d6a440ecbb22f32a86a6d51d212a2035b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8409
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
They can go under `nixhash`
Change-Id: Ia15835c57130b66d58f5df80ae9595dceee00941
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8408
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
It is moved into `store_path::utils` with the other path builders.
Change-Id: I3257170e442af5d83bcf79e63fa7387dd914597c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8410
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This switches out the previous compressed representation (count of
instructions per span) with a representation where the chunk's span
list stores the index of the first operation that belongs to a span,
and finds the right span by using a binary search when looking them
up.
This improves the lookup complexity from O(n) to O(log n).
This improvement was suggested and (mostly) implemented by GPT-4. I
only fixed up some names and updated the logic for deleting
spans (which it only did not do because I didn't tell it about that).
The code was verified by producing a complex error before/after the
change and ensuring that all spans in the error match exactly.
Co-Authored-By: GPT-4
Change-Id: Ibfa12cc6973af1c9b0ae55bb464d1975209771f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8385
Reviewed-by: ezemtsov <eugene.zemtsov@gmail.com>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
This doesn't have anything to do with ATerms, we just happen to be using
the aterm representation of a Derivation as contents.
Moving this into store_path/utils.rs makes these things much cleaner -
Have a build_store_path_from_references function, and a
build_store_path_from_fingerprint helper function that makes use of it.
build_store_path_from_references is invoked from the derivation module
which can be used to calculate the derivation path.
In the derivation module, we also invoke
build_store_path_from_fingerprint during the output path calculation.
Change-Id: Ia8d61a5e8e5d3f396f93593676ed3f5d1a3f1d66
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8367
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This only added a suffix to the input argument, if build_store_path was
building the path of a Derivation.
As we need to also add the `.drv` suffix to the name we pass into
text_hash_string inside calculate_derivation_path, we can simply add the
suffix there and drop the parameter from build_store_path.
Change-Id: Icd5343dd1458f112b9296b389e81ce2ebdd16a9f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8365
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Use it to calculate the text_hash_string, which is then used in the
calculate_derivation_path and path_with_references functions.
Relates to b/263.
Change-Id: I7478825e2a23a11224212fea5e3fd06daa97d5e5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8364
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
… and keep the pub exports as is.
Change-Id: I2ad21660577553395f05b5ba71083626429b0dfc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8363
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
… and keep the pub exports as is.
Change-Id: I9f89a738c508c478ddba61303c21ea294f01ee9f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8362
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
We do compare for equality. This comment probably was when I tried to
compare the `Result<T, E>`, and as `E` doesn't derive PartialEq it was
annoying.
Change-Id: I18bb19528c76af91c9d24d88d55dd46d0c092d20
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8354
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This moves the recursive BFS traversal of Directory closures from the
GRPCDirectoryServiceWrapper out into a a DirectoryTraverser struct
implementing Iterator.
It is then used from various implementors of DirectoryService in the
`get_recursive()` method.
This allows distinguishing between recursive requests and non-recursive
requests in the gRPC client trait implementation.
Change-Id: I50bfd4a0d9eb11832847329b78c587ec7c9dc7b1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8351
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This grows the frame stack as the call stack grows, which yields *much*
better user-facing error messages.
I haven't measured the performance impact this has yet, for now I'm
still just trying to add more information to errors and then cut down
again where necessary.
Change-Id: I89f058ef31979edacf4667775d460b60704ce4d7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8334
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
This provides a GRPCDirectoryService struct implementing
DirectoryService, allowing a client to Directory objects from a (remote)
tvix-store.
Remote in this case is anything outside the current process, be it
another process, or an endpoint on the network.
To keep the sync interface in the `DirectoryService` trait, a handle to
some tokio runtime needs to be passed into the constructor, and the two
methods use `self.tokio_handle.spawn` to start an async function, and
`self.tokio_handle.block_on` to wait for its completion.
The client handle, called `grpc_client` itself is easy to clone, and
treats concurrent requests internally. This means, even though we keep
the `DirectoryService` trait sync, there's nothing preventing it from
being used concurrently, let's say from multiple threads.
There's still two limitations for now:
1) The trait doesn't make use of the `recursive` request, which
currently leads to a N+1 query problem. This can be fixed
by `GRPCDirectoryService` having a reference to another
`DirectoryService` acting as the local side.
I want to wait for general store composition code to pop up before
manually coding this here.
2) It's currently only possible to put() leaf directory nodes, as the
request normally requires uploading a whole closure. We might want
to add another batch function to upload a whole closure, and/or do
this batching in certain cases. This still needs some more thinking.
Change-Id: I7ffec791610b72c0960cf5307cefbb12ec946dc9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8336
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
