Commit graph

5 commits

Author SHA1 Message Date
Ben Webb
2daa483249 fix(tvix/cli): make -I prepend to lookup paths provided by NIX_PATH
Update the tvix cli's -I option so that it aligns more closely with
nix's behavior: prepending entries to the list of lookup paths provided
by the NIX_PATH environment variable. Before this commit, using the -I
option would instead override and ignore the NIX_PATH variable.
Additionally, update the option's long name and help text to match the
new behavior.

While the tvix cli's interface does not appear to be attempting to mimic
nix exactly, I think this particular case of the -I option's diverging
behavior will inevitably surprise users because it's name, presumably
short for "include" and being similar to gcc's flag, evokes additivity.
The prior implementation hinted at this difference with the help text
and the long name, --nix-search-path, but I still suspect users will be
confused on first usage (at least I was). If we're willing to pay the
maintenance costs of additional code, we can avoid this and provide a
slightly smoother user experience.

Changes were tested by buiding the tvix cli, adding it to the PATH, and
executing simple tests as in the following bash script

    mg build //tvix/cli
    PATH="$PWD/result/bin:$PATH"
    one=$(mktemp) && echo "=> $one :: path" > "$one"
    two=$(mktemp) && echo "=> $two :: path" > "$two"
    dir1=$(mktemp -d) && file1="$dir1/file1" && echo "=> $file1 :: path" > "$file1"
    dir2=$(mktemp -d) && file2="$dir2/file2" && echo "=> $file2 :: path" > "$file2"
    # NIX_PATH works with a single non-prefixed lookup path.
    NIX_PATH="$dir1" tvix -E "<file1>" | cmp - "$file1"
    # NIX_PATH works with multiple non-prefixed lookup paths.
    NIX_PATH="$dir1:$dir2" tvix -E "<file2>" | cmp - "$file2"
    # NIX_PATH works with a single prefixed lookup path.
    NIX_PATH="one=$one" tvix -E "<one>" | cmp - "$one"
    # NIX_PATH works with multiple prefixed lookup paths.
    NIX_PATH="one=$one:two=$two" tvix -E "<one>" | cmp - "$one"
    NIX_PATH="one=$one:two=$two" tvix -E "<two>" | cmp - "$two"
    # NIX_PATH first entry takes precedence.
    NIX_PATH="one=$one:one=$two" tvix -E "<one>" | cmp - "$one"
    # The -I option works with a single non-prefixed lookup path.
    tvix -I "$dir1" -E "<file1>" | cmp - "$file1"
    # The -I option works with multiple non-prefixed lookup paths.
    tvix -I "$dir1" -I "$dir2" -E "<file2>" | cmp - "$file2"
    # The -I option works with a single prefixed lookup path.
    tvix -I "one=$one" -E "<one>" | cmp - "$one"
    # The --extra-nix-path option works with a single prefixed lookup path.
    tvix --extra-nix-path "one=$one" -E "<one>" | cmp - "$one"
    # The -I options works when passed multiple times with prefixed lookup paths.
    tvix -I "one=$one" -I "two=$two" -E "<one>" | cmp - "$one"
    tvix -I "one=$one" -I "two=$two" -E "<two>" | cmp - "$two"
    # The first -I option takes precedence.
    tvix -I "one=$one" -I "one=$two" -E "<one>" | cmp - "$one"
    # Both NIX_PATH and the -I option work together and are additive.
    NIX_PATH="one=$one" tvix -I "two=$two" -E "<one>" | cmp - "$one"
    NIX_PATH="one=$one" tvix -I "two=$two" -E "<two>" | cmp - "$two"
    # The -I option takes precedence over NIX_PATH.
    NIX_PATH="one=$one" tvix -I "one=$two" -E "<one>" | cmp - "$two"
    rm "$one"
    rm "$two"
    rm "$file1" && rmdir "$dir1"
    rm "$file2" && rmdir "$dir2"

The above script assumes it's being run from inside the depot.

Change-Id: I153e6de57939c0eeca1f9e479d807862ab69b2de
Reviewed-on: https://cl.tvl.fyi/c/depot/+/13189
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2025-03-03 02:14:08 +00:00
Aspen Smith
f5c6acbbeb fix(tvix/cli): always configure nix path
Configure the nix path even if globals is already set.

Change-Id: I6598c92ab40ff952f73da04d9e7d3aeb13c16b53
Reviewed-on: https://cl.tvl.fyi/c/depot/+/12176
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
Reviewed-by: Ilan Joselevich <personal@ilanjoselevich.com>
2024-08-11 01:12:42 +00:00
Aspen Smith
8821746d6c fix(tvix/repl): Share globals and sourcemap across evaluations
Now that we can bind (potentially lazy, potentially lambda-containing)
values in the REPL and then reference them in subsequent evaluations,
it's important that the values to which we construct shared references
are shared across those subsequent evaluations - otherwise, we get
panics due to unknown source map locations, or dropped weak references
to globals.

This change assigns both the globals and the source map as fields on the
Repl after the first evaluation, and then passes those in (to the
EvaluationBuilder) on subsequent evaluations.

On the EvaluationBuilder side, there's some panicking introduced - this
is intentional, as my intent is for the builder to be configured
statically enough that panicking is the best way to report errors
here (it's always a bug to misconfigure an Evaluation, and we'd never
want to handle it dynamically).

Change-Id: I37225697235c22b683ca48a17d30fa8fedd12d1b
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11960
Reviewed-by: flokli <flokli@flokli.de>
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
2024-07-07 15:04:26 +00:00
Aspen Smith
01765c3717 test(tvix/cli): Add some additional REPL tests
A couple of already-passing tests covering REPL behavior

Change-Id: Ie21f4abf68ab12827fd15128a8ef810cd8592d07
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11959
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: aspen <root@gws.fyi>
2024-07-07 14:19:17 +00:00
Aspen Smith
0ad986169d test(tvix/cli): Make the REPL testable
Juggle around the internals of the tvix-cli crate so that we expose the
Repl as a public type with a `send` method, that sends a string to the
repl and *captures all output* so that it can be subsequently asserted
on in tests. Then, demonstrate that this works with a single (for now)
REPL test using expect-test to assert on the output of a single command
sent to the REPL.

As the REPL gets more complicated, this will allow us to make tests that
cover that complex behavior.

Change-Id: I88175bd72d8760c79faade95ebb1d956f08a7b83
Reviewed-on: https://cl.tvl.fyi/c/depot/+/11958
Autosubmit: aspen <root@gws.fyi>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>
2024-07-07 14:19:17 +00:00