Regression in `Command::exec`'s PATH resolution

[euank]

Using env_clear() + exec() currently exhibits different behaviour on nightly and stable.

I believe the first nightly to regress was 2018-11-02.

A minimal reproduction is the following code:

use std::process::Command;
use std::os::unix::process::CommandExt;

fn main() {
    let mut cmd = Command::new("true");
    cmd.env_clear();
    let err = cmd.exec();
    println!("error: {}", err);
}

On stable, it prints no error. On nightly, it prints error: No such file or directory (os error 2).

Using strace -e trace=execve, we can see the difference:

# stable
execve("/bin/true", ["true"], 0x7fedf1821010 /* 0 vars */) = 0
# nightly
execve("true", ["true"], 0x561a58edbb80 /* 0 vars */) = -1 ENOENT (No such file or directory)

Meta

$ rustc --version --verbose
rustc 1.32.0-nightly (15d770400 2018-11-06)
binary: rustc
commit-hash: 15d770400eed9018f18bddf83dd65cb7789280a5
commit-date: 2018-11-06
host: x86_64-unknown-linux-gnu
release: 1.32.0-nightly
LLVM version: 8.0

[phansch]

Probably due to #55359

[Mark-Simulacrum]

Cc @rust-lang/libs @alexcrichton

[alexcrichton]

Thanks for the report! I'm... not actually sure how this ever passed on stable, it wasn't really ever intended to. #55359 is almost for sure the cause of this, though

cc @alex too

[alex]

I'm not sure I understand what the desired behavior is. We look up $PATH in the child process's environment, that's the decided on behavior. If the child is modifying unrelated environment variables, $PATH is the same as the parent's, and if $PATH itself is modified we use that. Is the desired behavior here that if env is cleared then we should look at the parent's $PATH instead? And does this apply to env_remove as well?

I'm concerned that this behavior is awfully complicated, and not consistent with any platform APIs. That said, I'm happy to implement this if we can clearly articulate what the desired behavior is.

[Mark-Simulacrum]

I believe we've backported the relevant PR to beta as well so marking as regression from stable to beta

[alex]

Beta backport hasn't landed yet, #55611.

[alexcrichton]

@alex there's not really a "desired behavior" in this case, at this point it's just whack-a-mole of trying to rationalize with what we have so far.

The original implementation happened to have the side effect of using the child's PATH to do the binary lookup. This was then duplicated on Windows. That was probably a mistake. The ship has sailed, however, and now we need to make sure bugs are fixed and hopefully not break too much.