Fix the nixpkgs eval hyperfine benchmark in tvix/scripts.bench.sh to:
- Eval both a pinned (never-changing) nixpkgs commit and the nixpkgs
commit used by the depot
- Not depend on fetchTarball, which is not yet implemented.
- Be called bench-windtunnel.sh
Change-Id: Id2ae18f983ab7327625320b5b16c082ecc369a49
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10364
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This moves the failing example from cl/10285 into its separate test
case.
There were multiple complications: tvix-[ca]store was panicking in some
places, rather than returning an error. This is now fixed.
It needs to live in tvix-glue, so we actually have a "proper" EvalIO
interface doing something.
> toString ({ line = 42; col = 42; file = /deep/thought; }.file)
Should not cause an error, because it shouldn't trigger an import, but
leave the path as-is, and not care about it not being present.
Change-Id: I76f70b3cb1f73a0fb05870375710fd9f67d5603c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10342
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: sterni <sternenseemann@systemli.org>
The default behavior of string coercion in C++ Nix is to weakly coerce
and import to store if necessary. There is a flag to make it strongly
coerce (coerceMore) and a flag that controls whether path values have
the corresponding file/directory imported into the store before
returning the (store) path as a string (copyToStore). We need to
implement our equivalent to the copyToStore (import_paths) flag for the
benefit of weak coercions that don't import into the store (dirOf,
baseNameOf, readFile, ...) and strong coercions that don't import into
the store (toString).
This makes coerce_to_string as well as CoercionKind weirder and more
versatile, but prevents us from reimplementing parts of the coercion
logic constantly as can be seen in the case of baseNameOf.
Note that it is not possible to test this properly in //tvix/eval tests
due to the lack of an appropriate EvalIO implementation being available.
Tests should be added to //tvix/glue down the line.
Change-Id: I8fb8ab99c7fe08e311d2ba1c36960746bf22f566
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10361
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
This matches the behavior of C++ Nix more closely where the decision is
made based on the first type based to ExprConcatStrings:
1f93fa2ed2/src/libexpr/eval.cc (L1967-L2025)
Note that this doesn't make a difference in any successful
evaluation (at least to my knowledge), but ensures that our error
messages will match C++ Nix more closely, e.g. in the case of
`1 + "string"`.
Change-Id: I8059930788f9c8d98baf98e3d93d8a060ef961f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10360
Tested-by: BuildkiteCI
Autosubmit: sterni <sternenseemann@systemli.org>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
This comment didn't make a lot of sense before.
Change-Id: Ie057a133ca4b1a099ed3c885e32316b0d87c5eb0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10339
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Namely, all trait implementations should reject invalid data being fed,
and detect invalid data being returned.
b/355 tracks writing some more tests for this, to ensure we're compliant
with this.
Change-Id: I3b05752932837ce208785efb21ffc21508b4b33a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10338
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: flokli <flokli@flokli.de>
This ensures importing these paths also behave the same way as Nix.
Change-Id: Icaa507bbe3d9867a301fc7a300c5d2b3f9feb911
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10355
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This creates a directory with a .keep file inside, and uses
"${path/to/there}" to coerce it to a string (and import it into the
store), ensuring it calculates the same store paths as Nix does.
Change-Id: Ie14ae075104ce278bc4f2cce93aab5762a2734d1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10343
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
If the path specified doesn't exist, construct a proper error instead
of panicking.
Part of b/344.
Change-Id: Id5c6a91248b0a387f3e8f138f8e686e402009e8f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10330
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
This will emit a log event / trace in case this function returns an
error-y type.
Change-Id: I48db6807f3e42304357c422a2b6e177cb8b95228
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10329
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
For all these calls, the caller has enough context about what it did, so
it should be fine to use io::Result here.
We pretty much only constructed crate::Error::StorageError before
anyways, so this conveys *more* information.
Change-Id: I5cabb3769c9c2314bab926d34dda748fda9d3ccc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10328
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This commit fixes out builtins.length so it propagates catchables
like cppnix does.
Change-Id: I7670bec5eee1d4cd3f67a04c9a6808979fb56a8d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10315
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
This commit fixes builtins.filter so it propagates catchables
correctly.
Change-Id: Ib23a383bc5e272e42052205ffd1e94649a0ebc47
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10313
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This adds an unimplemented placeholder for builtins.hashString.
Change-Id: Ibc770103acf5dbc3ea7589ab5ca23fe6e07bd91a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10311
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
This commit fixes builtins.elemAt so it propagates catchables like
cppnix does.
Change-Id: Ieca5e128da17e78af0b14dae4a28a1ff8796e4f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10308
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This commit adjusts the error produced by STRUCTURED_ATTRS so that
it is catchable. This way we are able to enumerate the release
packageset, and the enumeration process will simply treat the few
derivations using structured attributes as being broken, rather than
killing the whole eval session.
Change-Id: I2e17638b8e3227f88543c3718aaf505deaec22ae
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10306
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This fixes our implementation of builtins.splitVersion so it
propagates catchables like cppnix does.
Change-Id: Id5d83ea76229f8c8f202aa42353cb609e67de43f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10305
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Currently this just `throw`s a message explaining that it is not
implemented. This is necessary in order to allow enumerating the
nixpkgs release attrset (afaict only one package uses this builtin).
Change-Id: I45266d46af579ddb5856b192b6be4b481369543c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10302
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
... since this may import them to the store which changes their
basename.
Fixes b/350.
Change-Id: Iabd08ff4d6a424c66d6d7784d7a96b0c078f0a91
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10298
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
This commit adds __curPos (to the global scope, yuck) and
builtins.filterSource. These are not implemented; forcing them will
produce the same result as `throw "message"`.
Unfortunately these two post-2.3 features are used throughout
nixpkgs. Since an unresolved indentifier is a catchable error, this
breaks the entire release eval. With this commit, it simply causes
those broken packages that use these features to appear as they are:
broken.
Change-Id: Ib43dea571f6a9fab4d54869349f80ee4ec5424c2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10297
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
This commit fixes our handling of `throw` within an `assert`
condition.
Fixes: b/340
Change-Id: I40a383639ec266da50a853f16216b1b7868495da
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10318
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This commit fixes out `?` operator so it correctly propagates
catchables.
Change-Id: Iebaa153a8492101ee3ddd29893c98730ff331547
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10317
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Previously, using a catchable as either argument of OpAttrsSelect
would result in an unrecoverable error. This commit matches cppnix
behavior by propagating the catchable.
Change-Id: I4877f4068ec2b823225f185290693c101d0b9c9e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10303
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
When attempting to call a Value, if it is a Value::Catchable we must
not cause an uncatchable failure. This commit simply reuses the
Value::Catchable as the result of attempting to call it. This is
safe because nix is designed so that nix code cannot distinguish
between different catchable failures -- they all look the same to
the interpreted code.
This fixes b/351.
Change-Id: Ibf763a08753e541843626182ff59fdbf15ea2959
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10300
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
error[E006]: expected value of type 'bool', but found a 'internal[catchable]'
--> src/tests/tvix_tests/notyetpassing/eval-okay-test-catchables-in-implications.nix:1:43
|
1 | (builtins.tryEval (({ foo ? throw "up" }: foo -> true) { })).success
| ^^^^^^^^^^^
Relates to b/345
Change-Id: Ic331c32ea59bf67ae775f485b444dc6804ca13d5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10289
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
This commit adds Opcode::OpJumpIfCatchable, which can be inserted
ahead of most VM operations which expect a boolean on the stack, in
order to handle catchables in branching position properly.
Other than remembering to patch the jump, no other changes should be
required.
This commit also fixes b/343 by emitting this new opcode when
compiling if-then-else. There are probably other places where we
need to do the same thing.
Change-Id: I48de3010014c0bbeba15d34fc0d4800e0bb5a1ef
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10288
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
This is a test case for b/343, wherein tvix dies if you try to
branch on an argument whose defaulted value is a catchable.
Change-Id: I891ca825e39ad14dda9f220f06d9591874fcd45d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10287
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
After this commit, the only non-builtins uses of generators are:
- coerce_to_string() uses generators::request_enter_lambda()
- Thunk::force() uses generators::request_enter_lambda()
That's it! Once those two are taken care of, GenCo can become an
implementation detail of `builtins::BuiltinGen`. No more crazy
nonlocal flow control within the interpreter: if you've got a GenCo
floating around in your code it's because you're writing a builtin,
which isn't part of the core interpreter. The interpreter won't
need GenCos to talk to itself anymore.
Technically generators::request_path_import() is also used by
coerce_to_string(), but that's just because the io_handle happens to
be part of the VM. There's no recursion-depth issue there, so the
call doesn't need to go through the generator mechanism
(request_path_import() doesn't call back to the interpreter!)
Change-Id: I83ce5774d49b88fdafdd61160975b4937a435bb0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10256
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
This commit implements deep_force() nonrecursively, by maintaining
an explicit stack rather than using the call stack for recursion.
As an added bonus, we don't need to pass around the SharedThunkSet
anymore, and can in fact completely eliminate SharedThunkSet.
Change-Id: I7c4f59f37834d451a28bf6be317eb0a90eac4ee6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10252
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
The comment explaining ThunkSet makes it seem like it does the same
think as ThunkRepr::Blackhole. In fact neither one is a substitute
for the other. Let's explain the difference.
Change-Id: I89ceaaa9d3c499edbc7d48f70ca5d11f97666c43
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10250
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
LightSpan::Delayed was introduced in commit
bf286a54bc which claimed that "This
reduces the eval time for `builtins.length (builtins.attrNames
(import <nixpkgs> {}))` by *one third*!"
I am unable to reproduce this result. In fact, dropping the
LightSpan::Delayed variant of the enum makes eval of the same
expression slightly faster! I also tried a large evaluation
(pkgsCross...hello) and got similar results: slightly faster,
slightly less memory. See git footers.
I suspect that there was some unrelated horrific inefficiency that
has since been fixed. The avoided computation in `get_span()` is
nothing more than a binary search! If this were in fact a major
performance issue we could simply precompute the mapping from
CodeIdx to Span when the Chunk becomes immutable (i.e. at the end of
the compilation process, when compiler backtracking is no longer a
concern). Since a Span is just 64 bits this is not a space issue,
and since binary search is much simpler than compiling Nix
expressions it isn't a performance issue either.
Technically there is no longer any reason to have LightSpan since it
is now a single-variant enum. However there is no rush to remove
it, since Rust will optimize its representation into the same thing
you'd get if you replaced LightSpan by Span.
Prev-Benchmark: {"nixpkgs-attrnames":{"kbytes":"233824","system":"0.32","user":"2.02"}}
This-Benchmark: {"nixpkgs-attrnames":{"kbytes":"230192","system":"0.29","user":"2.00"}}
Prev-Benchmark: {"pkgsCross.aarch64-multiplatform.hello.outPath":{"kbytes":"458936","system":"0.73","user":"5.36"}}
This-Benchmark: {"pkgsCross.aarch64-multiplatform.hello.outPath":{"kbytes":"451808","system":"0.53","user":"5.10"}}
Change-Id: Ib9e04806850aa1fc4e66e2a042703986440a7b4e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10254
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
cl/7558 used this expression as a benchmark to justify the introduction
of LightSpan::Delayed:
builtins.length (builtins.attrNames (import ${pkgs.path} {}))
Let's add it as a benchmark case so it can be referenced easily.
Benchmark: {"nixpkgs-attrnames":{"kbytes":"233824","system":"0.32","user":"2.02"}}
Change-Id: Idb6c69ddd284605dd3b5fd9ac5c79a69b9a470b7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10253
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This is just a alias for Result<_, io::Error>, but shorter.
Change-Id: I7c22f61b85e3014885a747b5c1e5abd11b0ef17d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10327
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
This commit fixes b/338 by properly propagating catchables through
comparison operations.
Change-Id: I6b0283a40f228ecf9a6398d24c060bdacb1077cf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10221
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
Commit 05f42519b5 fixed b/281 by
establishing a hygiene regimen to partition *catchable* errors
(i.e. those which tryEval can detect) from all other errors, like
internal VM failures or I/O errors (which Nix must not be allowed to
detect, since these errors are fundamentally impure).
Unfotunately there are still cases where tvix assumes that anything
other than Value::Bool means it should panic!(). I found another
one, and added a test case for it in:
eval_okay_src_tests_tvix_tests_eval_okay_compare_ordering_catchable_nix
Not yet passing.
Change-Id: I69c62ed9ea5c8f81870e8de5c5fe12dcde849763
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10220
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This commit rewrites Value::nix_cmp_ordering() into an equivalent
nonrecursive form. Except for calls to Thunk::force(), the new form
no longer uses generators, and is async only because of the fact
that it calls Thunk::force().
I originally believed that this commit would make evaluation faster.
In fact it is slightly slower. I believe this is due to the added
vec![] allocation. I am investigating.
Prev-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"460048","system-seconds":"0.68","user-seconds":"5.73"}
This-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"460224","system-seconds":"0.67","user-seconds":"5.84"}
Change-Id: Ic627bc220d9c5aa3c5e68b9b8bf199837cd55af5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10212
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
This is part of a fix for b/338.
We should never use PartialOrd::partial_cmp().
All Nix types except floats are obviously totally-ordered. In
addition, it turns out that because Nix treats division by zero
rather than producing a NaN, and because it does not support
"negative zero", even floats are in fact totally ordered in Nix.
Therefore, every call to PartialOrd::partial_cmp() in tvix is an
error. We have to *implement* this function, but we should never
call it on built-in types.
Moreover, nix_cmp_ordering() currently returns an Option<Ordering>.
I'm not sure what was going on there, since it's impossible for it
to return None. This commit fixes it to return simply Ordering
rather than Option<Ordering>.
Change-Id: If5c084164cf19cfb38c5a15554c0422faa5f895d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10218
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This commit rewrites Value::nix_eq() into an equivalent. Except for
calls to Thunk::force(), the new form no longer uses generators, and
is async only because of the fact that it calls Thunk::force().
I believed that the nonrecursive form would be faster. It is, in
fact, slightly slower. I believe this is due to the vec![]
allocation; I am investigating.
Prev-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"459068","system-seconds":"0.71","user-seconds":"5.39"}
This-Nixpkgs-Benchmark: {"attrpath":"pkgsCross.aarch64-multiplatform.hello.outPath","peak-kbytes":"460048","system-seconds":"0.68","user-seconds":"5.73"}
Change-Id: I10f4868891e4b7475df13f0cbc41ec78dd985dd8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10118
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
This commit adds a simple MVP benchmark, built on our nix
infrastructure instead of cargo. It simply runs `tvix-eval` inside
of GNU time, and prints the three essential statistics in a short
JSON blob.
You can run the benchmark with a simple `nix run`, like:
nix run -f . tvix.cli.benchmark-hello
nix run -f . tvix.cli.benchmark-firefox
nix run -f . tvix.cli.benchmark-cross-firefox
Currently these blobs are stored only in the CI logs, which I'm sure
get garbage-collected at some point. We should be putting them in
the git trailers, but that can wait for a future CL.
I tried using `cargo bench` for this but found it incredibly
frustrating. Maybe I'm doing it wrong. It seems to be designed for
microbenchmarks only, and very hard to control. It kept building
all sorts of unnecessary stuff (like the tests), and unlike
crate2nix it was doing all the builds on only a single machine
instead of using more than one machine. Worse, for that single
machine it kept picking my laptop instead of my fast servers! It
seems excessively cargo-flavored for such a straightforward task.
Benchmark: {"hello.outPath":{"kbytes":"244736","system":"0.36","user":"2.76"}}
Benchmark: {"firefox.outPath":{"kbytes":"1506736","system":"2.38","user":"32.01"}}
Benchmark: {"pkgsCross.aarch64-multiplatform.firefox.outPath":{"kbytes":"11334548","system":"10.70","user":"107.07"}}
Change-Id: I85bc046ec551360284d7ecfc81a03914f0085909
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10216
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This adds a criterion.rs-based testbench into tvix-glue.
It can be invoked by running `cargo bench` from inside the `tvix-glue`
crate.
`target/criterion/report/index.html` contains nice graphs.
It's able to diff against the previous run, so you can invoke `cargo
bench` before and after a certain change to reason about the impact in
evaluation performance.
Currently, we need to create a bunch of Evaluator resources inside the
benchmark loop itself, which is a bit annoying, as it leaks into the
things we benchmark.
This should become better with b/262.
Fixes b/322.
Change-Id: I91656a308887baa1d459ed54d58baae919a4aaf2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10245
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
cl/9364 did introduce a warning here, which is visible when building in
release mode - or invoking `cargo bench` in tvix-glue.
Change-Id: Ia82082a58543f0fdd32866fdfcd37d0a5fdfda9c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10261
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
We don't really require the Path to be a PathBuf, we don't even require
it to be a Path, we only need it to be AsRef<Path>>.
This removes some conversion in the from_addr cases, which can just
reuse `url.path()` (a `&str`).
Change-Id: I38d536dbaf0b44421e41f211a9ad2b13605179e9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10258
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
It's been a while since the last sled release, and that one binds to a
pretty old version of zstd, requiring workarounds like cl/10090.
Upstream sled main branch currently has zstd halfway patched out (it's
a no-op, but the feature flag and options are still there), and it's in
that state for a year.
Rather than maintaining our own fork of sled, let's just stop using the
compression feature in sled, dropping the version pin to zstd that way,
removing the need for cl/10090.
This doesn't mean we won't reintroduce per-blob compression - but we
probably just won't let sled take care of the compression, but do it
ourselves - which is necessary for more chunked blob storage anyways.
Even though we do drop the feature flag, we still need to explicitly use
use_compression(false).
Change-Id: I0e4892d29e41c76653272dc1a3625180da6fee12
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10257
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
This only contained the (unused) evaluator.proto file.
Considering we're less likely to have the CLI talk to a long-running
evaluator, but instead embed the evaluator inside the CLI, remove this.
If we add a RPC to speak to an evaluator, we can resurrect this from
git history.
Change-Id: I2196aade55221660330dfd32dc3e52c39ec6ed43
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10241
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Only pass in the proto files that are actually needed to build that
crate. They are already constructed in depot.tvix.$crate.protos.protos.
Change-Id: If4381e6c3350e420ee4ddce1e0513bfe970678a2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10240
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Break up the go-bindings derivation. Keep "protos" containing all proto
files (well, and the buf config), and use it for a check phase running
linter and formatter, as well as the existing "go-bindings" attribute
Change-Id: I52cb9d08570bb76452acb831eb711c5b6c0eacfb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10239
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This function converts from a nix_compat::derivation::Derivation to
a BuildRequest.
In addition to the Derivation itself, it needs two lookup functions to
map input paths to their castore nodes.
Change-Id: I0332982f0bc7933a5fda137fe39d5a850639d929
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10236
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
The command is called `regenerate`, not `generate`.
Change-Id: I18075042ebd461e4dd0718a936e6bbe738a144d5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10259
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This adds the tvix-build crate, currently only containing a
`tvix_build::proto` module, exposing the data structures defined in
tvix/build/protos.
Change-Id: I75f5d9196969ed0877b1fe640cacfecba0fb2e03
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10235
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
This got renamed recently, but we forgot to update it here.
Change-Id: I7d713c8a0e6ccca57fe67985d9cb4e7f1eeef3b2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10243
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Add some hyperfine benchmarks to Tvix's windtunnel benchmark script for
evaluating the outPath of hello and a cross-compiled hello.
Change-Id: I9d76e5ce0a3fd7d9c125c36c5fced675b660a8a8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10248
Reviewed-by: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Autosubmit: grfn <grfn@gws.fyi>
Currently we produce wrong drvPaths for a large number of packages
that use fetchurl (but not fetchurlBoot, which is what stdenv uses).
A simple reproducer is `pkgs.perl538`.
I debugged this down to the root cause, which is the fact that tvix
doesn't realize that the mapping from FOD-paths to outputHash is
*NOT* a 1:1 mapping. It is a many-to-one mapping. You can have
lots of different FODs with the same outputHash or even the same
outPath. For example, perl538.src and perldevel.src use the same
source tarball but a different `version`.
Anyways, I have found the root cause but have run out of time for a
while, so I've added a panic!() to in the spot where we have a logic
bug in order to call it out.
Change-Id: I9766b39cfe2fe7eafec84945b2ad6cc28f9c4b7d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9364
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
Also run clippy on tests, and enable all features.
Change-Id: Ide9f1bc9f565333072afb918c391c7930b658f41
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10234
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
We match to destructure a single pattern.
Change-Id: I564a3510b4860e90b3315a9639effc48ee88b483
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10233
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This pushes to a Vec immediately after creation.
Change-Id: I2360b45810475d98ededc1d097fb4cbdeabc576b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10232
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
The only thing this was doing was invoking nix-store --add, which is not
gonna help us populate the tvix-store at all (and we now have
`tvix-store import`).
This is also (rightfully) causing clippy warnings, because of some
fields being unused.
It's more of a skeleton, and rather than shelling out to Nix for some
usecases, we might introduce a "compatible" Nix CLI frontend for a
subset of commands.
Drop this for now, to decrease the noise and confusion.
Change-Id: I2fd399e9320260f08893b685561755af9c7c961c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10226
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
The only non-test usage was only checking for the error case, and we can
still convert this to an owned StorePath by calling to_owned() on
StorePathRef.
Change-Id: I9f67a759e580c9c429c96896bcdd295392aa5a2a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10225
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Match the naming in parse_input_derivations, call the keys
"input_derivation", and the values "output_names".
Change-Id: I7d1974819028f8ea543dc3ad78afb803ff9db865
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10224
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
The golang mothership seems to be monkeying with hashes again.
Change-Id: I7430b4cde84fa51be2b572fba02e3567864bb87a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10209
Tested-by: BuildkiteCI
Autosubmit: Adam Joseph <adam@westernsemico.com>
Reviewed-by: flokli <flokli@flokli.de>
This runs `crate2nix generate` in CI and then runs `depotfmt` on the
result to ensure that our machine-generated code is really, really
readable and pretty. Then it checks that the result of all that
is identical to the committed Cargo.nix.
A self-hashing FOD is used to allow network access.
No magic hashes are involved.
Co-Authored-By: Florian Klink <flokli@flokli.de>
Change-Id: I68ec5003dbc6a40894a5a4d6e902f138c99f6719
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10194
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Allow running the benchmark script from any directory - primarily so
Windtunnel can point to the /tvix josh workspace rather than the depot
overall
Change-Id: Ie5fc3ef995bf8114277298ae5c5010e6a0bf13ac
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10205
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
This commit rewrites Thunk::force() so that it is not (directly)
self-recursive. It maintains a Vec of all the
previously-encountered thunks which point to the one it is currently
forcing, rather than recursively calling itself.
Benefits:
- Short term:
This commit saves the cost of a round-trip through the generator
machinery for the generators::request_force() which is removed by
this commit.
- Medium term:
Once a similar transformation has been applied to nix_cmp(),
nix_add(), nix_eq(), and coerce_to_string(), those four functions,
along with Thunk::force(), will make non-tail calls only to each
other. They can then be merged into a single tail-recursive
function which does not use the generator machinery at all:
enum Task { Cmp, Add, Eq, CoerceToString, Force};
fn Value::walk(task:Task, v1:Value, v2:Value) {
// ...
- Long term:
The long-term goal here is to use generators **only for builtins**
and [Marionette]-style remote control of the VM. In other words:
use `async` for things that actually involve concurrency. Calls
from the VM to builtins can then be blocking calls, because even
cppnix will overflow the stack if you make a MAX_STACK_DEPTH-deep
recursive call which passes through a builtin at every stack frame
(e.g. `{ func = builtins.sort (a: b: ... func ...) ...}`).
This way the inner "tight loop" of the interpreter doesn't pay the
costs of `async` and generators. These costs manifest in terms
of: performance, complex nonlocal control flow, and language
impediments (async Rust is a restricted subset of real Rust, and
is missing things like traits).
[Marionette]: https://firefox-source-docs.mozilla.org/testing/marionette/Intro.html
Change-Id: I6179b8abb2ea0492180fcb347f37595a14665777
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10039
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
If builtins.substring is invoked with (byte!!) offsets that aren't at
codepoint boundaries, return an error rather than panicking. This is
still incorrect (see b/337) but pushes the incorrectness forward a step.
Change-Id: I5a4261f2ff250874cd36489ef598dcf886669d04
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10199
Tested-by: BuildkiteCI
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Currently this just uses a Docker container, which is gross but works
fine for now since we don't have the ability to build benchmarks in nix
as of cl/7538
Change-Id: I48e317f44bc2c73533d7663403786a3a37c7952f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10189
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
Autosubmit: grfn <grfn@gws.fyi>
This now exists in tvix-store directly, as NixHTTPPathInfoService, and
contrary to this version, also validates signatures.
Change-Id: Ib6ca161e40d627b7d9741839fc849f2392f422da
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10155
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This allows setitng the trusted-public-keys URL parameter to a
(whitespace-separated) list of public keys.
NARInfo files retrieved need to contain a valid signature.
Change-Id: Ifd6580b723cbae3182e9cadfa54f1ca2b41d6599
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10153
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Introduce an Option<Vec<narinfo::PubKey>>, configurable with a
`set_public_keys` method.
If set, this configures NixHTTPPathInfoService to validate signatures.
Change-Id: I157c5e13c41fc9bfd40b0655381fb4cf33900868
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10152
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This makes it easy for each PubKey to check if a given Signature is
correct for a given fingerprint.
Change-Id: I56e6211d133f74f390fd1ae3ae799eef12221904
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10151
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This represents a ed25519 public key and "name".
These are normally passed in the `trusted-public-keys` Nix config option,
and consist of a name and base64-encoded ed25519 pubkey, separated by a `:`.
Change-Id: I9ab4b3e0e5821805ea6faf2499626630fc5a3f0a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10150
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Just call this Error, we can infer from the package what error this is.
Change-Id: I5df25d2873ec739c49c08804f35562c84c222e06
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10149
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Ensure the initially communicated NarHash/NarSize from the NarInfo
matches what we read, and don't return a PathInfo message if there's
a mismatch.
Also move the buffering layer around a bit.
Change-Id: I68c60ecfaf0f9cd5edacea648437ecb0c9729251
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10148
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
An error in the PathInfoService request can appear in case the
underlying request returns an error.
We shouldn't panic and bork the fuse mount, but instead return an IO
error.
Change-Id: I2daeae629e1627d06adcd7b82ddb76c50c602212
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10154
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
* picked avrdude from stable channel
* removed override for texlive, as the upstream fix is merged
* picked awscli2 from stable channel
* bump tdlib to 1.8.21 (new minimum for telega.el)
* tvix/turbofetch: switch to nixpkgs-native mechanism for
CARGO_MANIFEST_LINKS (whatever that is)
Change-Id: Ic695721b5ca750b89d21cab7a257e1db682b23c0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10083
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>
We should restrict this to alphanumeric mostly, and we definitely don't
want newlines.
Not entirely sure about the exact additionally allowed characters
outside of alphanumeric, but this can always be extended further.
Change-Id: I1357e79e553f2df2fa97792889f63f0f35d50ed5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10147
Reviewed-by: edef <edef@edef.eu>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
BASE64.decode_mut panics if we're passing data that has the wrong size.
Do the size check first and error out there.
Also update the error, and talk about b64-encoded sizes.
Change-Id: I290f80a37d48526a30bf1df9d1d9fe34865008eb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10146
Tested-by: BuildkiteCI
Reviewed-by: edef <edef@edef.eu>
Autosubmit: flokli <flokli@flokli.de>
No need to hardcode magic numbers here, we have a constant for that.
Change-Id: I67b671c0c4bb7c3bfb001e9c36499f31873ee717
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10145
Reviewed-by: edef <edef@edef.eu>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This fixes a future clippy lint.
Change-Id: Ic830e94ef23595580c1037f10878c76bbb546dd9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10110
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
This commit adds Thunk::unwrap_or_clone(), which uses
Rc::try_unwrap() to avoid cloning the Value out of a an Rc which has
only one strong reference.
Change-Id: Icacefe0c823dcddf046d90c0c5cd5ed59fe976d4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10037
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI
Change-Id: Ibddaa111a5b7a86c42dbe153ae8e53f9a5601a54
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10112
Tested-by: BuildkiteCI
Reviewed-by: Adam Joseph <adam@westernsemico.com>
This allows seeing a PathInfo as a nix_compat::narinfo::NarInfo<'_>.
It doesn't allocate any new data, but the NarInfo<'_> view allows us to
access things like signature verification, or rendering out
(alternations of this) as strings.
Change-Id: Id0d8d7feeb626ee02c3d8a4932f24ace77022619
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10108
Reviewed-by: edef <edef@edef.eu>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This is useful when creating a new Signature struct where the individual
elements are already parsed.
Change-Id: Ie33c66287641951e7a030aaa1e7ff0a86b2628ac
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10111
Reviewed-by: edef <edef@edef.eu>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
We can simply use .to_owned() on that thing afterwards if we want to
construct an owned StorePath.
Change-Id: I0f3e2e4434b99ee522f2a7dbfa391e13a987479c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10105
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: edef <edef@edef.eu>
Tested-by: BuildkiteCI
We always know this needs to end with a .drv, and fail parsing if it
doesn't, so there's no need to hang onto these 4 bytes.
This will make it much easier to synthesize a NarInfo<'_> later on from
a PathInfo proto, because we don't have to make this ".drv" appear out
of thin air.
Change-Id: Id95e7fd937d7c9a420a39b5a4bab73985640ca3b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10084
Tested-by: BuildkiteCI
Reviewed-by: edef <edef@edef.eu>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
These were used to format to and parse from strings.
Move this to the CAHash and NixHash structs directly, and be explicit in
the name about which encoding for digests is used.
For output path calculation, nix encodes the nixpaths in hex, but for
writing out NARInfos, it's using nixbase32.
Change-Id: Ia585a76a3811b2609e7ce259fda66a29403b7e07
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10079
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This adds support to compute the fingerprint string, which is what's
ed25519-signed in binary caches.
Change-Id: I8947239c609896acfd7261f110450014bedf465a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10080
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This adds support for verifying signatures found in NARInfo files.
This still needs to be hooked together with the nix+http[s] backend.
Change-Id: Ic1c8ddbdecfb05cefca2492808388b0f7f3f2637
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10081
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
NixHTTPPathInfoService acts as a bridge in between the Nix HTTP Binary
cache protocol provided by Nix binary caches such as cache.nixos.org,
and the Tvix Store Model.
It implements the [PathInfoService] trait in an interesting way: Every
[PathInfoService::get] fetches the .narinfo and referred NAR file,
inserting components into a [BlobService] and [DirectoryService], then
returning a [PathInfo] struct with the root.
Due to this being quite a costly operation, clients are expected to
layer this service with store composition, so they're only ingested
once.
The client is expected to be (indirectly) using the same [BlobService]
and [DirectoryService], so able to fetch referred Directories and Blobs.
[PathInfoService::put] and [PathInfoService::nar] are not implemented
and return an error if called.
This behaves very similar to the nar-bridge-pathinfo code in nar-bridge,
except it's now in Rust.
Change-Id: Ia03d4fed9d0657965d100299af97cd917a03f2f0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10069
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
There's no need to already require this to be buffered here.
Change-Id: Ib9a11b194e0754d87ab8d2ef0b8cb0f4edc01229
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10074
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Allow taking advantage of the buffer of the underlying reader to avoid
unnecessary copies of file data.
We can't easily implement the methods of BufRead directly, since we
have some extra I/O to perform in the final consume() invocation.
That could be resolved at the cost of additional bookkeeping, but this
will suffice for now.
Change-Id: I8100cf0abd79e7469670b8596bd989be5db44a91
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10089
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
We rely on being able to make small reads cheaply, so this was already
an implicit practical requirement. Requiring it explicitly removes a
performance footgun, and makes further optimisations possible.
Change-Id: I7f65880a41b1d6b5e6bf2e52dfe47d4c49b34bcd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10088
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
We don't need to validate UTF-8 separately, since valid names are
a strict subset of ASCII, and therefore a strict subset of UTF-8.
Change-Id: I3261bf0efe3480b5b315074efafcf5e47a6c5a65
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10087
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
We can just use take(size) to restrict reading to that as a max.
Change-Id: I0fbda74e4fb98ffeababae86a325233416029acf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10072
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This allows converting from the NarInfo falling out of the NarInfo
parser (which is a bit annoying to handle due to lifetimes) to the
PathInfo proto struct.
The narinfo field, containing most of the data from the original
NARInfo file, as well as the references (bytes) are populated.
The node field is not populated, because it requires ingesting the NAR
itself to describe the root node.
Change-Id: I9c04dd6ad4cae556b455188a4255e34b4f6443c5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10067
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
This suggests it's cheap to convert around, but name actually does
allocate.
Move to a `to_owned(&self) -> StorePath`, to better signal that this
does allocate.
Change-Id: Ifaf7c21599e2a467d06e2b4ae1364228370275db
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10066
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Make it less annoying to convert from io::Error to this. We already have
one direction, doesn't hurt to have the other too.
Change-Id: I9fe2c6da608c9d54910ee8c397572aadb1d90d99
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10068
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Having random if blocks and returning from them is error-prone.
Also, turns out we only need the unprefixed scheme in the fallback case,
so move it down to there.
Change-Id: Ifcb09279c963f8a39e0dbabe145990263f3d7cf9
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10041
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
This has been renamed to descend_to in cl/9373.
Change-Id: Ia6201fb81c7d4fa953d311451cfff95373549a50
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10045
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: edef <edef@edef.eu>
Tested-by: BuildkiteCI
This wasn't used at all, let's remove it.
Change-Id: I426e3d93c32ebe65247ae5cf8d05b5bf686be2d6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10044
Tested-by: BuildkiteCI
Reviewed-by: edef <edef@edef.eu>
This moves the sync `channel::from_url` to a async
`tonic::channel_from_url`. It now allows connecting non-lazily if `wait-
connect=1` is set in the URL params.
Also, make the pingpong tests for blobsvc and directorysvc use the wait-
connect=1 codepath.
Change-Id: Ibeea33117c8121814627e7f6aba0e943ae2e92ca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10030
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Make directoryservice::from_addr use the more specific constructors.
Change-Id: I9fee2afed77692505988d631d9fe246d9843d25a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10029
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Make blobservice::from_addr use the more specific constructors.
Change-Id: Id9637e279d6910ce6d92ff0086a984be5c65a8c8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10028
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
All we do is constructing some strings, and checking if from_addr
succeeds or not.
This can be written in a much more concise way using test_case.
Use lazy_static to provide temporary directories.
Also add some more grpc-related test cases.
Change-Id: Ia310dd01f617f7628f1e7e21304ac70da2ab3534
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10027
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
These gRPC PathInfoService tests were actually not too useful in here,
what we're mostly testing is the channel construction, so move it to
there.
Change-Id: Ic8c07558a1b28b46f863d5c39bcaa3a79cea007a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10024
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
We don't gain much from making this part of the trait, it's still up to
`tvix_store::pathinfoservice::from_addr` to do most of the construction.
Move it out of the trait and into the specific *Service impls directly.
This allows further refactorings in followup CLs.
Change-Id: I99b93ef4acd83637a2f4888a1e586f1ca96390dc
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10022
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This appears in the cache.nixos.org dataset.
Change-Id: I2eadafe8441e0132a448828026553da2dc7c12aa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9994
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This appears in the cache.nixos.org dataset.
Change-Id: I35921f7ef148f6681081a4e371abb8c9cc98854d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9993
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Rather than having our own error type, just make decoding errors use
the same common error type.
Change-Id: Ie2c86972f3745c695253adc3214444ac0ab8db6e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9995
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
This appears in the cache.nixos.org dataset.
Change-Id: I055b60b9950a1a6a36c1b0576b957e11e1d4264b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9990
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Do this upgrade whenever someone is actually interested in the children
of a directory, but that directory doesn't contain a more detailed
listing. This is much more predictable, and removes a bunch of confusing
code from the inode tracker itself.
Change-Id: Ib3a13694d6d5d22887d2d04ae429592137f39cb4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9982
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Have it return libc::ENOENT errors rather than an Option<…>.
Also avoid having to traverse inode_data multiple times, by synthesizing
the Arc<…> on our own in the insert case. In that case, the data is
quite small, so cloning it is faster than traversing a second time.
Change-Id: I7ab14bac8bb23859ed8d166a12070d4f4749b6d4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9981
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
As already established in the two previous CLs, these two pieces of code
where doing the same.
Move to a get_directory_children helper.
Change-Id: Id6876f0c34f3f40a31a22d59a2cdbfef39e2d8de
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9980
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Tested-by: BuildkiteCI
Very similar to the previous CL
Change-Id: I0df07ddca742b7b9485d48771c8d295dc3aa7136
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9979
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Code after this big match block only cares about parent_digest and
children, so there's no need to do another inode_tracker.get in there.
This also allows removing another if let block, right after, as we don't
need to destructure parent_data anymore.
Change-Id: I68fbbe3304194670caee5a453722369afa4e77ea
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9978
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
This makes it much harder to keep the read lock around for too long, and
the code a bit easier to understand.
Change-Id: I7d99c85cadd433cad444b8edd34e2c43d7eaf5a8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9977
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Not a single call site actually makes use of the Vec.
Change-Id: I6cf31073c9f443d1702a21937a0c3938c2c643b8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9988
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This small tool formats A-Term in a more readable format. It's a lossy
conversion for non-valid UTF-8 environment values.
Change-Id: I65a51054d7faf528321bc2d9fc4425180a7813f5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9970
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
This will invoke `cargo clippy` on the codebase, ensuring we spot things
early on.
Relates to b/321.
Change-Id: Ifba13e5bef995d33b24346c6cee134e84d6566c4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9106
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
This *might* contain a fix for a clippy lint thrown by that crate.
Relates to b/321.
Change-Id: Ia7ebd3e26e0feb8bcc7a6c811b1e583f9016fd9e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9966
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
Uses the standard library IntoIterator trait for the construction of
our iterators. Clippy complains about duplicating this.
While doing this, I opted to rename the `IntoIter` type into something
that is more useful to users, in case somebody ends up working with
these manually.
This fixes a clippy lint, and is related to b/321.
Change-Id: I851fde0d7b8b38d182343a0fd6d9f8dd2a33ee11
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9963
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: tazjin <tazjin@tvl.su>
Walking a btree_map twice is more expensive than copying a string,
especially because the cloning only happens in the (non-hot) error
path.
This fixes a clippy lint, so it's related to b/321.
Change-Id: I2ccfd0bc46792a45d277f47564e595b87107d8be
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9962
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This pdates the proto docstrings a bit, especially w.r.t. verified
streaming.
It also adds send_chunks, send_bao fields to StatBlobRequest (renamed
from BlobMeta)
Change-Id: I590cc8646d86b73bca9f38a9b6d9ea15e4df5cb6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9951
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
This wasn't doing anything, because $(regenerate) was a directory:
/nix/store/cxfxvz76zh9vb7x26h3cx98gkv234jz2-pb-go-check: line 2: /nix/store/my1nd1qvg5iis38rfyn1pm2c7ib7myn5-regenerate: Is a directory
Change-Id: I6ebed1d7b84dceb885c1f25527ce62d973146819
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9959
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
As surfaced in cl/9959, we didn't actually spot having to run the
regeneration in CI.
Change-Id: Iabdb79339d2a220a6cb368f9c5c70166f1ce9eb3
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9960
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This adds a function that can ingest a NAR file into a given
blob_service and directory_service, returning the root node.
Passing around the directory_putter is a bit annoying, but at least for
this linear "reading through a NAR" case, we don't need it to be clone
or sync.
Change-Id: Ia26d596f5dffc6dbd69aa39a82b59bba41685a9c
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9915
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
This already has the right type.
Change-Id: I8f5850a41f9e97f1bc5f2a45ca05cf7439665c9d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9954
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Having more than 4GiB files is quite possible (think about the NixOS
graphical installer, and an uncompressed iso of it).
No wire format changes.
Change-Id: Ia78a07e4c554e91b93c5b9f8533266e4bd7f22b6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9950
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
There's various bits and pieces in tvix-cli that use both the store and
evaluator, as well as nix-compat. For example, builtins.derivation, as
well as the reference scanning implementation.
This "glue code" currently isn't accessible from anywhere else, but it'd
be very useful if it were.
Move it out into a `glue` crate, and make `tvix-cli` a consumer of it.
All the KnownPaths setup and passing around, as well as NIX_PATH
handling is also something that should probably be moved into the glue
crate as well, but that's something left for a future CL.
Change-Id: I080ed3d1825ab23790666486840f301f00856277
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9908
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Delays allocation (through cloning) of the values to be compared
until *after* the keys have been compared.
Change-Id: I7d68c27d7a0fbcdcc387db7c092bce50ca4b94ea
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9900
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Similar to `into_iter_sorted`, add a marker function for call sites
that want *borrowed* sorted iteration.
Change-Id: I7c6f14e1ac43fdb14b861b3da183eb5d12bba139
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9899
Autosubmit: tazjin <tazjin@tvl.su>
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
builtins.intersectAttrs is used a _lot_ in nixpkgs eval, for whatever
reason. We previously had a very inefficient implementation that would
allocate for each comparison. It stuck out like a sore thumb in perf
analysis.
This moves to a custom algorithm with two iterators, one for the left
and one for the right side, advancing them along the (borrowed) map
keys until a match is found and allocation is required.
I've not made any effort to reduce the verbosity of this code, I don't
think it's worth it.
On my machine this reduces the mean runtime of evaluating
`nixpkgs.emacs.outPath` by ~8%.
Change-Id: Ie506d82cb8d5f45909628f771a6b73e0eca16b27
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9898
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
As described in https://b.tvl.fyi/issues/313, the `nix_oracle` tests
currently fail if run without Nix 2.3 in `$PATH`.
This is not a problem for people running Nix 2.3 on their systems, and
neither is it for the tests running inside Nix derivations itself (as we
hardcode Nix 2.3 there), but for interactive `cargo test` runs, either
using `mg shell //tvix:shell`, or `nix-shell` (from the tvix workspace)
it currently is broken.
Adding Nix 2.3 to the shell fixes that.
Change-Id: Idbe01aa92d63eb1a3dab5797b9be5eba1e8d7f7a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9907
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
The purpose of black_box is to actually prevent the compiler from being
able to optimize computation of the benchmarked function away.
To accomplish this, we need to actually *use* black_box to blackbox the
input data away, rather than the return type.
Change-Id: I5438982f57509fbf7b85034346a2739d76aef1fa
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9902
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Together with temporarily pointing nixpkgs past
b10994c38c61038970a19fa60bfbec21a61755cc, this now fixes cross-
compilation for tvix.
This incorporates the changes proposed in https://cl.tvl.fyi/9888 and
https://cl.tvl.fyi/9889, but by fixing it in crate2nix, and using the
(re-)generated version of it.
Changes were sent upstream at
https://github.com/nix-community/crate2nix/pull/309,
this pulls in a minimal patch for now.
Change-Id: I70bb6f003bbc3e89de9c4eb4985ea4708ac3a9fd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9890
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
These are still a bit WIP, but should already explain the idea better
than not having any documentation about how we intend to implement this
at all.
Let's put this into the repo, as a working document.
Co-Authored-By: edef <edef@edef.eu>
Change-Id: Ic0c5195cdf0a33c8382af1b5cff9064fe6ef9ffb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9894
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
These describe the castore data model, so it should live in the castore
crate.
Also, some minor edits to //tvix/store/docs/api.md, to honor the move of
the castore bits to tvix-castore.
Change-Id: I1836556b652ac0592336eac95a8d0647599f4aec
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9893
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
We also switch the MissingField error to &'static str, since we only
parse a fixed set of fields.
Together, this makes the performance impact of error handling
negligible in batch happy-path parsing.
Change-Id: I2bd0ef2f5b35fcaced56b32d238eca75ac199ef1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9867
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: edef <edef@edef.eu>
Tested-by: BuildkiteCI
