xtask-specific binaries in GRAPEVINE_XTASK_PATH

Some possible alternatives to this:

 - Keep putting them in PATH.
 - Make xtask a nix derivation. We would lose out on incremental
   compilation this way, and would end up recompiling xtask from scratch
   whenever something in the main package changed.
 - Have xtask call `nix build --inputs-from $toplevel nixpkgs#go` and
   such. Slow and tedious.

nix/shell.nix

@@ -6,6 +6,7 @@
 , go
 , inputs
 , jq
+, lib
 , lychee
 , markdownlint-cli
 , mdbook
@@ -19,6 +20,13 @@ mkShell {
     # sources, and it can read this environment variable to do so. The
     # `rust-src` component is required in order for this to work.
     RUST_SRC_PATH = "${toolchain}/lib/rustlib/src/rust/library";
+
+    # See the doc comment on `use_xtask_path` in `xtask/src/main.rs`.
+    GRAPEVINE_XTASK_PATH = lib.makeBinPath [
+      # Keep sorted
+      complement
+      go
+    ];
   };
 
   # Development tools
@@ -36,10 +44,6 @@ mkShell {
     markdownlint-cli
     mdbook
     toolchain
-
-    # TODO: don't pollute the devshell with these
-    go
-    complement
   ]
   ++
   default.nativeBuildInputs

xtask/src/complement.rs

@@ -58,9 +58,8 @@ pub(crate) struct Args {
 }
 
 #[allow(clippy::needless_pass_by_value)]
-pub(crate) fn main(args: Args) -> Result<()> {
-    let sh = Shell::new().unwrap();
-    let toplevel = get_toplevel_path(&sh)
+pub(crate) fn main(args: Args, sh: &Shell) -> Result<()> {
+    let toplevel = get_toplevel_path(sh)
         .wrap_err("failed to determine repository root directory")?;
     let baseline_path = args
         .baseline
@@ -74,12 +73,12 @@ pub(crate) fn main(args: Args) -> Result<()> {
     create_out_dir(&args.out).wrap_err_with(|| {
         format!("error initializing output directory {:?}", args.out)
     })?;
-    let docker_image = load_docker_image(&sh, &toplevel).wrap_err(
+    let docker_image = load_docker_image(sh, &toplevel).wrap_err(
         "failed to build and load complement-grapevine docker image",
     )?;
-    let test_count = count_complement_tests(&sh, &docker_image)
+    let test_count = count_complement_tests(sh, &docker_image)
         .wrap_err("failed to determine total complement test count")?;
-    let results = run_complement(&sh, &args.out, &docker_image, test_count)
+    let results = run_complement(sh, &args.out, &docker_image, test_count)
         .wrap_err("failed to run complement tests")?;
     let summary_path = args.out.join("summary.tsv");
     compare_summary(&baseline, &results, &baseline_path, &summary_path)?;

xtask/src/complement/test2json.rs

@@ -45,8 +45,6 @@ pub(crate) fn run_complement(
     test_count: u64,
 ) -> Result<TestResults> {
     // TODO: handle SIG{INT,TERM}
-    // TODO: XTASK_PATH variable, so that we don't need to pollute devshell with
-    // go
     let cmd = cmd!(sh, "go tool test2json complement.test -test.v=test2json")
         .env("COMPLEMENT_BASE_IMAGE", docker_image)
         .env("COMPLEMENT_SPAWN_HS_TIMEOUT", "5")

xtask/src/main.rs

@@ -1,8 +1,10 @@
 mod complement;
 
-use std::process::ExitCode;
+use std::{env, ffi::OsString, process::ExitCode};
 
 use clap::{Parser, Subcommand};
+use miette::{miette, IntoDiagnostic, Result, WrapErr};
+use xshell::Shell;
 
 #[derive(Parser)]
 struct Args {
@@ -16,11 +18,7 @@ enum Command {
 }
 
 fn main() -> ExitCode {
-    let args = Args::parse();
-    let result = match args.command {
-        Command::Complement(args) => complement::main(args),
-    };
-    let Err(e) = result else {
+    let Err(e) = try_main() else {
         return ExitCode::SUCCESS;
     };
     // Include a leading newline because sometimes an error will occur in
@@ -28,3 +26,49 @@ fn main() -> ExitCode {
     eprintln!("\n{e:?}");
     ExitCode::FAILURE
 }
+
+fn try_main() -> Result<()> {
+    let args = Args::parse();
+    let sh = new_shell()?;
+    match args.command {
+        Command::Complement(args) => complement::main(args, &sh),
+    }
+}
+
+fn new_shell() -> Result<Shell> {
+    let path = get_shell_path()?;
+    let sh = Shell::new()
+        .into_diagnostic()
+        .wrap_err("failed to initialize internal xshell::Shell wrapper")?;
+    sh.set_var("PATH", path);
+    Ok(sh)
+}
+
+/// Returns the value to set the `PATH` environment variable to in
+/// [`xshell::Shell`] instances.
+///
+/// This function appends the paths from the `GRAPEVINE_XTASK_PATH` environment
+/// variable to the existing value of `PATH` set in the xtask process.
+///
+/// Executable dependencies that are only called by commands in xtask should be
+/// added to `GRAPEVINE_XTASK_PATH` instead of `PATH` in the devshell, to avoid
+/// polluting the devshell path with extra entries.
+fn get_shell_path() -> Result<OsString> {
+    let xtask_path = env::var_os("GRAPEVINE_XTASK_PATH").ok_or(miette!(
+        help = "This tool must be run from inside the Grapevine devshell. \
+                Make sure you didn't interrupt direnv or something similar.",
+        "GRAPEVINE_XTASK_PATH environment variable is unset"
+    ))?;
+    if let Some(path) = env::var_os("PATH") {
+        let old_paths = env::split_paths(&path);
+        let xtask_paths = env::split_paths(&xtask_path);
+        env::join_paths(old_paths.chain(xtask_paths))
+            .into_diagnostic()
+            .wrap_err(
+                "error constructing new PATH value to include the paths from \
+                 GRAPEVINE_XTASK_PATH",
+            )
+    } else {
+        Ok(xtask_path)
+    }
+}