The public-consumable thing here is ChunkedReader, not ChunkedBlob.
ChunkedBlob is a helper that can be used to get a new AsyncRead, but
not AsyncSeek. It is used internally by ChunkedReader whenever the
client seeks.
Make this more obvious, by extending the documentation, and putting
ChunkedReader at the top of this file.
Also make ChunkedBlob and its methods private, and give ChunkedReader a
more useful constructor (from_chunks, instead of from_chunked_blob).
Change-Id: I2399867591df923faa73927b924e7c116ad98dc0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11079
Tested-by: BuildkiteCI
Reviewed-by: Brian Olsen <me@griff.name>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
The primary addition is the `builtin_hashFile` function which attempts
value-path coercion and on success reads the file to a nix_string.
Similar to the `builtins_hashString` implementation, the hash is then
derived using the `hash_nix_string` function in the introduced hash
module which has extracted hashing functionality initially specified in
cl/11005.
Change-Id: I5e3ef0317223af99488ebb79efc2fb49b4cbc1b6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11007
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Implements md5, sha1, sha256 and sha512 using the related crates from
the RustCrypto hashes project (https://github.com/RustCrypto/hashes)
Change-Id: I00730dea44ec9ef85309edc27addab0ae88814b8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11005
Tested-by: BuildkiteCI
Reviewed-by: aspen <root@gws.fyi>
To pave the way for adding a new error type for builtins in this crate,
move DerivationError to a new builtins::errors module.
Change-Id: I65fcad63e43ed40ad39c2c6540a2ab80fdd90fd4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11016
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: aspen <root@gws.fyi>
Move the the `fixed:out:[r:]{}:` generation to a helper function,
use matches! for more clarity.
Change-Id: I4e930c42aacbf5c7451d1f8c8c80ccb4c45389f0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11006
Tested-by: BuildkiteCI
Reviewed-by: aspen <root@gws.fyi>
Autosubmit: flokli <flokli@flokli.de>
Previously, Nix strings were represented as a Box (within Value)
pointing to a tuple of an optional context, and another Box pointing to
the actual string allocation itself. This is pretty inefficient, both in
terms of memory usage (we use 48 whole bytes for a None context!) and in
terms of the extra indirection required to get at the actual data. It
was necessary, however, because with native Rust DSTs if we had
something like `struct NixString(Option<NixContext>, BStr)` we could
only pass around *fat* pointers to that value (with the length in the
pointer) and that'd make Value need to be bigger (which is a waste of
both memory and cache space, since that memory would be unused for all
other Values).
Instead, this commit implements *manual* allocation of a packed string
representation, with the length *in the allocation* as a field past the
context. This requires a big old pile of unsafe Rust, but the payoff is
clear:
hello outpath time: [882.18 ms 897.16 ms 911.23 ms]
change: [-15.143% -13.819% -12.500%] (p = 0.00 < 0.05)
Performance has improved.
Fortunately this change can be localized entirely within
value/string.rs, since we were abstracting things out nicely.
Change-Id: Ibf56dd16c9c503884f64facbb7f0ac596463efb6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10852
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: aspen <root@gws.fyi>
This reverts commit d9565a4d0a.
Reason for revert: this was intentional - putting Rc::clone instead of
.clone is a common Rust idiom, and makes it explicit that we're cloning
a shared reference, not an underlying resource
Change-Id: I41a5f323ee35d7025dc7bb02f7d5d05d0051798d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10995
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
With our values using bstr now, we're not restricted to only reading
files that contain valid UTF-8.
Update our `read_to_string` function to `read_to_end`
(named like `std::io::Read::read_to_end`), and have it return a Vec<u8>.
Change-Id: I87f0291dc855a132689576559c891d66c30ddf2b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11003
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Pádraic Ó Mhuiris <patrick.morris.310@gmail.com>
Reviewed-by: flokli <flokli@flokli.de>
...in `Derivation`.
This is more type-safe and should consume less memory.
This also removes some allocations in the potentially hot path of output hash calculation.
https: //b.tvl.fyi/issues/264
Change-Id: I6ad7d3cb868dc9f750894d449a6065608ef06e8c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10957
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Peter Kolloch <info@eigenvalue.net>
Reviewed-by: Peter Kolloch <info@eigenvalue.net>
...so that we can also use `StorePath`s in
derivation.input_derivations.
Towards https://b.tvl.fyi/issues/264
Change-Id: I71d296ca273979c70f277a7f4f88a5f76de3d8be
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10973
Reviewed-by: Peter Kolloch <info@eigenvalue.net>
Autosubmit: Peter Kolloch <info@eigenvalue.net>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
One last place where we needed to wrap the forcing of the element of a
list in `try_value!`. This fixes a previously `notyetpassing` test
Change-Id: I8827a3e39630e6959013b70bdfa9cbcb93f4e91c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10789
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
We add a new set of builtins called `import_builtins`, which
will contain import-related builtins, such as `builtins.path` and
`builtins.filterSource`. Both can import paths into the store, with
various knobs to alter the result, e.g. filtering, renaming, expected
hashes.
We introduce `filtered_ingest` which will drive the filtered ingestion
via the Nix function via the generator machinery, and then we register
the root node to the path info service inside the store.
`builtins.filterSource` is very simple, `builtins.path` is a more
complicated model requiring the same logic albeit more sophisticated
with name customization, file ingestion method and expected SHA-256.
Change-Id: I1083f37808b35f7b37818c8ffb9543d9682b2de2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10654
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
(Re-)Adds an error variant that wraps a catchable error kind, which is
used for returning the result of an evaluation.
Previously this would return the internal catchable value, which would
lead to panics if users tried to use these. Somehow this was missed; I
think we need error output tests.
Change-Id: Id6e24aa2ce4ea4358a29b2e1cf4a6749986baf8c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10991
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
All these functions have an Rc of Rc<TvixStoreIO> or Rc<dyn StoreIO>, so
we can call io.clone() directly.
Change-Id: I96a5bcb9af4aca93892cb72bcfaf14540da52381
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10989
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This displays how users can configure an impure evaluation for
tvix-serde, which makes it possible to use e.g. `nixpkgs/lib`.
We might want to add an example showing how the full Nix-glue
compatibility stuff can be added here, too.
Change-Id: I2224a3fc66e739969d4c723c3d9d8127a046b6fd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10994
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
The previous behaviour (enabling `import`, but not allowing e.g.
`<nixpkgs/lib>` to resolve) was very confusing.
Now imports from NIX_PATH become enabled by default, unless the user
already overrode that behaviour with something else by setting
`Evaluation::nix_path` manually.
Change-Id: Iad970beb633d9887be4b185b01e6f5858d81bea3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10993
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
There's some code path where fancy_format_str()->Error::Display, which
recurses forever and overflows the stack. This was introduced in a
previous commit today.
Change-Id: I87a59492099f6c138c752478901b9aa614bb57cc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10990
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: Peter Kolloch <info@eigenvalue.net>
Makes use of the SourceCode field now being stored directly in
errors (see parent CL). With this change, the default `Display`
implementation can now format errors correctly, and there is no need
to keep a `SourceCode` around just for error formatting.
Updates dependent crates (CLI, serde, tvixbolt) to use this correctly.
Change-Id: Iddc5d7a6b4bab391f30a999e4c68aca34304c059
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10987
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
With this change it's no longer necessary to track the SourceCode
struct separately from the evaluation for error reporting: It's just
stored directly in the errors.
This also ends up resolving an issue in compiler::bindings, where we
cloned the Arc containing file references way too often. In fact those
clones probably compensate for all additional SourceCode clones during
error construction now.
Change-Id: Ice93bf161e61f8ea3d48103435e20c53e6aa8c3a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10986
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Some implementations of DirectoryService might not allow retrieval of
intermediate Directory nodes, that are not at the "root".
Think about an object store implementation. The client is doing a
get_recursive anyways to reduce the number of roundtrips.
By documenting the fact we don't need to support looking up intermediate
Directory messages, we can just batch all directories into the same
object, keyed by the root.
Change-Id: I019d720186d03c4125cec9191e93d20586a20963
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10988
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This makes some of the work of configuring an arbitrary I/O handler
easier.
Change-Id: I158db3235fe83df6e709578ed515e0e028c20086
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10959
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Rather than debug-formatting a Vec<u8>, which gives just a sequence of
numbers (not very helpful!) use String::from_utf8_lossy to give a much
more human-readable error message
Change-Id: I6cae795ebe2ab9a656c087b82a143411e6c7c81a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10980
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Initialize a new empty builtins module `fetcher_builtins`, which will
contain the builtins which fetch URLs from the internet:
* fetchurl
* fetchGit
* fetchTarball
* fetchTree (maybe? this is experimental)
These builtins are all implemented in CPP nix at:
https://github.com/NixOS/nix/blob/2.20.2/src/libexpr/primops/fetchTree.cc
These builtins are added to the evaluation context using a similar
mechanism to the derivation builtins, and have been added everywhere
derivation builtins were previously being added.
Change-Id: I133b91cc9560f23028621414537f712e7bd8a825
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10974
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
...so that we can parse (and at the same time
validate) store paths directly to `StorePath`s.
https: //b.tvl.fyi/issues/264
Change-Id: Ide4cb5403f8e3400f7d2a5838c652ac35dbd1a4f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10968
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Peter Kolloch <info@eigenvalue.net>
Tested-by: BuildkiteCI
That's one possible abstraction to drive builds.
Whenever IO into a store path is requested, we look up the root node,
and in case we don't have it in PathInfoService, but KnownPaths gives us
a Derivation for that output path, trigger a build and await the result.
This recursively might trigger builds for parent paths if they haven't
been built yet.
Another option would be to simply expose a PathInfoService interface for
a builder too, and loop all building into IO via PathInfoService
composition - but let's start with something.
Note tvix-cli doesn't have a configurable BuildService yet, it's plugged
to the DummyBuildService, so whenever it needs to do a build, it'll fail,
but that's how it can be provoked:
```
(builtins.readFile (import <nixpkgs> {}).hello.outPath + "/bin/hello")
[…]
error[E029]: I/O error: /nix/store/cg8a576pz2yfc1wbhxm1zy4x7lrk8pix-hello-2.12.1: builds are not supported with DummyBuildService
--> [code]:1:2
|
1 | (builtins.readFile (import <nixpkgs> {}).hello.outPath + "/bin/hello")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```
Note how this fails, while pure output path calculation
(`(import <nixpkgs> {}).hello.outPath + "/bin/hello")`) still succeeds.
Change-Id: Id2075d8a2b18554d0dd608b4b29146a8cd411e7f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10793
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Log levels can be statically disabled at compile time via Cargo
features.
Do this for everything more verbose than info in release mode.
Change-Id: I57d4282b4e19a8d44f68da2f15c7534dd3e08977
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10955
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This otherwise gets a bit spammy.
Change-Id: I288350a600d79a394c239f253424ad55bc3cefc5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10954
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This causes a bit too much spam otherwise.
Change-Id: If3ced9ddfee7f49453711cd26469d1eb81983c71
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10953
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Necessary, if we want to use it inside of `Derivation` etc.
Change-Id: I8888060417b2ee83ac52d7ec3e7b27c393271d8b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10947
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Peter Kolloch <info@eigenvalue.net>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
No otlp yet, this simply gives us structured log output and a cli
argument for the log level.
Change-Id: Ifaa60bae419640e92baebb6ee59eedd775c769c2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10853
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Print store paths with their ToString implementation for brevity, and
don't log the sucessful return value of read_to_string.
Change-Id: I01b6838398acd66b8818095622f361fcca26fa77
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10854
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Passing a StorePathRef is annoying if we only (already) have a
StorePath.
Change-Id: Ic3b36c0041707230515a6745a57f0d25b2bafd16
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10948
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Reviewed-by: Peter Kolloch <info@eigenvalue.net>
Instead of constructing a completely new derivation for hashing,
allow to call hashing with substituted input_derivations.
This
1. reduces the number of allocations,
2. prepares substituting `String`s for store paths with proper `StorePath`s without needing a separate derivation struct that allows hashes without `/nix/store` in input_derivations, and
3. keeps the change local to the `Derivation` implementation.
Change-Id: I36732c78f98fc59f0925b65823773222782017b0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10935
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Peter Kolloch <info@eigenvalue.net>
This will be used inside glue, to populate the CA field.
Change-Id: I52350d66ad09c90c4e9f3bc6ea44fba4427f1576
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10946
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
We need to not only store a map from drv path to hash derivation modulo,
but also keep the original Derivation struct - because we'll use that
later to trigger builds.
Change-Id: I78e2e8431ab5ae853188866b797a79025200de98
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10790
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Allow using a BUILD_SERVICE_ADDR env var, or cmdline args to configure
it. Still default to the dummy implementation.
Change-Id: I68f34f7b09eabef2b0491103857bbc798398ebfc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10846
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Populate the input_derivations struct field in-place, using
self.input_derivations.from_iter, which seems a bit more readable.
Change-Id: I4aa12cfd66e08486ef27aedab1bb731d94b63657
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10934
Reviewed-by: flokli <flokli@flokli.de>
Reviewed-by: Peter Kolloch <info@eigenvalue.net>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This is was dropped accidentially, but we actually want to run tests in
debug mode, to check for overflows.
Change-Id: Ic0cf2d4a263bfd71f3453ed335d8be9dacd945e8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10932
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Now that I've done a ton of things to make sure Value is small on the
stack (16 bytes, which is a perfectly reasonable size for a programming
language Value enum), add a test asserting it stays that way.
These size improvements have a measurable impact, too - here's the
`hello outpath` benchmark compared between canon (as of r/7495) and this
commit:
hello outpath time: [990.56 ms 995.83 ms 1.0070 s]
change: [-7.1397% -6.1302% -5.1651%] (p = 0.00 < 0.05)
Performance has improved.
Change-Id: If99a0976eab28eb5e516fcd2f4a0e068145af23e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10799
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
This is now the only enum variant for Value that is larger than 8
bytes (it's 16 bytes), so boxing it (especially since it's not
perf-critical) allows us to get the Value size down to only 16 bytes!
Change-Id: I98598e2b762944448bef982e8ff7da6d6683c4aa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10798
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: aspen <root@gws.fyi>
This reverts commit d3d41552cf.
This was well-intentioned, but now the boxed Path values are actually
the *largest* Value enum variants, at 16 bytes (because they're
fat-pointers, with a len) instead of 8 bytes like all the other values.
Having the double reference is a reasonable price to pay (it seems; more
benchmarks may end up disagreeing) for a smaller Value repr.
Change-Id: I0d3e84f646c8f5ffd0b7259c4e456637eea360f7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10797
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Storing a full BString here incurs the extra overhead of the capacity
for the inner byte-vector, which we basically never use as Nix strings
are immutable (and we don't do any mutation / sharing analysis).
Switching to a Box<BStr> cuts us from 72 bytes to 64 bytes per
string (and there are a lot of strings!)
Change-Id: I11f34c14a08fa02759f260b1c78b2a2b981714e4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10794
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
These strings are allocated once and never changed, so they don't need
the additional overhead of a capacity given by String - instead, we can
use Box<str> and save on 16 bytes for each of these, *and* for each
Value since this is currently the largest Value variant.
Change-Id: I3e5cb070fe6c5bf82114c92d04f6bae775663a7e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10796
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
serde_json::Value is pretty large, and is contributing (albeit not
exclusively) to the large size of the Value repr. Putting it in a box
is *especially* cheap (since it's rarely used) and allows us
to (eventually) cut down on the size of Value.
Change-Id: I005a802d8527b639beb4e938e3320b11ffa1ef23
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10795
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Adds a bunch (notably certain overlapping) tests for catchable situations.
This should cover many scenarios, argument is catchable, element in argument is catchable, function returns
catchable in the middle of the processing, etc.
Co-authored-by: Aspen Smith <root@gws.fyi>
Change-Id: Icd722cf8dbc91a24f45cd540a328711e5826f76c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10621
Reviewed-by: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
These provide seekable access into a Blob for which we have more
granular chunking information.
There's no support for verified streaming in here yet, this simply
produces a stream of readers for each chunk, skipping irrelevant chunks
and data from the first chunk at the beginning.
A seek simply does produce a new reader using the same process.
Change-Id: I37f76b752adce027586770475435f3990a6dee0b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10731
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Correctly propagate the case where the *key* of an attrset is a
Value::Catchable (eg { "${builtins.throw "c"}" = "b"; }) in
`NixAttrs::construct`, by converting the return type to
`Result<Result<Self, CatchableErrorKind>, ErrorKind>` (ugh!!) and
correctly handling that everywhere (including an `expect` in the
Deserialize impl for NixAttrs, since afaict this is impossible to hit
when deserializing from stuff like JSON).
Change-Id: Ic4bc611fbfdab27c0bd8a40759689a87c4004a17
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10786
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
In order to correctly propagate errors in the comparator passed to
builtins.sort, we need to do all the sorting in a context where we can
short-circuit return `Value`s (because catchables are Values on the `Ok`
side of the Result , not `Err`s). Unfortunately this means we have
to *inline* the List `sort_by` implementation into the builtin_sort
function - fortunately this is the only place that was called so this is
relatively low cost. This does that, and adds the requisite `try_value!`
invocation to allow us to propagate comparator errors here.
As before, this doesn't include tests, primarily since those are coming
in the next commit.
Change-Id: I8453c3aa2cd82299eae89828e2a2bb118da4cd48
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10754
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Nix doesn't propagate errors for the function argument to some builtins,
like genList and map:
❯ nix repl
Welcome to Nix version 2.3.17. Type :? for help.
nix-repl> (builtins.tryEval (builtins.genList (builtins.throw "a") 10)).success
true
nix-repl> (builtins.tryEval (builtins.map (builtins.throw "a") [ "" ])).success
true
Note that this is untested as of this particular commit, only because a
big test suite covering all sorts of catchable error propagation issues
is coming next
Change-Id: I48c8eb390a541204b1a6d438c753fa1ca9b3877e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10753
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Propagate catchables that we get from forcing thunks in builtins in a
few more places using the new try_value! macro
Change-Id: I95fd41a231f877ff153f4adbabd944372d4cc7eb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10738
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Rather than explicitly checking for Value::Catchable in all builtins,
make the #[builtin] proc macro insert this for all strict arguments by
default, with support for a #[catch] attribute on the argument to
disable this behavior. That attribute hasn't actually been *used*
anywhere here, primarily because the tests pass without it, even for
those builtins which weren't previously checking for Value::Catchable -
if some time passes without this being used I might get rid of support
for it entirely.
There's also a `try_value` macro in builtins directly for the places
where builtins were eg forcing something, then explicitly propagating a
catchable value.
Change-Id: Ie22037b9d3e305e3bdb682d105fe467bd90d53e9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10732
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
docs/verified-streaming.md explained how CDC and verified streaming can
work together, but didn't really highlight enough how chunking in
general also helps with seeking.
In addition, a lot of the thoughts w.r.t. the BlobStore protocol, both
gRPC and Rust traits, as well as why there's no support for seeking
directly in gRPC, as well as how clients should behave w.r.t. chunked
fetching was missing, or mixed together with the verified streaming
bits.
While there is no verified streaming version yet, a chunked one is
coming soon, and documenting this a bit better is gonna make it easier
to understand, as well as provide some lookout on where this is heading.
Change-Id: Ib11b8ccf2ef82f9f3a43b36103df0ad64a9b68ce
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10733
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
The Stat() method was just always signalling no granular chunks are
available. However, as we now have a .chunks() method, we can expose it
over gRPC.
Change-Id: I74f0890ae083f301bb0cec62f1ea4a95463ac590
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10736
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
All chunks must have valid blake3 digests. It is allowed to send an
empty list, if no more granular chunking is available.
Change-Id: I7ecb53579cdf40fd938bb68a85685751b4d3626f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10726
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Autosubmit: flokli <flokli@flokli.de>
This can be written without the additional function.
Change-Id: Ib11c5d5254d3e44c8fa9661414835b0622eb1ac4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10735
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
NixString is *quite* large - like 80 bytes - because of the extra
capacity value for BString and because of the context. We want to keep
Value small since we're passing it around a lot, so let's box the
NixString inside Value::String to save on some memory, and make cloning
ostensibly a little cheaper
Change-Id: I343c8b4e7f61dc3dcbbaba4382efb3b3e5bbabb2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10729
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
"given chunksize" is misleading here. It's up to the backend to decide
if it does chunking at all, and how it chunks.
Change-Id: I4f130ca9ac34db79f18ef1d6475295806ac7f9a4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10728
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
BlobService already implies Send and Sync, we don't need to explicitly
list it here.
Change-Id: I58a4c5912be61a60acd961565979aa01d94ee0f7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10727
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
In the compiler, skip emitting an OpForce if the last op was an
OpConstant for a non-thunk constant. This gives a small (~1% on my
machine) perf boost, eg when evaluating hello.outPath:
❯ hyperfine \
"./before --no-warnings -E '(import <nixpkgs> {}).hello.outPath'" \
"./after --no-warnings -E '(import <nixpkgs> {}).hello.outPath'"
Benchmark 1: ./before --no-warnings -E '(import <nixpkgs> {}).hello.outPath'
Time (mean ± σ): 1.151 s ± 0.022 s [User: 1.003 s, System: 0.151 s]
Range (min … max): 1.123 s … 1.184 s 10 runs
Benchmark 2: ./after --no-warnings -E '(import <nixpkgs> {}).hello.outPath'
Time (mean ± σ): 1.140 s ± 0.022 s [User: 0.989 s, System: 0.152 s]
Range (min … max): 1.115 s … 1.175 s 10 runs
Summary
./after --no-warnings -E '(import <nixpkgs> {}).hello.outPath' ran
1.01 ± 0.03 times faster than ./before --no-warnings -E '(import <nixpkgs> {}).hello.outPath'
Change-Id: I2105fd431d4bad699087907e16c789418e9a4062
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10714
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
PathBuf internally contains a heap pointer (an OsString), so we were in
effect double-boxing here. Removing the extra layer by making
Tvix::Value represented by a Box<Path> rather than a Box<PathBuf> saves
us an indirection, while still avoiding the extra memory overhead of the
capacity which was the reason we were boxing PathBuf in the first place.
Change-Id: I8c185b9d4646161d1921917f83e87421496a3e24
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10725
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
C++ nix uses C-style zero-terminated char pointers to represent strings
internally - however, up to this point, tvix has used Rust `String` and
`str` for string values. Since those are required to be valid utf-8, we
haven't been able to properly represent all the string values that Nix
supports.
To fix that, this change converts the internal representation of the
NixString struct from `Box<str>` to `BString`, from the `bstr` crate -
this is a wrapper around a `Vec<u8>` with extra functions for treating
that byte vector as a "morally string-like" value, which is basically
exactly what we need.
Since this changes a pretty fundamental assumption about a pretty core
type, there are a *lot* of changes in a lot of places to make this work,
but I've tried to keep the general philosophy and intent of most of the
code in most places intact. Most notably, there's nothing that's been
done to make the derivation stuff in //tvix/glue work with non-utf8
strings everywhere, instead opting to just convert to String/str when
passing things into that - there *might* be something to be done there,
but I don't know what the rules should be and I don't want to figure
them out in this change.
To deal with OS-native paths in a way that also works in WASM for
tvixbolt, this also adds a dependency on the "os_str_bytes" crate.
Fixes: b/189
Fixes: b/337
Change-Id: I5e6eb29c62f47dd91af954f5e12bfc3d186f5526
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10200
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Add a new --trace-runtime-timing flag (probably a better bikeshed for
this) that enables capturing the time, relative to the last event, of
each event recorded with the tracing observer.
This probably isn't *super* useful yet, but I'd like to start here in
adding new profiling tools to the VM, specifically based on the runtime
observer
Change-Id: Id7f12077291c39bf3eef42ab6744bfba53687a65
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10713
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Use clap derive to make the input and output files configurable, as well
as the chunk size parameters.
Change-Id: I02b29126f3bd2c13ba2c6e7e0aa4ff048ff803ed
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10691
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: edef <edef@edef.eu>
This is a tool for ingesting subsets of cache.nixos.org into its own flattened castore format.
Currently, produced chunks are not preserved, and this purely serves as a way of measuring
compression/deduplication ratios for various chunking and compression parameters.
Change-Id: I3983af02a66f7837d76874ee0fc8b2fab62ac17e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10486
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
In case a .exp.xml file is provided alongside the test, compare its
output with the desired state.
Also, add some function .exp.xml that were presumably moved out of the
way back to src/tests/nix_tests, as they now produce the correct XML
output.
Change-Id: Ibd8123f3e6ed7bae3a44407d2284a2b2c8ce9a28
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10687
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
These need to be preserved at least for builtins.toXML.
Also, we incorrectly only wrote an <attrspat> in case ellipsis was true,
but that's not the case.
Change-Id: I6bff9c47c2922f878d5c43e48280cda9c9ddb692
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10686
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: aspen <root@gws.fyi>
At least toXML wants to get these out in a sorted fashion.
Change-Id: I6373d7488fff7c40dc2ddeeecd03ba537c92c4af
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10685
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
It's debateable on whether the serialization code should be exposed a
bit more prominently or not.
Change-Id: Iff7a28f884b1490b12b145dfdadbedacb84fd387
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10684
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
This relies on TEST_VAR=foo being set to "foo".
Nix does this in tests/functional/lang.sh, we do it in the test suite.
Change-Id: I7ffa9ed27124530b7758aeadf07c79477656f34f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10683
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Right now all blob uploads are performed synchronously, this means if a
NAR contains many small files, the import time is dominated by round
trip time to the blob service. For small files, we can buffer them in
memory and upload them asynchronously to the blob service. Before
returning we make sure to join all the uploads to make sure they
complete successfully before responding OK.
This reduces time to import a bash-interactive closure on my machine
from 1m19s to 7s.
Change-Id: Ica3695c159e6c8ad8769281ac20d037e3143e856
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10679
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Connor Brewster <cbrewster@hey.com>
Sets up OpenTelemetry integration for nar-bridge. Right now it will
export spans for HTTP server requests and all gRPC client requests.
Having the spans available will make performance work significantly
easier as it provides a high level overview of where time is being
spent.
In the future we can add application-specifc metrics and
integrate logrus.
Change-Id: Ie3860675d7ffc626a95673ba062c3c798d8bb2a7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10678
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: Connor Brewster <cbrewster@hey.com>
There's no `expected_failures` feature, we run them unconditionally.
Change-Id: Ibe1c93497e040d0d5b6cbfcaa043027814c191f1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10681
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This kind of collects points to consider which should hopefully help in
figuring out what such a lang test suite could or should look like
exactly—which is something I currently struggle somewhat.
Change-Id: If4f47546fe4b8046fb79718743fa9a72f9801876
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10657
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: sterni <sternenseemann@systemli.org>
In the past, we had a `todo!` on unsupported node types, this returns a proper error
that can be caught by the caller.
Change-Id: Icba4c1dab33c0d670a97f162c9b358d1ed5855cb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10675
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
The BoxStream type alias is a more concise and easier to read than
the full `Pin<Box<dyn Stream<Item = ...> + Send + ...>>` type.
Change-Id: I5b7bccfd066ded5557e01f7895f4cf5c4a33bd44
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10677
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: Connor Brewster <cbrewster@hey.com>
Our docs say it's still todo, but that's wrong, it landed in cl/7835
and cl/7962.
Change-Id: Ice374ae47ab653146be18c02dce27145e7488397
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10674
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: sterni <sternenseemann@systemli.org>
Sometimes, Nix lets someone customize the `name` in the store for a
path, this is the case for `builtins.path` which takes a `name`
argument, we leave it to the caller to choose the name, which can be the
basename by default of the path.
Change-Id: Icdbf71d1d8f2dca5716b99d20aac885aab905b80
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10653
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: flokli <flokli@flokli.de>
Add multiple additional helpers such as:
- `path_to_name`: derive the basename of a given path
- `derive_nar_ca_path_info`: derive the `PathInfo` for a content
addressed NAR
which isolates further the tree walking feature and the ingestion feature.
Additionally, we don't `expect` anymore and propagate properly ingestion errors up.
Change-Id: I60edb5b633911c58ade7e19f5002e6f75f90e262
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10574
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
In one function that does the heavy lifting: `ingest_entries`, and three additional helpers:
- `walk_path_for_ingestion` which perform the tree walking in a very naive way and can be replaced by the user
- `leveled_entries_to_stream` which transforms a list of a list of
entries ordered by their depth in the tree to a stream of entries in
the bottom to top order (Merkle-compatible order I will say in the
future).
- `ingest_path` which calls the previous functions.
Change-Id: I724b972d3c5bffc033f03363255eae448f017cef
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10573
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
To make use of the filtering feature, we need to revert the internal walker to a real DFS.
We will therefore just invert the whole tree by storing all of its
contents in a level-keyed vector.
This is horribly expensive in memory, this is a compromise between CPU
and memory, here is the fundamental reason for why:
When you encounter a directory, it's either a leaf or not, i.e. it
contains subdirectories or not.
To know this fact, you can:
- wait until you notice subdirectories under it, i.e. you need to store
any intermediate nodes you see in the meantime -> memory penalty.
- getdents or readdir on it to determine *NOW* its subdirectories -> CPU
penalty and I/O penalty.
This is an implementation of the first proposal, we pay memory.
In practice, we are paying O(#nb of nodes) in memory.
There's a smarter albeit much more complicated algorithm that pays only
O(\sum_i #siblings(p_i)) nodes where (p_1, ..., p_n) is the path to a leaf.
which means for:
A
/ \
B C
/ / \
D E F
We would never store D, E, F but only E, F at a given time.
But we would still store B, C no matter what.
Change-Id: I456ed1c3f0db493e018ba1182665d84bebe29c11
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10567
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: flokli <flokli@flokli.de>
In cl/10468, we accepted SRI hashes of invalid padding while checking
their trailing bits.
In this commit, we accept SRI hashes of invalid padding and invalid length, as Nix does.
Real world example: `pkgs.javaPackages.openjfx11.deps`
<849e4dc5ff/pkgs/development/compilers/openjdk/openjfx/11.nix (L71)>
in nixpkgs.
Change-Id: I834437e7b94dab9fbb030163f7a2741f52bbf03a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10668
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Impure builtins need to be explicitly added. This was missed from
cl/10640.
Change-Id: I1f8a84aa02f1cf8a26247960d6eb15895e09c610
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10667
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
- Adjust to ecl 23.9.9 release
- Regenerate go protos after protoc-gen-go update
- Drop dhall fork which hasn't kept up with 1.42.*
- Address new clippy warnings:
- Variant naming of Error::ValidationError
- Simplify .try_into().unwrap()
- Drop unnecessary identity function
- Test module must be last in file
- Drop unused `pub use`
- Update agenix to 0.15.0. Current master has a installCheckPhase that
doesn't work with C++ Nix 2.3.*:
a23aa271be (commitcomment-137185861)
Change-Id: Ic29eef20d6fd1362ce1031364a5ca6b4edf195bd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10615
Reviewed-by: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Having something running the depot crate2nix and formatting it with
depotfmt is useful outside of tvix too.
Change-Id: Iecc8f207da38cc6995747c5ea48d3911433fd416
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10658
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
So that we can just `map_err` easily in functions returning `std::io::Error` but calling functions
returning `castore::import::Error`.
Change-Id: Id181b95e8431c69e95f3a8cd569ca10306656e1d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10572
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
TvixStoreIO triggers builds whenever IO into a not-yet-built store path
is requested, if it knows how to build that path.
Change-Id: If30e9db6be2f2a30cbc9d0576f357f3ecfa0d35a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10645
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
We propagate a `TvixStoreIO` as the `state` of our derivation-specific
builtins in the glue crate.
The evaluators `io_handle` itself is using a Rc<dyn EvalIO>.
An earlier version of TvixStoreIO was also introducing generics over the
different internal services themselves, but we opted for instead
hardcoding this to Arc<dyn …> for the sake of less macro voodoo.
Change-Id: I535c476f06b840858fa3070c4a237ece47f7a15b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10636
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Have a Evaluation::new() function that's used to set up the Evaluation
struct initially - which is also used by both new_pure and new_impure
internally.
It's generic over the exact type of IO, making it easier to instantiate
Evaluation with non-tvix-eval EvalIO implementations, that might not be
in a Box.
Change-Id: Ibf728da24aca59639c5b6df58d00ae98c99a63f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10640
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
I lost a lot of hope and had to read the source code of `quote!`, `cargo expand` was invaluable
in this adventure. We should keep it IMHO.
Change-Id: Icfb4c80d413602f2bdc6deab0d595183825d88ad
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10635
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Don't restrict to a Box<dyn EvalIO>.
There's still one or two places where we do restrict, this will be
solved by b/262.
Change-Id: Ic8d927d6ea81fa12d90b1e4352f35ffaafbd1adf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10639
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
`throw (throw "a")` should work and propagate the internal throw.
Before this commit, it didn't work.
Change-Id: Id5d46f74e484dba99e912ad9fa211f3bf1617bac
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10600
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
`elem` did not catch the list being a catchable.
This surfaced during Nixpkgs evaluation.
Change-Id: Icf19b94e914e35a435c4412d769ee63ba59ab7b0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10599
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This is an additional test suite on the top of the Nix ones
for context strings matters.
It already smoked out multiple mistakes and potential bugs and non-deterministic result from the evaluator.
It uses a similar technology as the one in the tvix-eval albeit we instantiate a fully fledged evaluator
with in-memory store.
We copy the files instead of symlinking them because crates are built in
isolation, so symlinks cannot work.
Change-Id: I63ae225ce4f83c6e2c8ccd60d779c2f8eb9d08fb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10619
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Previously, we were assembling very naively an attribute set composed of context we saw.
But it was forgetting that `"${drv}${drv.drvPath}"` would contain 2 contexts with the same key, but
with different values, one with `outputs = [ "out" ];` and `allOutputs = true;`.
Following this reasoning and comparing with what Nix does, we ought to merge underlying values systematically.
Hence, I bring `itertools` to perform a group by on the key and merge everything on the fly, it's not
beautiful but it's the best I could find, notice that I don't use
`group_by` but I talk about group by, that is, because `group_by` is a
`group_by_consecutive`, see
https://github.com/rust-itertools/itertools/issues/374.
Initially, I tried to do it without a `into_grouping_map_by`, it was akin to assemble the final `NixAttrs` directly,
it was less readable and harder to pull out because we don't have a lot of in-place mutable functions on
our data structures.
Change-Id: I9933c9bd88ffe04de50dda14f21879b60d8b8cd4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10620
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Yes, `hasContext e` should work where `e` is a contextful strings, otherwise, it is really useless.
Change-Id: I5eb071fc257217d6e8a63fe519132ebd98186696
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10617
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
`test-generator` has not been updated in the past 2 years.
`rstest` has not been updated in the past 5 months.
This is an improvement in the maintenance state… I guess?
We get also new features, it changes the name of the tests with numbers too.
Change-Id: I5376104c7704f525dba7524da78daa09867cc669
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10623
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
We want to handle bottoms in a consistent fashion. Previously this was
handled by repetitive is_catchable checks, which were not consistently
present.
Change-Id: I9614c479cc6297d1f64efba22b620a26e2a96802
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10485
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Rather than passing strings around, use a StorePathRef.
This makes things a bit more typesafe, and more aligned with what we
want to do in b/264.
Change-Id: Ib7080addf27e7f1a9c8da1d8aaa66744468e3b5a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10633
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
We need to vendor in the package expression, as it's not possible to
override cargoHash.
Change-Id: Ib123647bb9b96d41f4630daa431d020f1cb8d4fa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10624
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
This starts a BuildService as a separate process, currently defaulting
to the DummyBuildService.
Change-Id: Ic206f00831641d3ffebaa44883b7dc053700b9ca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10631
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
This allows constructing a BuildService from a URI, similar to how it's
done in tvix-[ca]store.
Change-Id: Ib962b329535c6c7e378ab7ac7f4dd254366497b3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10630
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Also provide a dummy implementation that just fails on any build that's
requested.
Change-Id: I0df743a730c5331ec9ce6e97a966abe18ce067f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10627
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Determining the inputs might trigger additional builds/substitutions,
so answering these lookups via a lambda in a lazy fashion gets
complicated.
You end up assembling the list of input nodes upfront, and the lambda
will just be a dumb lookup into that preassembled list.
Rather than doing that, simply have derivation_to_build_request leave
the work of determining the inputs to the caller.
Change-Id: I75880132916c76b930807c989090da298b6891bd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10626
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This are leftovers from the "reference scanning" approach (which we
didn't end up using).
We still want a concept of known paths, so we can trace IO into
storepaths back to the build recipe that'll produce it, so let's keep
the rest of this struct around.
Change-Id: I73d38e21e5b97950b8fc2a42176cae5f80d371c8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10632
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
A bunch of operations in Tvix are not aware of catchable values
and does not propagate them.
In the meantime, as we wait for a better solution, we just offer this
commit for moving the needle.
Change-Id: Ic3f0e1550126b0847b597dfc1402c35e0eeef469
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10473
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
`args` was not propagating context, here's a regression test for it.
Change-Id: I8b6a3148508d40df0077128f0bafe68c098a03bd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10610
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This adds support to handle the __structuredAttrs argument, which can be
passed to builtins.derivationStrict.
If __structuredAttrs is passed, and set to true, most of the arguments
passed to builtins.derivationStrict are not simply coerced to a string
and passed down to "environments", but instead kept in a more structured
fashion.
Inside ATerm, which is what's relevant as far as path calculation is
concerned, a virtual `__json` environment variable is present,
containing these structured values.
Inside Builds, these structured values are not made available as an
environment variable, but a JSON file (and source-able bash script).
This will need to be respected once we start emitting BuildRequests,
and for that we can probably just parse the `__json` key in
Derivation.environment again - or keep this additionally in
non-serialized form around during Evaluation.
No matter what, this is left for a followup CL.
The existing handle_derivation_parameters and populate_outputs helper
function were removed, as __structuredAttrs causes quite a change
in behaviour, and so handling both in the same place makes it more
readable.
There's some open questions w.r.t. string contexts for structured attrs
itself. A TODO is left for this, but at least path calculation for
individual structured attrs derivations are correct now.
Part of b/366.
Change-Id: Ic293822266ced6f8c4826d8ef0d2e098a4adccaa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10604
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Allow other crates (like tvix-glue) to look at a Value in JSON, which is
used by the structured attrs feature.
Change-Id: Iba02ace6e11a74c3f9b19dcbef4b008b76dec046
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10602
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
My OCD could not be stopped.
Change-Id: I2bf504fe0865a5084ad02aee18e6180a8a3e19d7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10609
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
The Derivation input_derivations field contains a list of input
derivations and (a subset of their) output names.
This means, multiple nodes can be returned, so return a Vec.
Also, update the name to better reflect the nodes are the nodes of the
selected outputs, not a node representing the .drv file itself.
Additionally, use a proto::node::Node (the naked enum), rather than
proto::Node, which wraps this in an optional struct field until
realizing the BuildRequest.
Change-Id: Iec5620b5d7ac0462f2c76acac4abcaeea2de0aad
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10608
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Provide a store_path_to_node_sync function which uses the runtime handle
to block on the async function internally, but make store_path_to_node
itself async, so it can call async functions internally.
We'll use that later when triggering builds and waiting on their
results.
Change-Id: Idae9da7aa5b0878e0d3a2eba34ea2623e1ba84b2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10607
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
We don't need Arcs in most of the cases, we're fine with some container.
Change-Id: Ic4f8acb5b9d93e2b0923bb607463fb91e9d0e4fe
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10606
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
To render NARs, we're fine with a simple AsRef to a BlobService and
DirectoryService. We just need to have the function pass back the
references, so we can reuse it after the recursion.
Change-Id: I8a1b899134ddda26cf14aa829a08383986101850
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10605
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
The error message is misleading. The errors we return can happen both
during serialization or deserialization, though the messages suggested
the latter only.
Change-Id: I2dafe17ec78ee75cab5937a3a81540fda3175eac
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10603
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This adds support to retrieve a list of chunks for a given blob to the
BlobService interface.
While theoretically all chunk-awareness could be kept private inside
each BlobService reader, we'd not be able to resolve individual chunks
from different Blobservices - and due to this, not able to substitute
chunks we already have in a more local store.
This function allows asking a BlobService for the list of chunks,
leaving any actual fetching up to the caller (be it through individual
calls to open_read), or asking another store for it.
Change-Id: I1d33c591195ed494be3aec71a8c804743cbe0dca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10586
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Make it clear this is only used inside the scope.
Change-Id: Ie94f88d7f0fb58cd4bf9c2f1176000b272e6f2e6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10585
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
We need to be a bit careful and pass the BlobService around (similar to
how we already do with the directory_putter), but that allows getting
rid of a bunch of annoying trait bounds.
We also stop spawning additional tasks where we can just use block_on.
Change-Id: If36de0ee947d2c779d20a384308241d2262d4764
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10580
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
The docstrings were not updated once we made the BlobService trait async.
There's no more need to turn things into a sync reader.
Also, rearrange the stream manipulation a bit, and remove the need to
create a new VecDeque for each element in the stream. bytes::Bytes
implements the Buf trait.
Fixes b/289.
Change-Id: Id2bbedca5876b462e630c144b74cc289c3916c4d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10582
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
This test serves as a minimal reproducer for output path calculation.
Derivations with the same name and output hash, but different build
recipe should end up with the same outPath.
However derivations with different name should end up with a different
outPath.
Change-Id: I555be59dd87ea675a0816188ed973f96c311e4e1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10416
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
It was a `//` not a `///`.
Change-Id: Iee3e8c116d73b5dd8a41c027153714415a66695f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10566
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
We don't need to spawn in all these places, we can just block_on
directly, this is all IO bound.
This also means, we don't need to clone any of the service handles
(except preserving clone-ability of the BlobService).
Change-Id: I7d90f4d6a263a98491caa071ada538a5197a5472
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10540
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This was wrongly named, it returns a specific node at a subpath.
Also, this code can be simplified a lot - we don't need to spawn
additional tasks, and can get rid of some clones too.
This is also where we need a certain build - so add some TODO to block /
fetch here.
Change-Id: Id26d7bd80f7a2095121e642b3f7716de78d6b6a6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10539
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Extend our validation function to also check for the None case.
Change-Id: Ib75f880646d7fb3d66588f1988e61ec18be816a2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10534
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Make this an `AsRef<dyn DirectoryService>`.
This helps dropping some Clone requirements.
Unfortunately, we can't thread this through to TvixStoreIO just yet.
Change-Id: I3f07eb28d6c793d3313fe21506ada84d5a8aa3ac
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10533
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
While doing the obvious thing, ensuring clippy doesn't complain on our
benchmarks and examples, this will also flag build failures on them,
and contrary to cl/10301 not pollute the /nix/store with stuff we can't
execute anyways.
Change-Id: I7ee1c2babbc67dac9794274d05bb9d6e1251ed01
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10544
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
cl/10475 updated the API of tvix_eval::Evaluation, but didn't update
this benchmark to use that new API.
Also, fixes the docstring to no longer specify that there is a "given
snippet".
Change-Id: Ibb8285731849dbeec814e2585bbaa36f22368afe
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10542
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
With context strings, we are now able to reproduce up to firefox.
Until a more problematic example can be found, we should at least
lean on this particular example.
Change-Id: Ibeaf799d26e1f160b6c93f8ccd978702a2a7e3e1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10460
Reviewed-by: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Raw printing will transform the result into a string
and print it.
In case of a contextful string, this will fail by default, as expected.
Change-Id: I5e564329e7b001adc57a77a9153b4425cb332bb7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10457
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Nix does something like:
```cpp
NixStringContext context;
const auto str = state.forceString(*args[1], context, pos, "while evaluating the second argument passed to builtins.split");
```
And then do nothing with that context, therefore, we follow them and
make `split` aware of the context but still do nothing with it.
Change-Id: I4fee1936600ce86d99d00893ca3f64013213935b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10428
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
We calculate the input context by performing the union of context
over all input of the derivation.
Then, we just pass the rest of it to the remaining machinery.
Finally, we re-emit an `outPath` and a `drvPath` containing the expected
contexts.
Change-Id: I74905fb258b5bee8b08d1208c9eb87f51b92a890
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10436
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This removes the reference tracking and uses instead the context for
references and returns some.
Change-Id: Ic359ca6b903b63f1a9c679c566004c617b792442
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10435
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
In the past reference tracking system, `tvix-io` glue was appending
plain paths in the known path state.
Now, we make up for this by just making contextful coercion of file
imports.
Change-Id: Ieb9b04dd83302c77909252d5f7733857ac3cf8fd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10443
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
String with contexts are always coerced to a string with the same
context.
Change-Id: I224814febd9cad196bb28876793e76bed564dc72
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10440
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: sterni <sternenseemann@systemli.org>
Otherwise, you just fail because they are not... contextless strings!
Change-Id: I0b8f63a18cd89c3841b613d41c12ec4ee336f953
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10442
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
`match` silently ignore the input context and do not propagate
it and successful matches.
The why is unclear but nixpkgs does rely implicitly on this behavior
because dynamic attribute selection cannot be done with contextful
strings.
Change-Id: I5167fa9b2c2db8ecab0c2fb3e9895c9cfce6eeb2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10441
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
And it also preserve the original context if it exists.
Change-Id: I904f7c13b7f003a267aace6301723780fccaafb7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10434
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
We just perform union of contexts of every pieces.
Change-Id: Ief925c1818cd8bbec0503e9c625b0630feebfdda
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10432
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
`substring` has a very funny behavior when it comes to empty strings,
it propagates the context too, this is used in nixpkgs to attach context
to strings without using any builtin: `lib.addContextFrom`.
Change-Id: Id655356799b3485f7519b3d1914c630f9d8416c3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10448
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
We make a case for adding a `reject_context` `CoercionKind` here.
It does happen during concatenation actually for path concats.
Change-Id: I0c196aad917550b9bcd0896cd2127a94f8181ffb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10444
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
I am still undecided whether we need a CoercionKind to control
the coerced context, here's a simple attempt.
Change-Id: Ibe59d09ef26c519a6acfdfe392014446646dd6d8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10426
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: raitobezarius <tvl@lahfa.xyz>
This extends derivation_to_build_request to handle passAsFile the same
way Nix does, and adds a unit test for it.
I opted to making this function fallible (if passAsFile contains a
non-existent env var), rather than pushing all of this into the
Derivation validate function.
Change-Id: I75b635f1f6c0c78d72b9a8fc7824f77e97b69951
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10522
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
While this can be influenced with `OTEL_SERVICE_NAME` or
`OTEL_RESOURCE_ATTRIBUTES`, "unknown_service" is an annoying name.
Let's pick tvix.store as a default.
Change-Id: I9b5c45703d9546775bb9266b29ab54daf7c54a9f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10524
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Prevent code in this function from accessing the Vec, realize it only
when returning the BuildRequest struct.
Change-Id: I385c1adca552c9cb261cd70c5ca3ec45f0d7a9c3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10521
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This already succeeds, as the output path calculation path does not need
to be aware of any builder-specific custom handling for passAsFile.
Change-Id: I9cc9e4e4351cdeaa3ec33ba58ee1569e7a368150
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10520
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Nix builds also have some other inputs, outside of inputs_dir, used in
derivations setting `passAsFile` and `__structuredAttrs`.
This extends our `BuildRequest` structure to accomodate these usecases.
Change-Id: I8de6516eb467fa48a4961d88933ebd548f131049
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10519
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
And include a test to ensure we show the warning.
Change-Id: Ib6a436dbba2592b398b54e44f15a48d1aa345099
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10470
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
In the past, we had `emit_warning` be no-op and we used `push_warnings` exclusively
but as we have consumers of this function, we need it to work somewhat.
Change-Id: I78a5ece199a473dec9ef5ea1fae60b36e35137b8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10477
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Nix is quite tolerant when it comes to parsing SRI hashes and their
padding (and only for SRI hashes, it funnily is strict about that in the
non-SRI-hash case).
Nix essentially accepts any number of padding characters, no matter if
it's too much or too little. So we do the only sane thing - simply strip
all padding characters, and parse it with BASE64_NOPAD and the length
the algo uses.
Change-Id: I6a721aa289b06cc36741589792b9dd4c4f930b86
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10468
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This is behind the otlp feature flag (currently enabled by default).
By default, this will try to push traces to a OTLP collector running at
https://localhost:4317.
You can make one available by running:
```
docker run -d --name jaeger \
-e COLLECTOR_ZIPKIN_HOST_PORT=:9411 \
-e COLLECTOR_OTLP_ENABLED=true \
-p 6831:6831/udp \
-p 6832:6832/udp \
-p 5778:5778 \
-p 16686:16686 \
-p 4317:4317 \
-p 4318:4318 \
-p 14250:14250 \
-p 14268:14268 \
-p 14269:14269 \
-p 9411:9411 --rm \
jaegertracing/all-in-one:1.42
```
Started like that, jaeger brings a web interface at
http://localhost:16686/search
As documented in
https://docs.rs/opentelemetry-otlp/latest/opentelemetry_otlp/, you can
point this to another location by setting `OTEL_EXPORTER_OTLP_ENDPOINT`.
Change-Id: Id1dca367d70027b2ea98bb70bcf99a68363ec2be
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8194
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: aaqaishtyaq <aaqaishtyaq@gmail.com>
Only convert to and reuse an Arc<…> where needed.
Change-Id: I2c1bc69cca5a4a3ebd3bdb33d6e28e1f5fb86cb9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10514
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
We can also drop the Clone requirement. Because the trait is async since
some time, there's no need to clone before moving into an async closure,
allowing us to simplify the code a bit.
Change-Id: I9b0a0e10077d8c548d218207b908bfd92c5b8de0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10515
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
This is doing the exact same thing.
Change-Id: Iadc5e13dd192efc91cc3d36b2bdf4b8b99a312b2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10512
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
While at it, make it a bit more generic.
Change-Id: Ic4caefda93aca3ffb656a09f8b4d648b41415532
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10511
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
We don't actually care if it's an Arc<dyn BlobService>, or something
else, as long as we can Deref to a BlobService and clone.
Change-Id: I0852aaf723f51c5e6b820be8db1199d17309ab08
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10510
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
We already have that data in output_path.
Change-Id: Iadf615551f0b44d539098446ff5926994deb606d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10508
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Instead, return an error, and move the entire check before starting to
ingest the data underneath.
Change-Id: Idcfba115cb7d599f5fc72a156aaad9d4d4714fcf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10507
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
We can use cli.json.then_some(…) to create a Some(…), allowing us to
omit the else { None } lines.
Change-Id: I6c8142a08d8cb88d6c8302e5ca7570698fcf2aa3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10505
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
This takes three URLs, and constructs Arc'ed
{Blob,Directory,PathInfo}Service, allowing to remove some of the
boilerplate.
Change-Id: I40e7c2b551442ef2acdc543dfc87ab97e7c742bb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10484
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
While we currently mostly use it in an Arc, as we need to clone it
inside PathInfoService, there might be other usecases not requiring it
to be Clone.
Change-Id: Ia05bb370340792a048e2036be30e285ef1e63870
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10483
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
While we currently mostly use it in an Arc, as we need to clone it
inside PathInfoService, there might be other usecases not requiring it
to be Clone.
Change-Id: I7bd337cd2e4c2d4154b385461eefa62c9b78345d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10482
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
At some point, tvix-cli needs to talk to the outside world to persist
things into a real store.
Introduce the same CLI options to configure
{Blob,Directory,PathInfo}Service URLs.
We need to be a bit careful with how we set up stores, and make
this separate from setting up TvixStoreIO, as it's holding a
Rc<RefCell<KnonPath>> which not Send.
At some point, we might make this a Arc<RwLock<_>> later anyways, and
then this can be simplified a bit, but for now, this is sufficient.
Change-Id: I87d84ca3a10ce947e194ff985073791469773f35
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10474
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Instead, it's passed in the evaluate/compile_only functions, which feels
more naturally. It lets us set up the Evaluation struct long before
we actually feed it with data to evaluate.
Now that Evaluation::new() would be accepting an empty list of
arguments, we can simply implement Default, making things a bit more
idiomatic.
Change-Id: I4369658634909a0c504fdffa18242a130daa0239
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10475
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
There's no need to require builds to also put their outputs in store_dir.
So rename store_dir to inputs_dir and make outputs consume paths, not
basenames.
Also document the possibility for builds to write to inputs_dir. Let's
see if we want to make this a constraint later.
Change-Id: Ib4df20afcdde2d771c269c422f04c7e95587cd0f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10447
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
By default, we don't want contextful strings and we almost always want contextless strings.
To this end, we make taking a contextful string a very explicit operation under `to_contextful_str`
and we implement manually the `to_str` cast which requires a `if !s.has_context()` guard that
the macro cannot cover.
Change-Id: I7aae8e57a7d73e547e62b1edb0b1cc7e8c0c69b6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10425
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This prepares the data structures to implement string contexts
in Nix.
Change-Id: Idd913c9c881daeb8d446907f4b940e462e730978
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10420
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Explain why we had to take the bullet on context strings now
and move the historical approach in a historical section.
Change-Id: Ie3bcc2213b391c6ba06547cc05c850891a41d06b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10446
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
I'm still trying to work out the exact stack invariants for tvix.
We really should add assertions for them; getting the stack messed
up is no fun. This commit adds one simple assertion. It also adds
a missing stack-push (my mistake) in one place, which was uncovered
by the assertion.
Change-Id: I9d8b4bd1702d954e325832c5935b0d7e3eb68422
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10369
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
We only do things with the reference, so we don't need to locally borrow
it.
Change-Id: I6073f7ec7aff717ae3069e28a00b1cb408a50ceb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10455
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
This is a bit more useful than evans when sending over the same message
over and over again.
Change-Id: I600b6b9f591c0c963c5d270005aa1cc84d2a0770
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10453
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: edef <edef@edef.eu>
Tested-by: BuildkiteCI
Make it more clear this is not tokio::io.
Change-Id: Ic2fa56f0baf1c200b6631098d556388a19629a45
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10452
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
- directory in which the castore input nodes are mounted
- working directory for the build command
- scratch paths
- network access y/n
- whether a (static) /bin/sh should be provided
Populate these fields appropriately, and extend the tests in tvix-glue
with a FOD example.
Change-Id: I4f9de1483d6696d74694a09784910c407acb0be0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10412
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
r/7176 introduced an incorrect assumption was the benefit of the
nonrecursive coercion algorithm, namely that a coercion operation always
returns a non empty string. This allows to detect whether we are
coercing a list or not by checking if the intermediate result is empty
or not. Unfortunately, coercing null and false yields an empty string,
so we need to explicitly track whether we are coercing a list.
Updated the test case to hopefully catch similar bugs in the future. I'm
not a hundred percent certain I have not introduced a new edge case with
this, so it may be interesting to add a prop test case for this to
nix_oracle down the line. At least lists are the only nested data
structures that can be serialized as nested data structures, so the
problem is kind of limited.
Change-Id: Ia41e904356f1c41a9d35e4e65ec02f2fe5a4100e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10418
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
There's no need to pass in an unused directory service into the
populate_blob_* method, and considering we have one or two invocation of
each of these, we don't really gain much from having all these functions
follow the same structure, at least for now.
Also, update some function names to better describe what they're doing.
Change-Id: I92f680745c157fb0a602b07342f8838bfad23ecd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10411
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
cl/10378 did already move store/fs to castore/fs, but we kept the tests
in tvix-store, as they were populating a PathInfoService to make nodes
appear in the mount root.
Update these tests to now just insert root nodes into a BTreeMap, and
ensure we can use that as a RootNodes too.
Change-Id: Iad7d1ee4f9423eb6e3a1da33f433842c9ae0de1f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10410
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
With the recent introduction of the RootNodes trait, there's nothing in
the fs module pulling in tvix-store dependencies, so it can live in
tvix-castore.
This allows other crates to make use of TvixStoreFS, without having to
pull in tvix-store.
For example, a tvix-build using a fuse mountpoint at /nix/store doesn't
need a PathInfoService to hold the root nodes that should be present,
but just a list.
tvix-store now has a pathinfoservice/fs module, which contains the
necessary glue logic to implement the RootNodes trait for a
PathInfoService.
To satisfy Rust orphan rules for trait implementations, we had to add a
small wrapper struct. It's mostly hidden away by the make_fs helper
function returning a TvixStoreFs.
It can't be entirely private, as its still leaking into the concrete
type of TvixStoreFS.
tvix-store still has `fuse` and `virtiofs` features, but they now simply
enable these features in the `tvix-castore` crate they depend on.
The tests for the fuse functionality stay in tvix-store for now, as
they populate the root nodes through a PathInfoService.
Once above mentioned "list of root nodes" implementation exists, we
might want to shuffle this around one more time.
Fixes b/341.
Change-Id: I989f664827a5a361b23b34368d242d10c157c756
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10378
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This makes PathInfoService::from_addr return a Box<dyn PathInfoService>,
rather than an Arc<dyn …>, and leaves it up to the consumers to rewrap
it into an Arc where needed.
This allows us to drop the Arc for the tvix-store daemon subcommand.
Change-Id: Ic83aa2ade6c51912281bd17c7eef7252e152b2d1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10409
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This is not gonna end up as a interlinked docstring.
Change-Id: I2b0ca106aa75bae0156c0b411da5931da60c725d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10406
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: edef <edef@edef.eu>
Tested-by: BuildkiteCI
