bnjbvr/cargo-machete

Wrong arguments are passed to machete when the process is launched from within rust

singhblom opened this issue · 4 comments

When the machete process is launched from within another rust script, like it does in our ci tool, it seems to receive the wrong arguments. I made this little script that reproduces it.

use std::process::Command;
use std::process::Stdio;
use std::env;

type Besult = Result<(), String>;


fn run_cmd(program: &str, args: &[&str]) -> Besult {
    run_cmd_with_env(program, args, false, std::iter::empty()).map(|_| ())
}

fn run_cmd_with_env<'env>(
    program: &str,
    args: &[&str],
    capture_stdout: bool,
    env: impl Iterator<Item = (&'env str, &'env str)>,
) -> Result<Option<String>, String> {
    let mut cmd = Command::new(program);
    cmd.envs(env);
    cmd.args(args);

    cmd.stderr(Stdio::inherit());
    if capture_stdout {
        cmd.stdout(Stdio::piped());
    }

    let child = cmd.spawn().map_err(|e| format!("failed to spawn: {e}"))?;
    let output = child
        .wait_with_output()
        .map_err(|e| format!("failed to run: {e}"))?;

    if output.status.success() {
        if capture_stdout {
            Ok(Some(String::from_utf8_lossy(&output.stdout).to_string()))
        } else {
            Ok(None)
        }
    } else {
        Err(format!(
            "running {program} returned with status {}",
            output
                .status
                .code()
                .map_or_else(|| "unknown".into(), |x| x.to_string())
        ))
    }
}

fn cargo(args: &[&str]) -> Besult {
    run_cmd("cargo", args)
}


// Run as `cargo run -- machete` to see broken behavior, just do cargo run to see working.
fn main() {
    let args: Vec<String> = env::args().collect();
    let _ = if args.len() > 1 && args[1] == "cargo" {
        cargo(&["machete"])
    } else if args.len() > 1 && args[1] == "standalone" {
        run_cmd("cargo-machete", &[])
    } else {
        println!("Wrong argument, use standalone or cargo");
        Err("Wrong argument".to_string())
    };
}

When running I get

martin@puff:~/Code/broken-machete$ cargo run -- standalone
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `target/debug/broken-machete standalone`
Analyzing dependencies of crates in this directory...
cargo-machete didn't find any unused dependencies in /home/martin/Code/broken-machete. Good job!
Done!
martin@puff:~/Code/broken-machete$ cargo run -- cargo
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
     Running `target/debug/broken-machete cargo`
Analyzing dependencies of crates in machete...
error when walking over subdirectories: IO error for operation on machete: No such file or directory (os error 2)
cargo-machete didn't find any unused dependencies in machete. Good job!
Done!

Thanks for opening an issue. I can't reproduce this on Linux using either zsh or bash. Which OS and terminal are you using, out of curiosity, please?

bash on PopOs.

Ah, I actually manage to reproduce it today on Linux (yesterday I've tried on Mac). So it seems the code trying to figure out whether the command is being run independently (cargo-machete) or not (cargo machete) is incorrect, when another Rust program invokes cargo machete.

It's heuristics all the way... Currently we look at environment variables to determine whether we're running as a cargo subcommand or not:

  • if CARGO is defined, then something is being run as a cargo subcommand
  • that can be machete with cargo machete, but that can also be cargo run in the machete directory
    • in that case, we also look at other env vars that Cargo defines only in the case of cargo run in a Rust directory. I've picked CARGO_PKG_NAME at random; if it's set we're doing cargo run, but if it's not, we're doing cargo machete, likely.

If we're running under cargo, then we'll skip the first argument (which is machete); otherwise we won't. Things are good so far.

Now with the example program you've passed, the heuristic fails because CARGO is set (we're running the example program with cargo run), and CARGO_PKG_NAME is set too (to the name of the toy program). So since the environment variables are inherited by default, cargo-machete always thinks it's running in cargo run mode, thus not removing the first argument ("machete").

I think there's no proper way to fix this directly in cargo-machete: CARGO and CARGO_PKG_NAME are set in both modes of the toy program (and the latter is always set to the name of the toy program), so there's no way to distinguish one use case from the other. There might be other ways, but I think the simplest way is to:

  • either invoke machete without inheriting the environment variables (or at least, clear the two machete is using as heuristics)
  • or use the cargo-machete invoke

Let me know if you have any other ideas of ways to fix this in cargo-machete.