Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
singhblom opened this issue Jan 16, 2023 · 4 comments

Comments

@singhblom
Copy link

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!
@bnjbvr
Copy link
Owner

bnjbvr commented Jan 16, 2023

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?

@singhblom
Copy link
Author

bash on PopOs.

@bnjbvr
Copy link
Owner

bnjbvr commented Jan 17, 2023

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.

@bnjbvr
Copy link
Owner

bnjbvr commented Jan 17, 2023

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.

@bnjbvr bnjbvr closed this as completed Jan 17, 2023
@bnjbvr bnjbvr closed this as not planned Won't fix, can't repro, duplicate, stale Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants