Skip to content

Commit

Permalink
Auto merge of #9991 - Byron:fix-test-failure-due-to-echo-resolution, …
Browse files Browse the repository at this point in the history
…r=joshtriplett

Don't canonicalize executable path

Otherwise symbolic links may also accidentally be resolved which may lead to unexpected results in the case of 'coreutils', a binary that depends on the executable name being a symbolic link.

This means a path like /bin/echo being canonicalized to /bin/coreutils will loose all information about the desired functionality.

For example, test failures will occur if 'echo' is resolved that way and it's not trivial to find the cause of it in the provided error messages.  For example`doc_workspace_open_different_library_and_package_names` did fail for me on MacOS, Nix packages in PATH, but works with this patch.

With this patch, there is still the possibility that a path gets canonicalized for its relative path components, but still results in changing the name of the binary. I could imagine to check for binary name changes and panic if `coreutils` or `busybox` is encountered, which are known to fail without a symlink telling them which program to emulate.
  • Loading branch information
bors committed Oct 23, 2021
2 parents 50a0af4 + cf8e464 commit e165bc8
Showing 1 changed file with 2 additions and 4 deletions.
6 changes: 2 additions & 4 deletions crates/cargo-util/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,13 @@ pub fn resolve_executable(exec: &Path) -> Result<PathBuf> {
});
for candidate in candidates {
if candidate.is_file() {
// PATH may have a component like "." in it, so we still need to
// canonicalize.
return Ok(candidate.canonicalize()?);
return Ok(candidate);
}
}

anyhow::bail!("no executable for `{}` found in PATH", exec.display())
} else {
Ok(exec.canonicalize()?)
Ok(exec.into())
}
}

Expand Down

0 comments on commit e165bc8

Please sign in to comment.