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
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>
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
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
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
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
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
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
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
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
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
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
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
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>
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
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>
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
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
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
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
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
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
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
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
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
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
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
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>
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>
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>
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>
Mostly good, but I'm not sure it works with `git diff --patch`. Not a big deal
though if true.
Change-Id: I268c52dd253f5b0f9dd462a1825206da5dd86dd2
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6889
Reviewed-by: wpcarro <wpcarro@gmail.com>
Autosubmit: wpcarro <wpcarro@gmail.com>
Tested-by: BuildkiteCI
i always seem to get this wrong
Change-Id: Ib6f31523aba1d9f9a32d9af95b96b8d75e0ec16e
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6863
Reviewed-by: wpcarro <wpcarro@gmail.com>
Autosubmit: wpcarro <wpcarro@gmail.com>
Tested-by: BuildkiteCI
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
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
