about summary refs log tree commit diff
path: root/tvix/eval
AgeCommit message (Collapse)AuthorFilesLines
2022-09-04 r/4632 test(tvix/eval): add tests for very simple closuresVincent Ambo4-0/+4
Change-Id: Ib8287ade4d5df6d29e1812fb2d349cee5d92ca6a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6296 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4629 feat(tvix/eval): implement runtime closure construction (OpClosure)Vincent Ambo1-1/+28
Implements the final bit of logic remaining for wiring up closures, which is the runtime construction of closure objects. When encountering an OpClosure, the VM walks through the bytecode collecting all the upvalue location operands (see commit introducing the OpCode::Data* variants for details) and stores the runtime values in the new closures upvalue vector. After that, the handling of the closure itself becomes functionally identical to that of lambdas. With this initial implementation of closures there are several large optimisation potentials available, the two most notable ones are: - Distinguish the runtime representation of lambdas and closures explicitly. - Detect and handle multiple-arity functions directly in the compiler. However, for both of these we should wait until we have appropriate benchmarking infrastructure in place. This is because our test implementations have shown that the complexity of either of these changes is quite significant, and we do not yet know if they really pay off. Change-Id: I077e977810fd5cb2b1ecd7f1a119e728025dd786 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6295 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4628 feat(tvix/eval): implement OpGetUpvalue in the VMVincent Ambo1-4/+6
This resolves an upvalue at runtime by pushing it on the stack from the closure's upvalue vector. Change-Id: Ic3e7a7ecd9f7032f679114a1995e5bbf83062fcf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6294 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4627 refactor(tvix/eval): store Closure inside of the VM's call framesVincent Ambo1-6/+6
In preparation for implementing calling of closures, store a closure directly in the VMs call frame. Change-Id: Iad24cd8c49fee4ebd4d0c84ffaa4c2505ee3dfd6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6293 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4626 feat(tvix/eval): add Value::to_closureVincent Ambo1-0/+11
... same as the others Change-Id: I9c8868388c10b0b6484c5bdd3799d801296c6979 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6292 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4625 feat(tvix/eval): compile creation of closure objectsVincent Ambo4-6/+51
Fully implements the instructions for compiling closure objects (without runtime handling yet). Closure (and thunk) objects are created at runtime by capturing all known upvalues. To represent this, the instructions for creating them need to have a variable number of arguments. Due to that, this commit introduces new variants in OpCode that are not actually operations, but data. If the VM is implemented correctly, the instruction pointer should never point at these. Due to this, the VM will panic if it sees a data operand during an execution run. Change-Id: Ic56b49b3a42736dc437751e76df0e89c8d0619c6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6291 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4624 chore(tvix/eval): print node representation for compiler errorsVincent Ambo1-1/+5
Better for development flow. Change-Id: I038efb39caca804f28a44fd4c83457e90abbcee4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6290 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4623 feat(tvix/eval): implement compilation of upvalue accessVincent Ambo4-4/+66
This adds a new upvalue tracking structure in the compiler to resolve upvalues and track their positions within a function when compiling a closure. The compiler will emit runtime upvalue access instructions after this commit, but the creation of the runtime closure object etc. is not yet wired up. Change-Id: Ib0c2c25f686bfd45f797c528753068858e3a770d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6289 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-03 r/4622 refactor(tvix/eval): add opcode::Count type for less ambiguityVincent Ambo3-17/+23
Change-Id: Ibde0b2baa1128a74c1364ee9a6330b62db3da699 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6288 Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4621 refactor(tvix/eval): add opcode::StackIdx type for less ambiguityVincent Ambo3-9/+14
Change-Id: I9b9de1f681972c205d4d20bc5731d2ce79858edb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6287 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4620 docs(tvix/eval): add docstrings for usize wrappers in opcodeVincent Ambo1-0/+3
Change-Id: I11b9324233c0aa48bd2fbac15a484962f925e72e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6283 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4619 refactor(tvix/eval): add opcode::JumpOffset type for less ambiguityVincent Ambo3-18/+27
This adds a transparent wrapper around `usize` used for jump offsets in the opcodes. This is a step towards getting rid of ambiguous plain `usize` usage in the opcode. Change-Id: I21e35e67d94b32d68251908b96c7f62b6f56a8bb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6282 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4618 refactor(tvix/eval): rename CompilationResult -> CompilationOutputVincent Ambo1-3/+3
grfn pointed out in cl/6174 that `Result` might cause developers to believe that this behaves like std::Result, which it does not. Change-Id: Ia30ab0dcb7e8da7bf842777ee3fe17bcf35cb0c1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6281 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4617 refactor(tvix/eval): explicitly discard uninteresting resultsVincent Ambo1-2/+2
Change-Id: I95a2ad61d9512b91017c801f325d0193b4da9c7d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6280 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4616 refactor(tvix/eval): avoid cloning in NixAttrs::update if possibleVincent Ambo2-23/+36
Refactors the update function to take the attribute sets by value instead. To facilitate this, we use an equivalent of the currently unstable `Rc::clone_or_unwrap` in the VM when encountering attribute sets, so that in cases where the only references to the attrs being updated are the ones on the stack those clones are avoided completely. This does make update() a little bit more tricky internally, as some optimised branches can directly return the moved value, and others need to destructure with ownership. For this reason there are now two different match statements handling the different ownership cases. Change-Id: Ia77d3ba5c86afb75b9f1f51758bda61729ba5aab Reviewed-on: https://cl.tvl.fyi/c/depot/+/6279 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-03 r/4615 test(tvix/eval): Add attr merge benchmarksGriffin Smith3-1/+19
Add a quick couple of benchmarks for merging attribute sets, large and small. Change-Id: I26940a9cf4e0d30e3d9eb07a7b8c366ca4072ca3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6286 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: grfn <grfn@gws.fyi>
2022-09-03 r/4614 refactor(tvix/eval): slightly more readable AttrsRep::selectVincent Ambo1-11/+5
Suggestion from grfn in cl/6158. Change-Id: I16dcf2296a5ec5d299d5a080ca099b8eda6c254e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6278 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4613 refactor(tvix/eval): rename Value::NotFound & OpAttrOrNotFoundVincent Ambo4-10/+10
grfn suggested clearer naming for these in cl/6166. Change-Id: I83164bf1d1902ebd42272e9d5d63819a0f6a72c5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6277 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4612 docs(tvix/eval): add doc comment on `compiler::patch_jump`Vincent Ambo1-0/+6
Change-Id: Ifdd7b99223d239d955ac7eeeae95db97eb742bf0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6276 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4611 refactor(tvix/eval): get rid of Value::Blackhole variantVincent Ambo2-5/+3
This is no longer needed for anything and the extra clone here is not really more costly than constructing a blackhole value in a different place. Change-Id: I5c63085b1b4418b629ea58a42e3bfe9a9b586d76 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6275 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4610 test(tvix/eval): add a test for float representationVincent Ambo2-0/+3
Change-Id: I4893a37719b9bf08b35963d48e6851a194a08aa7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6274 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4609 docs(tvix/eval): add a note on how to run Nix testsVincent Ambo1-1/+3
Change-Id: I9cd61ac79ed11b4c6580f31c5af5ebbfd45054b6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6273 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4608 fix(tvix/eval): address all current clippy lintsVincent Ambo6-13/+10
Change-Id: I758fc4f3b9078de7ca6228a75a4351c3e085c4cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6272 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4607 fix(tvix/eval): correctly escape `${` in stringsVincent Ambo3-12/+17
Without this escape, it is possible for Nix to produce escaped representations which are not literal Nix values again. This was fixed in upstream Nix in https://github.com/NixOS/nix/pull/4012 (though only for eval, not in the REPL) and the updated test is picked from upstream after that commit. Because we run the C++ Nix tests against our test suite as well, this also bumps our custom Nix 2.3 to a commit that includes the cherry-picked fix from the PR above. Change-Id: I478547ade65f655c606ec46f7143932064192283 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6271 Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-03 r/4606 refactor(tvix/eval): move resolve_local to Scope structVincent Ambo1-15/+18
This is a more sensible place for this function to live and makes upvalue resolution easier down the line. Change-Id: I48ee39bdcdb4f96a16a327f7015aff60db5b15fb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6270 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-03 r/4605 refactor(tvix/eval): introduce Closure struct in Value typeVincent Ambo5-10/+17
This struct will carry the upvalue machinery in addition to the lambda itself. For now, all lambdas are wrapped in closures (though technically analysis of the environment can later remove innermost Closure wrapper, but this optimisation may not be worth it). Change-Id: If2b68549ec1ea4ab838fdc47a2181c694ac937f2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6269 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-02 r/4604 chore(tvix/eval): Build with --all-targetsGriffin Smith1-0/+1
Primarily to make sure we build benchmark targets, and avoid breaking them Change-Id: I0c43f4cf99ddfd38e7545ef2d8276ef6b240a1e8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6285 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-02 r/4603 fix(tvix/eval): Fix build of benchmarksGriffin Smith1-1/+1
Interpret was updated to take an optional path arg in 6fe5e2d75 (feat(tvix/eval): resolve relative path literals, 2022-08-12), but since benchmarks aren't building in CI the resulting breakage of benchmarks was missed. Change-Id: I8a93f1b25ae62e2d032fafc153d91977c6466712 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6284 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-02 r/4602 chore(tvix/eval): move compiler module to a new folderVincent Ambo1-0/+0
Change-Id: I76157f9cf1369cd17506de1b1ded1a4fd06f004a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6268 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-02 r/4601 refactor(tvix/eval): avoid a use of Value::BlackholeVincent Ambo1-2/+2
The blackhole allocation is not going to be cheaper than cloning this. Change-Id: Id3ad44812decb4392830be06645e67bb0a982b96 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6267 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-02 r/4600 refactor(tvix/eval): separate out `let inherit ...` logicVincent Ambo1-9/+14
Compilation of `let`-expressions is going to become a lot more complicated due to attempts to avoid thunking when encountering internal references, so this is just being moved out of the way. Change-Id: Iecfa4b13d14532e21c2540e6561b4235ce29736a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6266 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-02 r/4599 chore(tvix/eval): print slightly more information about warningsVincent Ambo1-1/+2
This is just for dev comfort, it's not going to be useful for the final version. Change-Id: I05fdd590097a61085ed641810655d9ddaf8f3511 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6265 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-02 r/4598 fix(tvix/eval): consider `let ... inherit ...` in dynamic scopesVincent Ambo3-3/+40
In conditions where no dynamic identifiers exist in a scope, inheriting is usually a no-op - *unless* the identifier is not statically known and the scope has a non-empty `with`-stack. Change-Id: Iff4138d9cd4c56e844bc574203708dacc11c3f73 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6264 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-02 r/4597 refactor(tvix/eval): add NixAttrs::contains functionVincent Ambo2-1/+13
This avoids copying around the value more than needed. Change-Id: I35949d16dad7fb8f76e0f641eaccf48322144777 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6263 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4596 feat(tvix/eval): implement builtins.catAttrsVincent Ambo2-1/+18
Change-Id: Idf92ac82438fbfcf7b2f6e058830e4744637d8c6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6262 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4595 feat(tvix/eval): implement builtins.typeOfVincent Ambo1-0/+3
Change-Id: Ibc5039b444fadf6f9e5cd9132fcd825a871cee06 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6261 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4594 feat(tvix/eval): implement type-checking builtinsVincent Ambo1-0/+28
Change-Id: I70d7d837beaaed7e10cdc7577d96130f9e1b6d39 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6260 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4593 feat(tvix/eval): implement 'throw' and 'abort' builtinsVincent Ambo2-1/+18
These do essentially the same, but return different error variants as upstream Nix considers `throw` to be (sometimes) catchable. Change-Id: I1a9ea84567d46fb37287dbf3f3f67052f9382cca Reviewed-on: https://cl.tvl.fyi/c/depot/+/6259 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4592 refactor(tvix/eval): implement clearer mechanism for globalsVincent Ambo2-15/+60
The set of things that can leak out of `builtins` into the global scope is statically known (it is what Nix 2.3 leaks there, essentially). This is a mild change over the previous mechanism, where instead at the point where the `builtins` set is constructed we "lift" the globals out of there (if they exist). This way users will still eventually be able to add additional builtins, HOWEVER they will not be able to leak them into the global scope. Note that upstream Nix technically leaks _all_ builtins into the global scope using the `__*` prefix, but we are trying to avoid this in Tvix if it is not required in nixpkgs. Change-Id: Ie9dec2ce33740134f3b2464eba3749f421dd5953 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6258 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4591 feat(tvix/eval): emit warnings when globals are being shadowedVincent Ambo2-0/+2
Change-Id: I7dae6978c2a4548382d7fa059b20ccdf35d2cf7f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6257 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4590 feat(tvix/eval): add builtins.isNullVincent Ambo1-0/+5
Change-Id: Iae251d41b4ac6b77df56078a954ec3e33b7f9ccf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6256 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4589 test(tvix/eval): add a simple test for builtins resolutionVincent Ambo2-0/+7
Change-Id: I91f54778b8a17f3448664c21308de656b4b04b3e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6255 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4588 feat(tvix/eval): introduce mechanism for defining builtinsVincent Ambo5-6/+48
Adds a new builtins module in which builtins can be constructed. The functions in this module should return a correctly structured value to be passed to the compiler's `globals`. This is wired up all the way to the compiler with an example `toString` builtin, available as a global. Note that this does not yet actually behave like the real toString, which has some differences from `Display`. Change-Id: Ibb5f6fbe6207782fdf2434435567fc1bd80039a5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6254 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4587 refactor(tvix/eval): handle scope poisoning & globals dynamicallyVincent Ambo1-61/+125
Previously, the tokens that could poison a scope (`true`, `false`, `null`) had individual fields in the scope to track whether or not they were poisoned. This commit sets up new machinery that instead tracks scope poisoning dynamically using a HashMap, and which makes it possible to introduce additional tokens to the top-level ("global") scope that are directly resolved by the compiler by passing a map of runtime values to be used. With this solution, the compiler now contains all machinery required for wiring up builtins resolution. The set of builtins to be exposed at runtime must, however, be constructed *outside* of the compiler and passed in. Everything is prepared for this, but it is not yet wired up (so the only existing builtins are the ones we already had before). Note that this technically opens up an optimisation potential when compiling selection operations, where the attribute set being selected from is `builtins`. The compiler could directly resolve the builtins and place the right values on the stack. Change-Id: Ia7dad3c2a98703e7ea0c6ace1a722d57cc70a65c Reviewed-on: https://cl.tvl.fyi/c/depot/+/6253 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4586 docs(tvix/eval): add an overview of all builtins in NixVincent Ambo1-0/+120
Change-Id: Ie187f3317046c6c9e59852d4a128f25ceed99309 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6252 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-02 r/4585 feat(tvix/eval): add initial representation of builtinsVincent Ambo3-1/+76
Builtins are represented as a Rust function pointer that accepts a vector of arguments, which represents variable arity builtins. Change-Id: Ibab7e662a646caf1172695d876d2f55e187c03dd Reviewed-on: https://cl.tvl.fyi/c/depot/+/6251 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-09-02 r/4584 feat(tvix/eval): compile function applicationsVincent Ambo3-1/+14
Change-Id: I1b9230601895a1f09ef1a8037201147020b85f36 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6250 Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-02 r/4583 feat(tvix/eval): implement opcode for function calls in VMVincent Ambo3-8/+30
Nix functions always have a single argument and we do not yet make efforts to optimise this in Tvix for known multi-argument functions being directly applied. For this reason, the call instruction is fairly simple and just calls out to construct a new call frame. Note that the logic for terminating the run loop has moved to the top of the dispatch; this is because the loop run needs to be skipped if the call frame for the current lambda has just been dropped. Change-Id: I259bc07e19c1e55cd0a65207fa8105b23052b967 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6249 Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-09-02 r/4582 refactor(tvix/eval): add VM::call helper to set up call framesVincent Ambo1-7/+12
Change-Id: Ia7ff572af90ae379b23bbd0f5215cd13a4dc0ab5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6248 Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-01 r/4581 feat(tvix/eval): compile lambda definitionsVincent Ambo1-1/+40
Compiles lambda definitions of the simple form (i.e. without formals arguments) and emits them as constants like any other value. This does not yet implement actually invoking these functions in the VM. Change-Id: Ie1e0a13220b68c1728be229b875f0992e685c5ef Reviewed-on: https://cl.tvl.fyi/c/depot/+/6247 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>