Remove a race condition which appears when uploadHashLayer is called
with the same key from multiple threads simultaneously. This can
easily happen when the same image path is requested by multiple
clients at the same time. When it does, a 500 status is returned and
the following error message is logged:
{
"context": {
"filePath": "github.com/google/nixery/builder/builder.go",
"lineNumber": 440,
"functionName": "github.com/google/nixery/builder.uploadHashLayer"
},
"error": "rename /var/lib/nixery/staging/<hash> /var/lib/nixery/layers/<hash>: no such file or directory",
"eventTime": "...",
"layer": "<hash>",
"message": "failed to move layer from staging",
...
}
To solve this issue, introduce a mutex keyed on the uploaded hash and
move all layer caching into uploadHashLayer. This could additionally
provide a small performance benefit when an already built image is
requested and NIXERY_PKGS_PATH is set, since symlink layers and config
layers are now also cached.
Change-Id: I50788a7ec7940cb5e5760f244692e361019a9bb7
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6695
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This is the New Thing that is intended to replace the find-owners
and owners plugins.
In particular:
* It inserts a submit requirement rather than providing a Prolog
predicate.
* The default OWNERS file formats are suspiciously Googley.
* It provides a neat UI for finding OWNERS and tracking approval
state on a per-file basis.
When we fully migrate to using the code-owners plugin, a few
things will need to land, which I will likely do "offline"
directly to the Gerrit backing Git repos:
* Add the corresponding Gerrit config
* Replace OWNERS files depot-wide
* Add OWNERS files to the refs/meta/config branch
* Introduce the Owners-Override label, settable by depot-interventions
The enclosed patch adds two extra pieces of functionality that
we need in tvldepot but aren't upstream:
1. The ability to just specify usernames rather than email addresses
2. The ability to specify `group:GROUPNAME`, _as long as_ that group is
visible to everyone. This is a restriction intended to avoid having
the plugin just leak group membership.
Change-Id: I27d92b6cb7449af83030b9015f09a1571aa8452f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6664
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Reviewed-by: sterni <sternenseemann@systemli.org>
And insert the missing newline the C++ Nix test script needs.
Change-Id: I04ddd7268f9caa1414fd23314c281bb7c1e854cf
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6689
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Instead of arity, we pass a array reference to Builtin::new that
describes how many arguments there are and which of them need to be
forced, eliminating the need to force manually.
Note that this change doesn't fix some of the instances where the the
Builtin doesn't consider that the value could be a Thunk.
Change-Id: Iadb58bb79886c30dc6b09dcf0ffad8abf28036a1
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6662
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
TL;DR:
- support `builtins.tail`
- define `ErrorKind::TailEmptyList` and canonical error code
- support basic unit tests
Unsure whether or not the error should be a dedicated `ErrorKind`...
Change-Id: Iae90fda1bb21ce7bdb1aaa2aeb2b8c1e6dcb0f05
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6545
Reviewed-by: wpcarro <wpcarro@gmail.com>
Autosubmit: wpcarro <wpcarro@gmail.com>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Refactor the `force!` macro to a method on `Value` which returns a
smart-pointer-esque type, which simplifies the callsite and eliminates
rightward drift, especially for high-arity builtins.
Change-Id: I97a7837580accfb4bbd03b24f2acdbd38645efa5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6656
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
Previously only the first one was guaranteed to be forced, but we need
to do this for all of them.
Fixes b/190
Change-Id: I76b5667dbfb2f3fde3587e7b91d268cbf32aca00
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6645
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: tazjin <tazjin@tvl.su>
Document the OpAttrs op, paying special attention to the (perhaps
confusing) behavior of taking the number of *pairs*, not the number
of *values*, which will be popped off the stack into the resulting attr
set.
Change-Id: I64df0290308ecae7a5c7e14ead37091d32701507
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6654
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: tazjin <tazjin@tvl.su>
Refactor the environment variable and argument parsing for the tvix repl
to use Clap instead of doing things ad-hoc, and thread through options
obtained from environment variables via explicit arguments rather than
obtaining them from the environment as they're needed. This makes adding
more flags more sustainable, and also makes the binary fully
self-documenting, including supported env vars, via `--help`.
Change-Id: Ib1f6a0cd20056e8c9196760ff755fa5729667760
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6653
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Thunks might be encountered deep in equality comparison (eg nested
inside a list or attr-set), at which point we need to force them in
order to compare them for equality (or else we panic when trying to get
at their value).
Fixes: b/192
Change-Id: I912151085f8298f30d5214c7965251c9266443f2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6652
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Pass in, but ignore, a mutable reference to the VM to the `nix_eq`
functions, in preparation for using that VM to force thunks during
comparison.
Change-Id: I565435d8dfb33768f930fdb5a6b0fb1365d7e161
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6651
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Using rust's PartialEq trait to implement Nix equality semantics is
reasonably fraught with peril, both because the actual laws are
different than what nix expects, and (more importantly) because certain
things actually require extra context to compare for equality (for
example, thunks need to be forced). This converts the manual PartialEq
impl for Value (and all its descendants) to a *derived* PartialEq
impl (which requires a lot of extra PartialEq derives on miscellanious
other types within the codebase), and converts the previous
nix-semantics equality comparison into a new `nix_eq` method. This
returns an EvalResult, even though it can't currently return an error,
to allow it to fail when eg forcing thunks (which it will do soon).
Since the PartialEq impls for Value and NixAttrs are now quite boring,
this converts the generated proptests for those into handwritten ones
that cover `nix_eq` instead
Change-Id: If3da7171f88c22eda5b7a60030d8b00c3b76f672
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6650
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This disconnects ownership of the `File` reference in a compiler from
the calling scope, which is required for when we implement `import`.
`import` will need to carry an `Rc<RefCell<CodeMap>>` (or maybe, in
the future, Arc) to give us the ability to add new detected code
files at runtime.
Note that the choice of `Arc` over `Rc` here is not ours - it's the
codemap crate's.
Change-Id: I3aeca4ffc167acbd1701846a332d93550b56ba7d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6630
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>
As before, this limits the cases to a relatively small number because
otherwise things get quite large.
Change-Id: I5371dc56418fca52e1dd1d905b20868f647091ba
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6649
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Only running 20 cases for now, since Value can get quite big if you let
it run for a while.
Change-Id: I09ef19da22c789c4869793836c98937c44595340
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6648
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
this was found by proptests!
Change-Id: I16d6a6ece3b20cdddd6f78c94cc87befb1b651e6
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6647
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Add an optional config argument to the `<trait>_laws` macros, to allow
configuring the generated tests with a ProptestConfig struct (to limit
the number of cases run)
Change-Id: I2143ddb72c6a870e8be4a9058135b6f9a703039e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6646
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
Impl Arbitrary for Value (and NixAttrs and NixList) in the same way we
did for NixString. Value currently only generates non-"internal"
values (no thunks, AttrNotFound, etc.) and can't generate
functions (builtins or closures), because those'd require full
generation of tvix bytecode, which is a bit more work than I'd like to
do now - there's a `todo!` left in the code for a place where we could
allow opting-in to internal values and functions later.
Change-Id: I07a59e2b1d89cfaa912d4ecebd642caf4ddb040a
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6627
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This is pretty boring at the moment, but mostly serves as a foot in the
door in the direction of writing more tests
Change-Id: Id88eb4ec7e53ebb2d5b5c254c8f45ff750238811
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6637
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This makes it possible to enter something into tvixbolt and then share
the link with someone else.
Suggested by Profpatsch originally.
Change-Id: I9886e76a7b821070f13ea7005df09188821e091d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6636
Tested-by: BuildkiteCI
Reviewed-by: Profpatsch <mail@profpatsch.de>
This will be used to set/get query parameters for making shareable links.
Change-Id: I05ccf8cab2521564710523ccd3b25ec26f435dd5
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6633
Tested-by: BuildkiteCI
Reviewed-by: Profpatsch <mail@profpatsch.de>
Invalid integers (eg integers that're too long) end up as error returns
on the `.value()` returned from the literal in the AST - previously we'd
unwrap this error, causing it to panic the compiler, but now we've got a
nice error variant for it (which just unwraps the underlying
std::num::ParseIntError).
Change-Id: I50c3c5ba89407d86659e20d8991b9658415f39a0
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6635
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This bumps rnix-parser to a commit that should be unaffected by the
Nix >= 2.4 bug that prevents it from cloning repositories with filters.
Change-Id: Ie01da95245ec6740fa889eb710819e512202f665
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6634
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Just want to note those down somewhere before I forget them again,
we can delete them later if we have it all figured out.
Change-Id: Icafa2d8fc7ca39e38e9637b7eca6f2bbf487c2b8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6632
Autosubmit: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
* //users/sterni/emacs: fix for the bqn-mode issue is in channels now.
* //third_party/buzz: pin openssl to 1.1, as nixpkgs now defaults to 3.0
Change-Id: I4b8410cbeb2d6ac210789b0b5687209d50e6572e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6628
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Add a suite of proptests covering the laws of the handwritten stdlib
trait impls (Eq, Ord, and Hash) for String, generated from a new set of
macros for generating those tests which can be applied to other types.
Change-Id: Ib3276c9e96fca497aece094e5612707d3dc77ccd
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6626
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
The From<String> impl for NixString only generates StringRepr::Heap
strings, but we want to make sure we're testing StringRepr::Smol too
Change-Id: I6d04b9cf12ef8462fe2788e0c6414b165f40311d
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6629
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
It's proptest time!
Add deps on the proptest and test_strategy crates, gated behind a
new (default-enabled) "arbitrary" feature flag so that they don't affect
dependencies of things like tvixbolt that depend on tvix.
These are going in dependencies, not dev-dependencies, so that we can
impl Arbitrary for stuff outside of test modules (which will be
important for integration suites which want to run proptests)
Change-Id: I1613bd3ea9a835e22986ad4e59700e8736007963
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6624
Autosubmit: grfn <grfn@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
This matches the name of the AST node from which it was compiled.
Suggested by sterni in cl/6231
Change-Id: Ia51525158d2f47467c01fce2282005b1a8417a47
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6623
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Reviewed-by: grfn <grfn@gws.fyi>
Add the start of a test suite that compares tvix eval results against
nix, using the string repr of the value as the comparison. This shells
out to a nix-instantiate binary, which is configurable as an environment
variable, to eval - there's some extra machinery there to setup a new
nix store as a tempdir to allow running this test inside the nix build
for tvix-eval itself.
Currently this has a macro that'll allow writing lots and lots of
hardcoded tests, but going forward I'm also going to be looking into
adding proptest-based generation of expressions to compare.
Change-Id: I9f4895fab1e668ed2b7dfd6f92f8c80de1bbb16b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6307
Autosubmit: grfn <grfn@gws.fyi>
Reviewed-by: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
... it would be nice if we could thread it through to the `Scope`
stuff (declaring phantoms & locals).
Change-Id: Id3b84e79032b8fbb12138b719e657565355fbc79
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6616
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
This trait can be used to convert most structures from rnix-parser
into a codemap::Span. It uses a macro to implement the trait for the
various expression types in the rnix AST, as Rust's silly semantic
versioning restriction stops us from doing a blanket implementation.
This will be used in the next commit to clean up the span handling in
the compiler a bit.
Change-Id: I0a437034e5fa203b5a49c6f25c45932a9f3b2bca
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6615
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
ical, but smol
to fit in qr
Change-Id: I37f99a20cfc96b85778a097b7c4f70923f026cd4
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6617
Autosubmit: Profpatsch <mail@profpatsch.de>
Tested-by: BuildkiteCI
Reviewed-by: Profpatsch <mail@profpatsch.de>
... and emit a warning if anyone decides to use.
Change-Id: Iaa6fe9fa932340e6d0fa9f357155e78823702576
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6611
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
Yep.
This is kind of ugly right now. The idea is that the recursive_scope
compilation function is used for recursive sets as well by emitting
the keys. The rest of the logic is pretty much identical.
There is quite a lot of code here that can be cleaned up (duplication
between attrs and let, duplication inside of the recursive scope
compilation function etc.), but I'd like to get it working first and
then make it nice.
Note that nested keys are *not* supported yet.
Change-Id: I45c7cdd5f0e1d35fd94797093904740af3a97134
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6610
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
This type is used in the list temporarily populated by the *second*
pass over all identifiers in a recursive scope. This first pass only
serves to make all bindings known to the compiler, without populating
their values yet.
Having a type here is going to be useful once we implement `rec`,
which needs to thread through slightly more information.
Change-Id: Ie33e0f096c5fcb6c864c991255466748b6f0d1eb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6609
Reviewed-by: sterni <sternenseemann@systemli.org>
Tested-by: BuildkiteCI
