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>
This commit is contained in:
Ben Webb 2025-03-02 14:17:40 -06:00 committed by benjaminedwardwebb
parent d76d699a53
commit 2daa483249
3 changed files with 59 additions and 5 deletions

View file

@ -45,9 +45,13 @@ pub struct Args {
#[clap(long)]
pub no_warnings: bool,
/// A colon-separated list of directories to use to resolve `<...>`-style paths
#[clap(long, short = 'I', env = "NIX_PATH")]
pub nix_search_path: Option<String>,
/// Additional entries to the Nix expression search path, a colon-separated list of directories
/// used to resolve `<...>`-style lookup paths.
///
/// This option may be given multiple times. Paths added through -I take precedence over
/// NIX_PATH.
#[clap(long = "extra-nix-path", short = 'I', env = "NIX_PATH", action = clap::ArgAction::Append)]
pub extra_nix_paths: Option<Vec<String>>,
/// Print "raw" (unquoted) output.
#[clap(long)]
@ -76,3 +80,53 @@ pub struct Args {
#[clap(long)]
pub drv_dumpdir: Option<PathBuf>,
}
impl Args {
pub fn nix_path(&self) -> Option<String> {
resolve_nix_path(std::env::var("NIX_PATH"), &self.extra_nix_paths)
}
}
fn resolve_nix_path(
nix_path: Result<String, std::env::VarError>,
extra_nix_paths: &Option<Vec<String>>,
) -> Option<String> {
let nix_path_option = nix_path.ok().filter(|string| !string.is_empty());
let extra_nix_paths_option = extra_nix_paths.to_owned().map(|vec| vec.join(":"));
match (nix_path_option, extra_nix_paths_option) {
(Some(nix_path), Some(mut extra_nix_paths)) => {
extra_nix_paths.push(':');
Some(extra_nix_paths + &nix_path)
}
(nix_path_option, extra_nix_paths_option) => nix_path_option.or(extra_nix_paths_option),
}
}
#[cfg(test)]
mod tests {
use super::resolve_nix_path;
#[test]
fn test_resolve_nix_path() {
let nix_path = Ok("/nixpath1:nixpath2=/nixpath2".to_owned());
let extra_nix_paths = Some(vec!["/extra1".to_owned(), "extra2=/extra2".to_owned()]);
let expected = Some("/extra1:extra2=/extra2:/nixpath1:nixpath2=/nixpath2".to_owned());
let actual = resolve_nix_path(nix_path, &extra_nix_paths);
assert!(actual == expected);
let nix_path = Err(std::env::VarError::NotPresent);
let extra_nix_paths = Some(vec!["/extra1".to_owned(), "extra2=/extra2".to_owned()]);
let expected = Some("/extra1:extra2=/extra2".to_owned());
let actual = resolve_nix_path(nix_path, &extra_nix_paths);
assert!(actual == expected);
let nix_path = Ok("/nixpath1:nixpath2=/nixpath2".to_owned());
let extra_nix_paths = None;
let expected = Some("/nixpath1:nixpath2=/nixpath2".to_owned());
let actual = resolve_nix_path(nix_path, &extra_nix_paths);
assert!(actual == expected);
let nix_path = Err(std::env::VarError::NotPresent);
let extra_nix_paths = None;
let expected = None;
let actual = resolve_nix_path(nix_path, &extra_nix_paths);
assert!(actual == expected);
}
}

View file

@ -118,7 +118,7 @@ pub fn evaluate(
eval_builder = add_import_builtins(eval_builder, Rc::clone(&tvix_store_io));
}
};
eval_builder = configure_nix_path(eval_builder, &args.nix_search_path);
eval_builder = configure_nix_path(eval_builder, &args.nix_path());
if let Some(source_map) = source_map {
eval_builder = eval_builder.with_source_map(source_map);

View file

@ -11,7 +11,7 @@ macro_rules! test_repl {
let tokio_runtime = tokio::runtime::Runtime::new().unwrap();
let args = tvix_cli::Args::parse_from(vec![
OsString::from("tvix"),
OsString::from("--nix-search-path"),
OsString::from("--extra-nix-path"),
OsString::from("nixpkgs=/tmp"),
]);
let mut repl = tvix_cli::Repl::new(init_io_handle(&tokio_runtime, &args), &args);