Exclusive range patterns were stabilized in Rust version 1.80 but Tvix
still uses Rust 1.79 and so would fail this one test when you ran
doctests from `mg shell //tvix:shell`.
It was not caught by CI because that does not currently run doctests.
The fix is just to use an inclusive pattern instead.
Fixes: b/417
Change-Id: Ifea7a3b84bb8f6f8c76e277979833713bdf51f46
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12375
Autosubmit: Brian Olsen <me@griff.name>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
tonic-reflection 0.12.x moved from the v1alpha to v1 of the reflection
protocol.
However, most clients, like Postman, Kreya and evans don't support that
one yet.
Bump tonic-reflection to 0.12.2, which re-introduces v1alpha support
alongside the v1 version of it, registering both services.
This fixes the example documented in tvix/store/README.md, it was
previously failing as evans couldn't find the v1alpha reflection
service.
See https://github.com/hyperium/tonic/pull/1888 for details.
Change-Id: I55438877317f82dc39face13afeb9594cda07a4e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12353
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
When only the wire feature was enabled two methods were unused and so
would faild `cargo check --no-default-features --features wire`.
This feature flags those two methods on async feature since that is the
only place they are used.
Change-Id: I6ec18a670e3c6e3ecee8d1417c99f1a5084e0ae7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12346
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Brian Olsen <me@griff.name>
Tested-by: BuildkiteCI
This adds a nix-compat-derive derive crate that implements a deriver
for NixDeserialize implementations. This is to reduce the amount of
code needed to implement deserialization for all the types used by
the Nix daemon protocol.
Change-Id: I484724b550e8a1d5e9adad9555d9dc1374ae95c2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12022
Autosubmit: Brian Olsen <me@griff.name>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Add a trait for deserializing a type from a daemon worker connection.
This adds the NixDeserialize trait which is kind of like the serde
Deserialize trait in that individual types are meant to implement it
and it can potentially be derived in the future.
The NixDeserialize trait takes something that implements NixRead as
input so that you can among other things mock the reader.
Change-Id: Ibb59e3562dfc822652f7d18039f00a1c0d422997
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11990
Autosubmit: Brian Olsen <me@griff.name>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This adds a function which can be used across the monorepo to create a
an extra CI step that checks whether the Cargo.nix file is up-to-date.
Change-Id: Idb8298b29ddc2ca5dff1facb1b9ed86a236ee66d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12227
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This prevents browsers from treating NARInfo and nix-cache-info paths as
a separate "Download", but just show it in plaintext.
Change-Id: If99abe20ef1d24e4fa86c055160861ca47aa81ce
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12267
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This adds the MIME types Nix uses when uploading files to a S3 bucket /
HTTP endpoint. It ignores them when retrieving.
Change-Id: I75fa96d5e53349f0470bbe8cbba93f961fbd5f05
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12266
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
Fix a discrepancy in the error message for DirectoryError::SizeOverflow.
The message indicates that the SizeOverflow error occurs when total size
exceeds u32::MAX, but that's not true. All size fields within the
castore's internal Directory ADT are u64, and the SizeOverflow error is
only returned after a call to the checked_add implementation on u64.
See tvix/castore/nodes/directory.rs +111
and tvix/castore/nodes/directory.rs +88
as of this commit.
Change-Id: I74d161ea8927362e1cb601ba163489aa96fb91b1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12259
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This moves the URL component parsing code we had in nar-bridge to
nix-compat.
We change the function signature to return an Option, not a
Result<_, StatusCode>.
This allows returning more appropriate error codes, as we can
ok_or(…) at the callsite, which we now do: on an upload to an
invalid path, we now return "unauthorized", while on a GET/HEAD, we
return "not found".
This also adds support to parse compression suffixes. While not
supported in nar-bridge, other users of nix-compat might very well want
to parse these paths.
Also fix the error message when parsing NAR urls, it mentioned 32, not
52, which is a copypasta error from the narinfo URL parsing code.
Change-Id: Id1be9a8044814b54ce68b125c52dfe933c9c4f74
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12260
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Similar to how cl/12253 already did this for `Signature`, we apply the
same logic to `StorePath`.
`StorePathRef<'a>'` is now a `StorePath<&'a str>`, and there's less
redundant code for the two different implementation.
`.as_ref()` returns a `StorePathRef<'_>`, `.to_owned()` gives a
`StorePath<String>` (for now).
I briefly thought about only publicly exporting `StorePath<String>`
as `StorePath`, but the diff is not too large and this will make it
easier to gradually introduce more flexibility in which store paths to
accept.
Also, remove some silliness in `StorePath::from_absolute_path_full`,
which now doesn't allocate anymore.
Change-Id: Ife8843857a1a0a3a99177ca997649fd45b8198e6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12258
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Don't call functions bstr or str when they return BString or String,
it's confusing.
Rename them to `string` and `bytes`. We might be able to generalize over
this being BString or Vec<u8> later.
Change-Id: I8198551ed3ba1cfc479bf7e3cbbc13a426faf4c0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12257
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This implements a simple DFS locator in listing structures.
It is interoperable with the Rust standard library paths, we also build our
own errors to restrict path values to reasonable secure defaults, e.g.
relative paths with no `..` component.
Tests are added for this new feature for a positive and a negative
check.
In addition, a path validation test was added. The Windows-style prefix
is gated on the Windows platform as UNIX does not parse `C:\\` as a
`Component::Prefix(_)` but as a `Component::Normal(_)`.
Change-Id: Iae2a80bebd8138e41af94aa7d09f2842c3c5a786
Signed-off-by: Ryan Lahfa <tvl@lahfa.xyz>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12255
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Requiring `name` to be a `&str` means it'll get annoying to pass around
`Signature`, but being able to pass them around in an owned fashion is
kinda a requirement for a stronger typed `PathInfo` struct, where we
want to have full ownership.
Rework the `Signature` struct to become generic over the type of the
`name` field. This means, it becomes possible to have owned versions
of it.
We don't want to impose `String` or `SmolStr` for example, but want to
leave it up to the nix-compat user to decide.
Provide a type alias for the existing `&str` variant (`SignatureRef`),
and use it where we previously used the non-generic `Signature` one.
Add some tests to ensure it's possible to *use* `Signature` with both
`String` and `SmolStr` (but only pull in `smol_str` as dev dependency
for the tests).
Also, add some more docstrings, these were a bit sparse.
Change-Id: I3f75691498c6bda9cd072d2d9dac83c4f6c57287
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12253
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
.ls files are useful to seek in a NAR without parsing it entirely.
The responsibility of validating the files is on the caller.
Change-Id: I5d1da28b5479c38f20ca5babe60e362a2217c9ea
Signed-off-by: Ryan Lahfa <tvl@lahfa.xyz>
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12196
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
We can avoid depending on things outside //tvix by just using a similar
util from nixpkgs.
Change-Id: I9ea3e1f0a8a059ea10caaec173569ba9f316aec6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12247
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Using pkgs.nixos directly allows us to create a smaller nixos closure
for the tests and also not depend on things in depot.ops which can be
beneficial for extending the tvix josh workspace.
Change-Id: Ic6ad2122733418114b43aa692d6e42ac1e308eeb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12251
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: Ilan Joselevich <personal@ilanjoselevich.com>
It's a `[u8; SIGNATURE_LENGTH]` type alias, and conveys what we're
accepting or returning a bit nicer.
Change-Id: I974cd97d56d383e51417eb0f26e1431a05711922
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12252
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
//tvix depending less on other parts of depot is prefered as it will
help with extending the josh workspace of tvix.
Change-Id: Ifcac3af1782dfd82e7543cb4c3ae57fbd186edff
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12250
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Removes imbl::OrdMap in favour of an Rc over the standard library's BTreeMap,
which allows us to drop the imbl dependency completely.
In my local tests this is actually slightly faster for `hello` and `firefox`.
Change-Id: Ic9597ead4e98bf9530f290c6a94a3c5c3efd0acc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12201
Reviewed-by: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
This vector type has served us well for now, but it contains internal refcounts
which are incompatible with upcoming changes related to garbage collection.
The performance impact of this change within all benchmarks I ran was within the
margin of error:
[nix-shell:/tmp/perf]$ hyperfine "./before -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings"
Benchmark 1: ./u64 -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings
Time (mean ± σ): 7.528 s ± 0.272 s [User: 6.578 s, System: 0.631 s]
Range (min … max): 7.160 s … 8.012 s 10 runs
nix-shell:/tmp/perf]$ hyperfine "./std-vec -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings"
Benchmark 1: ./std-vec -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings
Time (mean ± σ): 7.515 s ± 0.178 s [User: 6.508 s, System: 0.652 s]
Range (min … max): 7.276 s … 7.861 s 10 runs
Change-Id: Ib95f871956e336a1e5771f6293583854b1efb276
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12197
Reviewed-by: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
This replaces the OpCode enum with a new Op enum which is guaranteed to fit in a
single byte. Instead of carrying enum variants with data, every variant that has
runtime data encodes it into the `Vec<u8>` that a `Chunk` now carries.
This has several advantages:
* Less stack space is required at runtime, and fewer allocations are required
while compiling.
* The OpCode doesn't need to carry "weird" special-cased data variants anymore.
* It is faster (albeit, not by much). On my laptop, results consistently look
approximately like this:
Benchmark 1: ./before -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings
Time (mean ± σ): 8.224 s ± 0.272 s [User: 7.149 s, System: 0.688 s]
Range (min … max): 7.759 s … 8.583 s 10 runs
Benchmark 2: ./after -E '(import <nixpkgs> {}).firefox.outPath' --log-level ERROR --no-warnings
Time (mean ± σ): 8.000 s ± 0.198 s [User: 7.036 s, System: 0.633 s]
Range (min … max): 7.718 s … 8.334 s 10 runs
See notes below for why the performance impact might be less than expected.
* It is faster while at the same time dropping some optimisations we previously
performed.
This has several disadvantages:
* The code is closer to how one would write it in C or Go.
* Bit shifting!
* There is (for now) slightly more code than before.
On performance I have the following thoughts at the moment:
In order to prepare for adding GC, there's a couple of places in Tvix where I'd
like to fence off certain kinds of complexity (such as mutating bytecode, which,
for various reaons, also has to be part of data that is subject to GC). With
this change, we can drop optimisations like retroactively modifying existing
bytecode and *still* achieve better performance than before.
I believe that this is currently worth it to pave the way for changes that are
more significant for performance.
In general this also opens other avenues of optimisation: For example, we can
profile which argument sizes actually exist and remove the copy overhead of
varint decoding (which does show up in profiles) by using more adequately sized
types for, e.g., constant indices.
Known regressions:
* Op::Constant is no longer printing its values in disassembly (this can be
fixed, I just didn't get around to it, will do separately).
Change-Id: Id9b3a4254623a45de03069dbdb70b8349e976743
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12191
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Since we switched from reference scanning to string context, this only
handles the `__corepkgs__` hack. Update the docstrings.
Change-Id: Ie857c8c99ae1cdb4697323ec738f88be0580df3e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12246
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
I'm gonna be doing some poking around in the internals of Context, so in
preparation this pulls it out into its own module.
Change-Id: I72ea7df80b5f36f838934ee07bdba66874c334c9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12189
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Don't use ValidateNodeError, but SymlinkTargetError.
Also, add checks for too long symlink targets.
Change-Id: I4b533325d494232ff9d0b3f4f695f5a1a0a36199
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12230
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: edef <edef@edef.eu>
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
Tested-by: BuildkiteCI
Don't use DirectoryError, but PathComponentError.
Also add checks for too long path components.
Change-Id: Ia9deb9dd0351138baadb2e9c9454c3e019d5a45e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12229
Tested-by: BuildkiteCI
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: edef <edef@edef.eu>
Replace the hand-rolled code comparing names with a try_fold. Also, make
it slightly stricter here, detecting duplicates in the same fields
earlier.
Change-Id: I9c560838ece88c3b8b339249a8ecbf3b05969538
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12226
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: edef <edef@edef.eu>
Tested-by: BuildkiteCI
This provides a batched variant to construct a Directory, which reuses
the previously calculated size.
Checking and inserting code is factored out into a check_insert_node
function, taking the current size as a parameter and returning the new
size.
Change-Id: Ia6c2970a0c12181b7c40e63cf7ce8c93298ea37c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12225
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: edef <edef@edef.eu>
Tested-by: BuildkiteCI
The boot tests are sometimes flaky, and we don't want them to
periodically fail other build. Have Buildkite auto-retry them 2 times
on failure.
Logs for individual failures are still available, so it won't hinder
flakiness debuggability.
See https://buildkite.com/docs/pipelines/command-step#retry-attributes
for a documentation of a parameter, and cl/8983 for the introduction of
that feature to //nix/buildkite.
Change-Id: I1c0d25fa1d0ca940b3bdcd145ede87154b0c28eb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12228
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
Tested-by: BuildkiteCI
This is covered by clippy.toml these days.
Change-Id: I2330af5781844d5f9d975793d770efcea48d371b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12223
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Make this a proper struct with named fields. We apparently never access
B3Digest in there, so it can be removed.
Change-Id: Ifc07310393e1afb0a835778eae137a19b54070b9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12224
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Provide a into_nodes() function on a Directory, which consumes self and
returns owned PathComponent and Node.
Use it to provide a proper conversion from Directory to the proto
variant that doesn't clone.
There's no need for the one taking only &Directory, we don't use it
anywhere, and once someone needs that they might as well clone Directory
before converting it.
Update all other users of the `.nodes()` function to use `.into_nodes()`
where applicable, and avoid some more cloning there.
Change-Id: Id4577b9eb173c012e225337458898d3937112bcb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12218
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This encodes a verified component on the type level. Internally, it
contains a bytes::Bytes.
The castore Path/PathBuf component() and file_name() methods now
return this type, the old ones returning bytes were renamed to
component_bytes() and component_file_name() respectively.
We can drop the directory_reject_invalid_name test - it's not possible
anymore to pass an invalid name to Directories::add.
Invalid names in the Directory proto are still being tested to be
rejected in the validate_invalid_names tests.
Change-Id: Ide4d16415dfd50b7e2d7e0c36d42a3bbeeb9b6c5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12217
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
Add a `SymlinkTarget` type to represent validated symlink targets.
With this, no invalid states are representable, so we can make `Node` be
just an enum of all three kind of types, and allow access to these
fields directly.
Change-Id: I20bdd480c8d5e64a827649f303c97023b7e390f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12216
Reviewed-by: benjaminedwardwebb <benjaminedwardwebb@gmail.com>
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
Nodes only have names if they're contained inside a Directory, or if
they're a root node and have something else possibly giving them a name
externally.
This removes all `name` fields in the three different Nodes, and instead
maintains it inside a BTreeMap inside the Directory.
It also removes the NamedNode trait (they don't have a get_name()), as
well as Node::rename(self, name), and all [Partial]Ord implementations
for Node (as they don't have names to use for sorting).
The `nodes()`, `directories()`, `files()` iterators inside a `Directory`
now return a tuple of Name and Node, as does the RootNodesProvider.
The different {Directory,File,Symlink}Node struct constructors got
simpler, and the {Directory,File}Node ones became infallible - as
there's no more possibility to represent invalid state.
The proto structs stayed the same - there's now from_name_and_node and
into_name_and_node to convert back and forth between the two `Node`
structs.
Some further cleanups:
The error types for Node validation were renamed. Everything related to
names is now in the DirectoryError (not yet happy about the naming)
There's some leftover cleanups to do:
- There should be a from_(sorted_)iter and into_iter in Directory, so
we can construct and deconstruct in one go.
That should also enable us to implement conversions from and to the
proto representation that moves, rather than clones.
- The BuildRequest and PathInfo structs are still proto-based, so we
still do a bunch of conversions back and forth there (and have some
ugly expect there). There's not much point for error handling here,
this will be moved to stricter types in a followup CL.
Change-Id: I7369a8e3a426f44419c349077cb4fcab2044ebb6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12205
Tested-by: BuildkiteCI
Reviewed-by: yuka <yuka@yuka.dev>
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: benjaminedwardwebb <benjaminedwardwebb@gmail.com>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
The tvix-cli tests now include evaluating the NixOS GNOME installer ISO
image and making sure that its drvPath and outPath matches the one
evaluated by Nix. (This required extending the helper function a bit and
adding docs).
NixOS docs generation is disabled for now, see comments in diff.
Change-Id: Ia510f209b1ec3ef9a823f1e5ac0ef2f5f193976f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12177
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This splits the existing ReferenceScanner into a ReferenceScanner and
ReferencePattern as well as adds an AsyncRead implementation that can
do a scan while you read from it.
The reason to split the scanner in two is that generating the pattern
is expensive and when ingesting build results with multiple outputs you
want to do several independant scans that look for the same pattern.
The reader is for scanning files without having to load the entire file
into memory.
Change-Id: I993f5a32308c12d9035840f8e04fe82e8dc1d962
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12052
Autosubmit: Brian Olsen <me@griff.name>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This comment is a bit misleading - everything that's not in this field
is referenced in a content-addressed fashion (by their digest).
Change-Id: I5097131530fd188173393063643c057f588ea2c4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12219
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
