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
This is not an unclosed <html> tag.
Change-Id: I2bd2426fc600de2d96dbab47743f1c7bd5fed35e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10398
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: edef <edef@edef.eu>
self_reference being set to true is only allowed for
`CAHash::Nar(NixHash::Sha256(_))`, so we can handle this in a check at
the front.
Change-Id: Ic363ade4789a7767cbe26a6959b143bb53e50e5a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10391
Reviewed-by: edef <edef@edef.eu>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
All match cases essentially construct `ty` and `hash`, which is then
passed to the `build_store_path_from_fingerprint_parts` function.
Change-Id: I01dfd219f9b0ac1afe8af7c6e361ea048117a0e6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10390
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: edef <edef@edef.eu>
In most case, we don't actually need an owned `StorePath` struct, but a
`StorePathRef<'_>` is sufficient. The lifetime is only due to it holding
onto the name, but that one is mostly left untouched.
`Derivation::calculate_derivation_path` still needs to return
`StorePath`, as its name has a `.drv` appended.
Change-Id: Ie0d52f369d785711bb0658ea2b0bd2617fd9f45e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10389
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: edef <edef@edef.eu>
With the introduction of StorePathRef::to_absolute_path(), there's no
need to convert this StorePathRef to a StorePath first.
Change-Id: I634c977c4b63858e4f329fd21726e0611b99da4a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10388
Tested-by: BuildkiteCI
Reviewed-by: edef <edef@edef.eu>
Autosubmit: flokli <flokli@flokli.de>
Keep the method around in StorePath for convenience, but move the
implementation to StorePathRef.
Change-Id: Ie1844fa01ce6529dc1a58907563c95c3112c831d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10387
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: edef <edef@edef.eu>
Run criterion benchmarks (via `cargo bench`) for the *whole tvix
workspace*, not just `//tvix/eval`, and report the results to
windtunnel.
Change-Id: I9235c3c166ed9121f35c5bb4c46d59dc1f4c4055
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10382
Tested-by: BuildkiteCI
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: flokli <flokli@flokli.de>
These are still leftovers from before the traits being async, where we
had to clone before moving into an async closure.
Change-Id: I1b3937edf61ce3e23bb07803306622c37a3572c6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10381
Tested-by: BuildkiteCI
Reviewed-by: Connor Brewster <cbrewster@hey.com>
Autosubmit: flokli <flokli@flokli.de>
The simple filesystem `BlobService` enable a user to write blob store
on an existing filesystem using a prefix-style layout in the provided root directory,
e.g. the two first bytes of the blake3 hashes are used as directories prefixes.
Change-Id: I3451a688a6f39027b9c6517d853b95a87adb3a52
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10071
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
This is only used in the gRPC version (GRPCPutter), during the test
automation.
So define it as a method there, behind #[cfg(test)], and remove from
the trait.
Change-Id: Idf170884e3a10be0e96c75d946d9c431171e5e88
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10340
Tested-by: BuildkiteCI
Autosubmit: flokli <flokli@flokli.de>
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
There's nothing store-path specific here anymore, it's just a name in
the mountpoint root.
Change-Id: I0f8004491baa03ba560d390053a42678ee81154a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10377
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
The PR has since been replaced with another version, using a bit of
unsafe, as rustc doesn't like the lifetimes.
We're still waiting on a backport of that to 0.3.
Change-Id: I8dd344e78162cd956f6e53ff05dc4c02763fbb04
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10376
Reviewed-by: raitobezarius <tvl@lahfa.xyz>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
To support tvix builds, we need to be able to use the `TvixStoreFs` to
materialize the sandbox's /nix/store filesystem with just the list of
inputs needed for the build. Currently we'd need to seed an in-memory
`PathInfoService`, which includes more functionality than what is
required for `TvixStoreFs`. Additionally, the `PathInfoService` is
specific to Nix. By decoupling `TvixStoreFs` and `PathInfoService`,
we allow for usage of `TvixStoreFs` with `tvix-castore` without needing
a `PathInfoService`.
This introduces a new `RootNodes` trait which provides a way for the
filesystem to look up CA nodes via their basename in the root directory
of the filesystem. We then implement `RootNodes` for any
`PathInfoService`. Additionally, the filesystem root inode tracker now
stores basenames rather than `StorePath`s since `StorePath`s are
specific to Nix.
As a followup we can rename `TvixStoreFs` to `TvixCaStoreFs` and move
it to the `castore` crate (or its own crate).
b/341
Change-Id: I928372955017c23b1bf2b37190cbc508a4ed10d5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10363
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
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>
