about summary refs log tree commit diff
AgeCommit message (Collapse)AuthorFilesLines
2022-10-11 r/5106 fix(nix/buildGo): fix for Nix >= 2.6 readFile changessterni1-1/+4
The string context retained by readFile would leak into attribute keys in fromJSON which may not have string context in any Nix version. We don't need Nix >= 2.6 support, but buildGo may have external users and this change is simple enough. Change-Id: I593f1ef513502691119428d26d508a5f4d378543 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6946 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-11 r/5105 fix(tvix/eval/builtins): force acc not list element in foldl'sterni4-1/+12
When investigating discrepancies between foldl' in tvix and C++ Nix, I discovered that C++ Nix's foldl' doesn't seem to be strict at all. Since this seemed wrong, I looked into Haskell's foldl' implementation which doesn't force the list elements (`val` in our code), but the accumulation value (`res` in our code). You can look at the code here: https://hackage.haskell.org/package/base-4.17.0.0/docs/src/GHC.List.html#foldl%27 This actually makes a lot of sense: If `res` is not forced after each application of `op`, we'll end up thunks nested as deeply as the list is long, potentially taking up a lot of space. This can be limited by forcing the `res` thunk before applying `op` again (and creating a new thunk). I've also PR-ed an equivalent change for C++ Nix at https://github.com/NixOS/nix/pull/7158. Since this is not merged nor backported to our Nix 2.3 fork, I've not copied the eval fail test yet, since it wouldn't when checking our tests against C++ Nix in depot. Change-Id: I34edf6fc3031fc1485c3e714f2280b4fba8f004b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6947 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-11 r/5104 fix(nix/buildkite): split extra steps based on all known phasesEvgeny Zemtsov1-1/+1
Fix a bug introduced by 2ca153141 (I merged the wrong patchset). Issue happens when pipeline is split by phases into independent evals (e.g. build/release/deploy). Splitting extra steps requires knowledge of all known phases, otherwise pipeline evaluation fails due to extra steps from inactive phases. Change-Id: Iab0f2dc3eadda281e483055e26f00a95442e15b0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6942 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-11 r/5103 feat(tvix/eval): Allow directly evaluating an expr via mainGriffin Smith1-0/+7
This *maybe* should do something to check that we don't pass both a file and an expr, but for now this is useful enough to cut corners (plus we're probably due for a CLI revamp eventually anyway). Change-Id: Id44357074150b336b6215ba596cc52d01d037dbd Reviewed-on: https://cl.tvl.fyi/c/depot/+/6941 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-11 r/5102 feat(tvix/eval): Implement builtins.removeAttrsGriffin Smith4-1/+31
Change-Id: I28910991a0108436a42ac7bf3458f9180a44154e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6928 Reviewed-by: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-11 r/5101 feat(tvix/eval): Implement builtins.traceGriffin Smith1-0/+12
This is currently implemented with a simple println inline, but in the future we could hook into this via something pluggable on the VM. Change-Id: Idd9cc3b34aa13d6ebc64c02aade81ecdf439656a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6938 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-11 r/5100 fix(tvix/eval): Pop frames even if running op failsGriffin Smith3-269/+285
Previously, the VM assumed that if an error was returned from `run()`, the evaluation was "finished" and the state of the VM didn't matter. This used to be a reasonable assumption, but now that we've got `tryEval` around we need to actually make sure that we clean up after ourselves if we're about to return an error. Specifically, if the *last* instruction in an evaluation frame returns an error, we previously wouldn't pop that evaluation frame, which could cause all sorts of bizarre errors based on what happened to be in the stack at the time. This commit splits out a `run_op` method from `VM::run`, and uses that to check the evaluation frame return condition even if the op we're running is about to return an error, and pop the evaluation frame if we're at the last instruction. Change-Id: Ib40649d8915ee1571153cb71e3d76492542fc3d7 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6940 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-11 r/5099 feat(tvix/eval): observe stack after exiting call frames/builtinsVincent Ambo2-8/+22
Change-Id: I1937d37551503a0b6bb0ac899d067302e4791e5f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6939 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-11 r/5098 fix(tvix/eval): implement functor calling for non-tail-callsVincent Ambo1-0/+15
Change-Id: I1113921c46010021b61c412d74d60193c156e0f1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6937 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5097 fix(tvix/eval): Actually trace spans for thunksGriffin Smith4-22/+34
Currently, the span on *all* thunk force errors is the span at which the thunk is forced, which for recursive thunk forcing ends up just being the same span over and over again. This changes the span on thunk force errors to be the span at which point the thunk is *created*, which is a bit more helpful (though the printing atm is a little... crowded). To make this work, we have to thread through the span at which a thunk is created into a field on the thunk itself. Change-Id: I81474810a763046e2eb3a8f07acf7d8ec708824a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6932 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: Adam Joseph <adam@westernsemico.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5096 feat(tvix/eval): Implement builtins.getEnvGriffin Smith1-1/+7
Change-Id: I09f512a60989a37184e73e521d4a3aa23f33a1a8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6922 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com> Reviewed-by: kanepyork <rikingcoding@gmail.com>
2022-10-10 r/5095 feat(tvix/eval): Implement builtins.readFileGriffin Smith4-1/+16
Change-Id: If3fd0b087009a2bfbad8bb7aca0aa20de906eb12 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6921 Tested-by: BuildkiteCI Reviewed-by: Adam Joseph <adam@westernsemico.com> Reviewed-by: kanepyork <rikingcoding@gmail.com> Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-10 r/5094 refactor(tvix/eval): after calling, the caller has to popVincent Ambo3-21/+29
Previously the various call functions either returned `EvalResult<()>` or `EvalResult<Value>`, which was confusing. Now only vm::call_with returns a Value directly, and other parts of the API just leave the stack top in the post-call state. This makes it easier to reason about what's going on in non-tail-call cases (which are making a comeback). Change-Id: I264ffc683a11aca72dd06e2220a5ff6e7c5fc2b0 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6936 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-10 r/5093 feat(wpcarro/scratch): Rewrite Python compiler in OCamlWilliam Carroll4-0/+154
Just to see how productive I could be in OCaml with little familiarity. Overall I really like it. Change-Id: I8affc65a5ee86a29d4f8c01426529ae9948660f9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6934 Reviewed-by: wpcarro <wpcarro@gmail.com> Autosubmit: wpcarro <wpcarro@gmail.com> Tested-by: BuildkiteCI
2022-10-10 r/5092 fix(tvix/eval): format nested compiler errors in fancy outputVincent Ambo1-10/+22
Instead of just printing the number of errors (useless!) actually emit separate diagnostics for each nested error. Change-Id: I97b53c3276c906af5def89077b5b6ba6ec108b37 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6933 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5091 fix(tvix/eval): path resolution errors are catchableGriffin Smith3-2/+3
Despite this not being documented, `tryEval` is empirically able to catch errors caused by a <...> path not resolving (and nixpkgs depends on this). Change-Id: Ia3b78a2d9d2d0c603aba829518b351102dc55396 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6926 Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-10 r/5090 fix(tvix/eval): Thunk `if` exprGriffin Smith3-1/+10
Since the body of an `if` expr can refer to deferred upvalues, it needs to be thunked so when we actually compile those deferred upvalues we have something for the finalize op to point at. Without this all sorts of weird things can happen due to the finalize op being run in the wrong lambda context, up to and including a panic. Change-Id: I040d5e1a7232fd841cfa4953539898fa49cbbb83 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6929 Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-10 r/5089 feat(tvix/eval): Implement builtins.tryEvalGriffin Smith4-0/+32
With asserts compiled using conditional jumps, this ends up being quite straightforward - the only real tricky bit is that we have to know whether an error can or can't be handled. Change-Id: I75617da73b7a9c5cdd888c0e26ae81d2c5c0d714 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6924 Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-10 r/5088 feat(tvix/eval): Implement builtins.pathExistsGriffin Smith3-6/+10
Change-Id: Ife8a690e9036868964771893ab29a9ae3a2d2365 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6919 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5087 feat(tvix/eval): Initial resolution of `<...>` pathsGriffin Smith9-21/+77
This commit implements (lazy) resolution of `<...>` paths via either the NIX_PATH environment variable, or the -I command-line flag - both handled via EvalOptions. As a result, EvalOptions can no longer derive Copy, meaning we have to clone it at each line of the repl - this is probably not a huge deal as repl performance is not exactly an inner loop and we're not cloning very much. Internally, this works by creating a thunk which pushes a constant containing the string inside the brackets to the stack, then a new opcode to resolve that path via the `NixPath`. To get that opcode to work, we now have to pass in the NixPath when constructing the VM. This (intentionally) leaves out proper implementation of path resolution via `findFile` (cppnix just calls whatever identifier called findFile is in scope!!!) as that's widely considered a bit of a misfeature, but if we do decide to implement that down the road it likely wouldn't be more than a few extra ops within the thunk introduced here. Change-Id: Ibc979b7e425b65cbe88599940520239a4a10cee2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6918 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5086 feat(tvix/eval): Add a struct implementing NIX_PATHGriffin Smith2-0/+208
Add a simple struct implementing both the string parsing and path resolution rules of Nix's `NIX_PATH` environment variable, for use in resolving `<...>`-style paths Change-Id: Ife75f39aa5c12928278d81fe428fbadc98bac5cc Reviewed-on: https://cl.tvl.fyi/c/depot/+/6917 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Reviewed-by: Adam Joseph <adam@westernsemico.com> Tested-by: BuildkiteCI
2022-10-10 r/5085 feat(tvix/eval): Allow adding strings to pathsGriffin Smith5-6/+40
Implement adding paths and strings via OpAdd. Since the nix rules are quite obscure, I'm electing to test this one with an oracle test to avoid the danger of getting the actual asserted result wrong. Change-Id: Icdcca3690ca2e8459e386c1f29cc48eaaa39e9a3 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6914 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5084 feat(tvix/eval): Implement builtins.concatStringsSepGriffin Smith5-0/+46
Change-Id: I6e46bcdbf3b5258b60edb017709fee577eb8ec74 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6907 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5083 feat(tvix/eval): Support builtins.readDirWilliam Carroll7-2/+73
Co-authored-by: Griffin Smith <root@gws.fyi> Change-Id: I5ff19efbe87d8f571f22ab0480500505afa624c5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6552 Autosubmit: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5082 refactor(tvix/eval): Compile OpAssert using conditional jumpsGriffin Smith3-10/+26
In order to behave nicely with tryEval, asserts need to leave the instruction pointer in a reasonable place even if they fail - whereas with the previous implementation catching a failed assert would still end up running the op for the *body* of the assert. With this change, we compile asserts much more like an `if` expression with conditional jumps rather than having an OpAssert op. Change-Id: I1b266c3be90185c84000da6b1995ac3e6fd5471b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6925 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5081 feat(wpcarro/scratch): Proof-of-concept register VMWilliam Carroll1-0/+161
I heard that register VMs might be slightly faster than stack VMs, and then it occurred to me that I wouldn't know how to write a register VM if I tried. So I wrote one (sort of). Change-Id: I15309bca88f4b43f6e04957acedc90d9adf16673 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6902 Reviewed-by: wpcarro <wpcarro@gmail.com> Autosubmit: wpcarro <wpcarro@gmail.com> Tested-by: BuildkiteCI
2022-10-10 r/5080 feat(tvix/eval): Implement builtins.elemGriffin Smith5-0/+82
Change-Id: Id99c1d33f87ad9866990d3483d3531e9e48f861f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6916 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5079 feat(nix/readTree): Handle a builtins w/o scopedImportGriffin Smith1-1/+1
We (might) not want to implement scopedImport in tvix given it's considered a bit of a misfeature; this makes readTree work with a `builtins` set that doesn't have it (and if we decide we do want tvix to have scopedImport, we can revert this pretty easily). Change-Id: Ia3bbc847514672063a607d977ce167d489fa1131 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6915 Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-10 r/5078 feat(nix/buildkite): allow custom phasesEvgeny Zemtsov1-17/+24
This change automatically extends the list of known phases as soon as they are added to active phase list. This is great when a user wants to design pipelines with multiple groups of dynamic steps. For example in Resoptima we want to design deployment pipeline where first only staging k8s namespaces are updated/tested and only after, we update production. Change-Id: Iab0f2dc3eadda281e483055e26f00a95442e15b9 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6923 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-10 r/5077 refactor(tvix/eval): Abstract away calling functionsGriffin Smith2-27/+32
The process of calling a function from a builtin, especially if it's got more than 1 arrgument, is reasonably involved and easy to get wrong due to having to interact directly with the stack - instead of having that done entirely manually in builtins, this wraps it up in a new `call_with` function which handles pushing arguments onto the stack and recursively calling the (partially applied) function. Change-Id: I14700c639a0deca53b9a060f6d70dbc7762e9007 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6910 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5076 feat(tvix/eval): Implement builtins.foldl'Griffin Smith3-0/+24
Change-Id: Ibc97db4343cb3a1a1677f69fb6c3518c61978aad Reviewed-on: https://cl.tvl.fyi/c/depot/+/6906 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-10 r/5075 feat(tvix/eval): Implement builtins.genListGriffin Smith4-0/+19
Change-Id: Iabe28656229f508226b244d81382e517961eb3cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6901 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-10 r/5074 feat(tvix/eval): Implement builtins.concatMapGriffin Smith3-0/+15
Change-Id: I08bfd040a242aa43b64760c19f48a28303f206ac Reviewed-on: https://cl.tvl.fyi/c/depot/+/6900 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-09 r/5073 feat(tvix/eval): Implement builtins.listToAttrsGriffin Smith3-0/+33
Implement the listToAttrs builtin, which constructs an attribute set from a list of attribute sets with keys name and value. This is tested using an adaptation of the nix `eval-ok-listtoattrs.nix`, with the utilities from `lib.nix` inlined. Change-Id: Ib5bf743466dda9722c2c1e00797df4b58448cf0f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6894 Autosubmit: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-09 r/5072 refactor(tvix/eval): Use Display impl for Error messageGriffin Smith1-153/+161
This is generally more idiomatic (over just delegating to Debug), and also allows us to avoid intermediate allocations if we ever end up using error messages as part of larger strings (because we don't have to allocate a full String for the return value). Change-Id: I67e48b44570c72761ed0fcaded9ae4bf3fcbaacf Reviewed-on: https://cl.tvl.fyi/c/depot/+/6896 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-09 r/5071 fix(nix/tag): correct no match check in discrsterni2-1/+6
It uses discrDef internally, but passes `null` as the default tag name, causing Nix to drop the attribute and return an empty attribute set if the default case is hit. Consequently we need to check for the empty attribute set, not `null` to figure out if there was no match found. We can also test this behavior using `assertThrows` which was introduced after the tag library was originally written. Change-Id: I45adb2f9602762dfc867956323fb3f5ae4c8bd1d Reviewed-on: https://cl.tvl.fyi/c/depot/+/6904 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: Profpatsch <mail@profpatsch.de> Tested-by: BuildkiteCI
2022-10-08 r/5070 feat(tvix/eval): Handle invoking binary with a directoryGriffin Smith1-9/+7
Similar to what we do for import, push on a `default.nix` to the path that the top-level is invoked with (if any) if it's a directory. Change-Id: I281bd44e3c8803b6765c886ae5fd08f549e2e563 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6895 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-10-08 r/5069 fix(tvix/eval): end scope after compiling legacy let bindingsVincent Ambo3-0/+7
When contrasting the compilation of the desugared version to the "sugared" version, this was the noticeable difference. This fixes b/203. Change-Id: Iae02ffc56e06de1de091b84cdc59d8fe83a17d69 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6898 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-08 r/5068 feat(tvix/tests): Import default.nix inside directoryGriffin Smith5-2/+11
This requires actually passing the source directory into `interpret` in the eval tests, but otherwise this is fairly straightforward - if we're trying to import a directory, just push `default.nix` onto it and import that instead. Change-Id: I0b7d4234f81977e78d14dfa651bf0cf9721017e5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6893 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-10-08 r/5067 refactor(tvix/eval): Encapsulate Value::Attrs constructionGriffin Smith4-6/+13
Factor out the construction of Value::Attrs (including the Rc) into a new `attrs` constructor function, to abstract away the presence of the Rc itself. Change-Id: I42fd4c3841e1db368db999ddd651277ff995f025 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6892 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-08 r/5066 fix(tvix/eval): Force thunks when comparing against ground valsGriffin Smith3-3/+13
Thunks correctly force when comparing for equality against other thunks, but weren't being forced correctly when comparing against non-thunk values, in either direction. Change-Id: Ia03702895ec4d70aed3445c1b0a9a7a641d1a300 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6897 Autosubmit: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-08 r/5065 feat(tvix/eval): add some slightly more descriptive span labelsVincent Ambo1-1/+36
Change-Id: I530c491f60a33fdb97e1553b193de51e7ee57d9a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6873 Reviewed-by: wpcarro <wpcarro@gmail.com> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-08 r/5064 feat(tvix/eval): chain error spans for thunk errorsVincent Ambo1-0/+21
Adds secondary spans for errors that occur deeply nested within a thunk. This is pretty raw right now, there's technically nothing stopping one of these error chains from being a hundred thunks deep into code, producing unmanageable error output. We should trim these down according to some heuristics (e.g. when crossing file boundaries, o r just - for starters - beginning and end). Change-Id: Ia73892512737850b6fa3e07cabc37fa9c534c4d5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6872 Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-08 r/5063 feat(tvix/eval): fancy-format parse errors returned by rnixVincent Ambo4-23/+267
This change is quite verbose, so a little bit of explaining: 1. To correctly format parse errors, errors must be able to return more than one annotated span (the parser returns a list of errors for each span). To accomplish this, the structure of how the `Diagnostic` struct which formats an error is constructed has changed to delegate the creation of the `SpanLabel` vector to the kind of error. 2. The rnix structures don't have human-readable output formats by default, so some verbose methods for formatting them in human-readable ways have been added in the errors module. We might want to move these out into a submodule. 3. In many cases, the errors returned by rnix are a bit strange - so while we format them with all information that is easily available they may look weird or not necessarily help users. Consider this CL only a first step in the right direction. Change-Id: Ie7dd74751af9e7ecb35d751f8b087aae5ae6e2e8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6871 Reviewed-by: sterni <sternenseemann@systemli.org> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-10-08 r/5062 refactor(nix/stateMonad): optimize aftersterni1-1/+1
This should save on one function application which can be a big deal for bigger for_ loops, I suspect. It's not really complicated, so why not. Change-Id: I2bfcd254e55f1bea366b09de294b2bef9f5b5dda Reviewed-on: https://cl.tvl.fyi/c/depot/+/6834 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-08 r/5061 feat(nix/dependency-analyzer): improved directDrvDeps for Nix >= 2.6sterni1-11/+25
This codepath will basically never be used in depot, but I want to add it as kind of a note to myself. It's kind of a neat feature, although I'm not quite sure it is going to stick around. Change-Id: If0e26ef47bdedc6dbf3d048ad4fc9a3a1fd6c5a2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6833 Reviewed-by: grfn <grfn@gws.fyi> Tested-by: BuildkiteCI
2022-10-08 r/5060 feat(nix/dependency-analyzer): find deps among a list of known drvssterni4-0/+307
This was written with the same intention (and reuses a little of its code) as cl/5060 and cl/5063: We want to be able to emit dependencies between //nix/buildkite pipeline steps, so that no agent is occupied with waiting on locks for derivations built by a different agent. This dependency information is already available to the Nix store implementation (e.g. via `nix-store --query --references`) and can also be obtained in the Nix language which is important, since the pipeline is generated at evaluation time. (Note: For Nix 2.3, you either need a strong convention about how derivations expose their dependencies (which we don't) or rely on store implementation internals (drv files). For Nix 2.6 there is a better trick, but it also relies on the existence of drv files.) The actual task can be formulated as follows: Given a set of derivations, calculate the the closest derivations also in the input each derivation depends on. (We call these (next) known dependencies.) This is crucial because pipeline step often depend on each other only indirectly with any number of intermediate derivations. For cl/5064 I determined that 6 intermediate layers is quite common for dependencies that are perceived to be “direct”. This problem is solved as follows: 1. Calculate the dependency graph of the combined dependency closure of all input derivations. This is quite easy and fairly quick thanks to the C++ implementation of builtins.genericClosure. One weak point of the current implementation is that the function to determine the direct derivation dependencies for Nix < 2.6 is quite hacky. 2. Take the graph from 1. and calculate a dependency graph that only connects the known derivations of the input, but retains all connections between them (minus intermediate nodes). In practice the dependency graph is represented as an attribute set mapping derivation paths to a list of derivation paths it depends on. The second step is performed by adding a second list of known derivation paths it depends on. The main improvements over the previous concept (cl/5060 and cl/5063): * We only try to find the closest known dependencies in the dependency graph whereas we would traverse emit dependencies for the entire dependency closure. * We immediately store the calculation of the closest known dependency in the dependency graph, even for intermediate nodes. This avoids recalculating the connection (which was a big drawback of the previous approach) and makes the calculation itself cheaper. You can run `mg build //nix/dependency-analyzer:example` to build a visualization of the internal dependencies between `depot.ci.targets` as discovered by dependency-analyzer. Change-Id: If8c0cdfc8470d4b337336257d9818aaa0d51110f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6832 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-08 r/5059 feat(ops/pipelines): allow accessing the nix storesterni1-1/+2
This is already allowed de facto, since there seems to be a special exception for reading from derivation outputs. What is forbidden, is access to files imported to the store (even via builtins.toFile) and derivation files. The latter is required for doing dependency analysis on arbitrary derivations, unfortunately. Access to the store allows kind of evil things, but it should be (hopefully) hard to do this by accident, and accessing derivation files is not impure, though it relies on store implementation internals so to speak. Change-Id: I33a7de83ef0ee20a7076690329d62f6caffffe5f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6835 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-08 r/5058 refactor(tvix/eval): implement ToSpan directly for rnix::TextRangeVincent Ambo1-10/+9
This logic was duplicated between the two rnix types before, but more crucially - it is also needed for correctly displaying the text ranges contained in syntax errors. Change-Id: Ifc6a521de1594d6ced9cba6274f1931df99b6634 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6870 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-08 r/5057 refactor(tvix/eval): move `spans` module to crate rootVincent Ambo3-11/+12
This is also useful for error-handling related logic, outside of just the compiler module. Change-Id: I5c386e2b4c31cda0a0209b31136ca07f00e39e45 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6869 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>