about summary refs log tree commit diff
path: root/tvix/eval/src/builtins
AgeCommit message (Collapse)AuthorFilesLines
2022-10-13 r/5125 feat(tvix/eval): implement builtins.baseNameOfAdam Joseph1-0/+5
This commit implements builtins.baseNameOf and adds a test case eval-okay-basenameof.nix to the test suite. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: Ib8bbafba2ac9ca0e1d3dc5e844167f94890d9fee Reviewed-on: https://cl.tvl.fyi/c/depot/+/6997 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-13 r/5124 fix(tvix/eval): parseDrvName should not coerce, and xfail testAdam Joseph1-2/+2
builtins.parseDrvName should not coerce its argument to a string. This commit fixes that oversight in my previous commit, and adds an xfail test to cover this condition. Thanks to @sterni for noticing this. Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I76bc78f1a82e1e08fe5c787c563a221d55de2639 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6991 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-10-12 r/5114 feat(tvix/eval): builtins: implement parseDrvNameAdam Joseph1-0/+25
This commit passes nix_eval_okay_src_tests_nix_tests_eval_okay_versions_nix. See also: https://github.com/NixOS/nix/pull/7149 Signed-off-by: Adam Joseph <adam@westernsemico.com> Change-Id: I24605c2a0cd0da434f37f6c518f20693bfa1b799 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6913 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-10-11 r/5105 fix(tvix/eval/builtins): force acc not list element in foldl'sterni1-1/+1
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/5102 feat(tvix/eval): Implement builtins.removeAttrsGriffin Smith1-1/+20
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-10 r/5097 fix(tvix/eval): Actually trace spans for thunksGriffin Smith1-1/+1
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 Smith1-1/+7
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/5089 feat(tvix/eval): Implement builtins.tryEvalGriffin Smith1-0/+15
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 Smith1-6/+7
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/5084 feat(tvix/eval): Implement builtins.concatStringsSepGriffin Smith1-0/+16
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 Carroll1-2/+39
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/5080 feat(tvix/eval): Implement builtins.elemGriffin Smith1-0/+8
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/5077 refactor(tvix/eval): Abstract away calling functionsGriffin Smith1-27/+7
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 Smith1-0/+18
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 Smith1-0/+11
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 Smith1-0/+13
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 Smith1-0/+17
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-08 r/5068 feat(tvix/tests): Import default.nix inside directoryGriffin Smith1-1/+4
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 Smith1-2/+1
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/5063 feat(tvix/eval): fancy-format parse errors returned by rnixVincent Ambo1-2/+3
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-07 r/5050 feat(tvix/eval): coerce values to paths when importingVincent Ambo1-12/+1
This enables the use of string paths (and, in the future, derivations), as long as their string values represent an absolute path. Change-Id: I4b198efeb70415ed52f58bd1da6fa79a24dad14c Reviewed-on: https://cl.tvl.fyi/c/depot/+/6866 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-10-07 r/5049 feat(tvix/eval): add method for emitting runtime warningsVincent Ambo1-4/+4
This lets the VM emit warnings when it encounters situations that should only be warned about at runtime. For starters, this is used to pass through compilation warnings that come up when `import` is used. Change-Id: I0c4bc8c534d699999887c430d93629fadfa662c4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6868 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-10-07 r/5048 feat(tvix/eval): insert `import` into the builtins itselfVincent Ambo1-2/+6
Adding `import` to builtins causes causes a bootstrap cycle because the `import` builtin needs to be initialised with the set of globals before being inserted into the globals, which also must contain itself. To break out of the cycle this hack wraps the builtins passed to the compiler in an `Rc` (probably sensible anyways, as they will end up getting cloned a bunch), containing a RefCell which gives us mutable access to the builtins. This opens up a potentially dangerous footgun in which we could mutate the builtins at runtime leading to different compiler invocations seeing different builtins, so it'd be nice to have some kind of "finalised" status for them or some such, but I'm not sure how to represent that atm. Change-Id: I25f8d4d2a7e8472d401c8ba2f4bbf9d86ab2abcb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6867 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-06 r/5041 feat(tvix/eval): initial implementation of `builtins.import`Vincent Ambo2-6/+80
This adds an initial working version of builtins.import which encapsulates the entire functionality of `import` within the builtin itself, without requiring any changes in the compiler or VM. The key insight that enables this is that we can simply return a Thunk from `import` that is constructed from the output of running the compiler and - ta-da! - no other component needs to know about it. A couple of notes: * builtins.import needs to capture variables like the SourceCode structure. This means it can not currently be constructed the same way as other builtins and has special handling, which leaks out to `eval.rs`. I have postponed dealing with that until we have this working a bit more. * the `globals` are not yet passed through * the error representation for the new variants is absolutely not done yet, we probably want to switch to something that supports cause-chaining now (like miette) * there is no mechanism for emitting warnings at runtime; we need to add that Change-Id: I3117a7ae3ff2432bf44f5ff05ad35f47faca31d5 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6857 Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: wpcarro <wpcarro@gmail.com> Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-06 r/5040 refactor(tvix/eval): builtins now contain closuresVincent Ambo1-86/+100
For some upcoming builtins (notably, import) we need to capture arguments in the builtin's implementation. To allow this, we can no longer use function pointers for builtins, but must use a reference-counted closure object instead. Unfortunately this adds an extra pointer operation to every builtin call. We should benchmark this later against having a split builtin representation. Change-Id: I109d98d0e25998870542f47573eb1ec2e546f2a2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6856 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi>
2022-10-05 r/5038 feat(tvix/eval): implement tvix's user-agent, err, nixVersionVincent Ambo1-0/+6
nixpkgs has hardcoded references to Nix versions, we need to provide it with something that looks like a Nix version while actually being a Tvix version. For now, we do this by stealing a trick out of the browser book and constructing a version that looks like a Nix version to Nix, but like a Tvix version to people who know what they are looking for. Nevermind that we don't actually have any kind of versioning for Tvix (yet?), other than depot revisions. Change-Id: I7ce8079dd8164a2079891d38e707f09a45f0bbc1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6858 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-10-04 r/5027 feat(tvix/eval): implement `builtins.currentTime`Vincent Ambo1-2/+12
Returns time since epoch in seconds. This has a slight behaviour difference from Nix, in that we don't pin the time between REPL entries (Nix pins it for the program lifetime), but this is probably inconsequential as long as it is pinned during an evaluation. Change-Id: I010c02e93097a209d8ad69e278397c7e30e54c86 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6846 Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: wpcarro <wpcarro@gmail.com>
2022-10-04 r/5026 refactor(tvix/eval): allow impure Value builtinsVincent Ambo2-10/+27
Allows impure builtins that have a different shape than a Rust function pointer; specifically this is required for builtins.currentTime which does not work in WASM. Change-Id: I1362d8eeafe770ce4d1c5ebe4d119aeb0abb5c9b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6849 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: wpcarro <wpcarro@gmail.com>
2022-10-03 r/5024 feat(tvix/eval): implement `builtins.any`Vincent Ambo1-0/+14
Change-Id: I640ee20e7c0a68c4e024a577e429fed9b3a49ece Reviewed-on: https://cl.tvl.fyi/c/depot/+/6845 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-10-03 r/5023 feat(tvix/eval): implement `builtins.all`Vincent Ambo1-0/+14
Change-Id: I19ec2b2194681efd73041f4aa1e5f2c893e839c2 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6844 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-10-03 r/5022 feat(tvix/eval): implement builtins.concatListsVincent Ambo1-0/+14
Concatenates (but not flattens) a list of lists. Change-Id: I692e0b3e7b5a5ff93d5768d3a27849b432ec5747 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6843 Autosubmit: tazjin <tazjin@tvl.su> Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-29 r/4991 chore(tvix/eval): fix all current clippy lintsVincent Ambo2-3/+3
Change-Id: I28d6af8cb408f8427a75d30b9120aaa809a1ea40 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6784 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-23 r/4964 chore(tvix/eval): mark coerce_value_to_path as intentionally unusedVincent Ambo1-0/+1
This was added in preparation for some builtins that don't exist yet and is producing some noise during compilation. Change-Id: I51fb0d14c3edf0bd6d9a288d50e44dacf35166c6 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6769 Reviewed-by: flokli <flokli@flokli.de> Autosubmit: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-23 r/4963 feat(tvix/eval): implement 'builtins.filter'Vincent Ambo1-0/+29
This is a little ugly because the plain Iterator::filter method can not be used (it does not support fallible primitives), so we need to resort to an `Iterator::filter_map` and deal with the wrapping in Options everywhere. This prevents use of `?` which introduces the need for some matching, but it's not *too* bad. Change-Id: Ie2c3c0c9756c4c627176f64fb4e0054e717c26d1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6765 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-22 r/4961 feat(tvix/eval): add builtins.stringLengthsterni1-0/+5
Fairly straightforward, only thing of note is that we coerce (weakly) to string here as well. Change-Id: I03b427e657e402f1f9eb0f795b689bbf5092aba1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6745 Tested-by: BuildkiteCI Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-22 r/4960 fix(tvix/eval): handle thunks in arithmetic builtinssterni1-20/+20
The simplest solution seems to be to pass references to arithmetic_op!() which avoids the moving annoyance we had to deal with in the builtins (no more popping!). We then use .force() to force the values and dereference any Thunks (which arithmetic_op! doesn't do for us). Change-Id: I0eb8ad60e80a0b3ba9d9f411e973ef8bcf136989 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6724 Tested-by: BuildkiteCI Reviewed-by: wpcarro <wpcarro@gmail.com> Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-22 r/4955 feat(tvix/eval): Support builtins.lessThanWilliam Carroll1-1/+6
Extend and export the `cmp_op`, and this becomes trivial. Change-Id: I9c93fa4db0f5a1fc8b56928ea144676f79247de1 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6557 Autosubmit: wpcarro <wpcarro@gmail.com> Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-22 r/4954 feat(tvix/eval): Sketch out impure builtinsGriffin Smith2-2/+19
Sketch out a new set of "impure" builtins, which supplement the existing set of "pure" builtins but are gated behind a feature flag, which allows them to be omitted by crates depending on tvix-eval that only want pure evaluation, such as tvixbolt. Change-Id: I2736017b5c9b4776bbba8758e108ec84887abd66 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6655 Reviewed-by: wpcarro <wpcarro@gmail.com> Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org> Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-21 r/4953 refactor(tvix/eval): drop empty Word version cmp rulesterni1-4/+0
This was derived from else if (c1 == "" && n2) return true; // true implies c1 < n2 However, this has no effect since Word always looses out against Number anyways and the `pre` rules are also unaffected by this change – since this only affects comparison of an empty Word part with a Number. Change-Id: Ia04e42ac726352b688c87674b0fdb355f06edbcb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6722 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-21 r/4950 fix(tvix/eval): compare versions with an extra empty componentsterni2-2/+17
This is necessary because builtins.compareVersions compares versions in a subtly not-quite-but-still-lexicographical way: `pre` for example can have an effect if it is post-fixed: `2.3 < 2.3pre`. This is a violation of the rule that in a lexicographical ordering, the longer string is considered greater if they are otherwise equal. builtins.compareVersion is comparing lexicographically though, if you do the following transformation beforehand: 2.3 --split--> [ "2" "3" ] --append--> [ "2" "3" "" ] 2.3pre --split--> [ "2" "3" "pre" ] --append--> [ "2" "3" "pre" "" ] Comparing the transformed version is then done lexicographically: 2.3 < 2.3.0pre since [ "2" "3" "" ] < [ "2" "3" "0" "pre" ] Here, the `pre` rule never comes into effect because no comparison on it happens, instead we use the longer string rule of a lexicographical comparison. In the C++ codebase, the reason for this behavior is that the iterator-esque construct they use always yields the empty string before it exposes it has been fully consumed. This is probably intentional to support the postfixed `pre` which is, for example, used by NixOS versions (e.g. unstable post 22.05 is 22.11-pre). We replicate this behavior using the `Chain` iterator in `VersionPartsIter::new_for_cmp`. Change-Id: I021c69aa27b0b7deb949dffe50ed18b6de3a7b1f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6720 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-21 r/4949 fix(tvix/eval): implement C++ Nix version part comparison algorithmsterni1-1/+37
This is based on the [relevant code] in C++ Nix. Our version has more branches because the C++ one only checks if it is less than or not, so can save handling a few cases. We on the other hand, can avoid calling the algorithm twice. It'd be nice to implement proptests for this in the future and to make sure that this weird little algorithm doesn't violate the Ord laws. [relevant code]: https://github.com/NixOS/nix/blob/cd35bbbeef72375873e396b9ffed14a4638693a8/src/libstore/names.cc#L81-L94 Change-Id: I46642e6da5eac7c0883cdce860622cdba04cd12b Reviewed-on: https://cl.tvl.fyi/c/depot/+/6719 Tested-by: BuildkiteCI Reviewed-by: tazjin <tazjin@tvl.su>
2022-09-21 r/4947 fix(tvix/eval/versions): preserve the Number string exactlysterni2-6/+5
This is relevant for builtins.splitVersion: nix-repl> builtins.splitVersion "unstable-2022-02-21" [ "unstable" "2022" "02" "21" ] Change-Id: I0a0add178d95d5a82e112b41ed5f3ca5a19608f8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6710 Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-21 r/4946 fix(tvix/eval/versions): dash is a divider, not underscoresterni1-1/+1
https: //github.com/NixOS/nix/blob/cd35bbbeef72375873e396b9ffed14a4638693a8/src/libstore/names.cc#L63 Change-Id: I1d5aba6060d11778f3b79089f4b27ef8849e4d37 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6709 Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-21 r/4945 feat(tvix/eval): implement builtins.splitVersionsterni1-1/+16
This was fairly easy, thanks to the work already done by Thomas Frank. However, the implementation is suboptimal because we parse number parts only to convert them back to strings afterwards. I've chosen to tackle this problem in the future, since having an (inefficient) implementation of splitVersion will be helpful for debugging the slight discrepancies between C++ Nix and Tvix in the case of compareVersions. Change-Id: Id6ed8eeb77663ff650c8c53ea952875b1fb7ea84 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6688 Autosubmit: sterni <sternenseemann@systemli.org> Reviewed-by: grfn <grfn@gws.fyi> Reviewed-by: tazjin <tazjin@tvl.su> Tested-by: BuildkiteCI
2022-09-20 r/4943 refactor(tvix/eval): add VM::call_value helper methodVincent Ambo1-4/+2
This makes it possible to call a callable value (builtin or closure/lambda) directly, without unwrapping it first. This is needed for pretty much all higher-order functions to work correctly. This is mostly equivalent to the previous code in coerce_to_string for calling `__toString`, except it expects the argument(s) to already be placed on the stack. Note that the span for the `NotCallable` error is not currently guaranteed to make any sense, will experiment with this. Change-Id: I821224368d438a28900858b343defc1817e46a0a Reviewed-on: https://cl.tvl.fyi/c/depot/+/6717 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-20 r/4941 feat(tvix/eval): implement builtins.mapVincent Ambo1-1/+17
As we already have a VM passed to the builtins, we can simply execute the provided closure/lambda in it for each value. The primary annoyance with this is that we have to clone the upvalues for each element, but we can try making this cheaper in the future (it's also a general problem in the VM itself). Change-Id: I5bcf56d58c509c0eb081e7cf52f6093216451ce4 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6714 Tested-by: BuildkiteCI Reviewed-by: sterni <sternenseemann@systemli.org>
2022-09-20 r/4936 fix(tvix/eval): make sure to deref thunk in type predicate builtinssterni1-18/+28
Previously we only matched the outer constructor after forcing which would mean that we would always return `false` if the inspected value was a thunk, regardless what value would be present inside. Change-Id: I361ea6e855e23ef8e5b59098a50b9cd59253803f Reviewed-on: https://cl.tvl.fyi/c/depot/+/6692 Reviewed-by: tazjin <tazjin@tvl.su> Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
2022-09-20 r/4933 fix(tvix/eval): make sure to force before selecting in catAttrssterni1-5/+6
Previously, this would almost always crash because list items are thunked more often nowadays and selecting from a thunk would fail. Also we no longer pop from args, accessing it by index should avoid an unnecessary clone here. Change-Id: I4410c4c2e28cc255a2c7cf2a5322db3d2c556a0e Reviewed-on: https://cl.tvl.fyi/c/depot/+/6693 Reviewed-by: grfn <grfn@gws.fyi> Autosubmit: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI