Fix poor message for executable that user doesn't have permissi… (#1535)

Previously, if the user didn't have the appropriate permissions to execute the
binary/script, they would see "command not found", which is confusing.

This commit eliminates the `which` crate in favour of `ichwh`, which deals
better with permissions by not dealing with them at all! This is closer to the
behaviour of `which` in many shells. Permission checks are then left up to the
caller to deal with.
This commit is contained in:
Jason Gedge 2020-03-29 21:15:55 -04:00 committed by GitHub
parent 2ddab3e8ce
commit efbf4f48c6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 13 additions and 23 deletions

11
Cargo.lock generated
View File

@ -2304,7 +2304,6 @@ dependencies = [
"umask", "umask",
"unicode-xid", "unicode-xid",
"users", "users",
"which",
] ]
[[package]] [[package]]
@ -4288,16 +4287,6 @@ dependencies = [
"nom 4.2.3", "nom 4.2.3",
] ]
[[package]]
name = "which"
version = "3.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d011071ae14a2f6671d0b74080ae0cd8ebf3a6f8c9589a2cd45f23126fe29724"
dependencies = [
"failure",
"libc",
]
[[package]] [[package]]
name = "widestring" name = "widestring"
version = "0.4.0" version = "0.4.0"

View File

@ -87,7 +87,6 @@ trash = "1.0.0"
typetag = "0.1.4" typetag = "0.1.4"
umask = "0.1" umask = "0.1"
unicode-xid = "0.2.0" unicode-xid = "0.2.0"
which = "3.1.1"
clipboard = { version = "0.5", optional = true } clipboard = { version = "0.5", optional = true }
starship = { version = "0.38.0", optional = true } starship = { version = "0.38.0", optional = true }

View File

@ -644,7 +644,7 @@ async fn process_line(
{ {
if dunce::canonicalize(name).is_ok() if dunce::canonicalize(name).is_ok()
&& PathBuf::from(name).is_dir() && PathBuf::from(name).is_dir()
&& which::which(name).is_err() && ichwh::which(name).await.unwrap_or(None).is_none()
&& args.list.is_empty() && args.list.is_empty()
{ {
// Here we work differently if we're in Windows because of the expected Windows behavior // Here we work differently if we're in Windows because of the expected Windows behavior

View File

@ -94,7 +94,7 @@ pub fn nu_value_to_string(command: &ExternalCommand, from: &Value) -> Result<Str
} }
} }
pub(crate) fn run_external_command( pub(crate) async fn run_external_command(
command: ExternalCommand, command: ExternalCommand,
context: &mut Context, context: &mut Context,
input: Option<InputStream>, input: Option<InputStream>,
@ -102,7 +102,7 @@ pub(crate) fn run_external_command(
) -> Result<Option<InputStream>, ShellError> { ) -> Result<Option<InputStream>, ShellError> {
trace!(target: "nu::run::external", "-> {}", command.name); trace!(target: "nu::run::external", "-> {}", command.name);
if !did_find_command(&command.name) { if !did_find_command(&command.name).await {
return Err(ShellError::labeled_error( return Err(ShellError::labeled_error(
"Command not found", "Command not found",
"command not found", "command not found",
@ -633,22 +633,22 @@ fn spawn(
Ok(Some(stream.to_input_stream())) Ok(Some(stream.to_input_stream()))
} else { } else {
Err(ShellError::labeled_error( Err(ShellError::labeled_error(
"Command not found", "Failed to spawn process",
"command not found", "failed to spawn",
&command.name_tag, &command.name_tag,
)) ))
} }
} }
fn did_find_command(name: &str) -> bool { async fn did_find_command(name: &str) -> bool {
#[cfg(not(windows))] #[cfg(not(windows))]
{ {
which::which(name).is_ok() ichwh::which(name).await.unwrap_or(None).is_some()
} }
#[cfg(windows)] #[cfg(windows)]
{ {
if which::which(name).is_ok() { if ichwh::which(name).await.unwrap_or(None).is_some() {
true true
} else { } else {
let cmd_builtins = [ let cmd_builtins = [
@ -738,7 +738,9 @@ mod tests {
let mut ctx = Context::basic().expect("There was a problem creating a basic context."); let mut ctx = Context::basic().expect("There was a problem creating a basic context.");
assert!(run_external_command(cmd, &mut ctx, None, false).is_err()); assert!(run_external_command(cmd, &mut ctx, None, false)
.await
.is_err());
Ok(()) Ok(())
} }

View File

@ -35,11 +35,11 @@ pub(crate) async fn run_pipeline(
} }
(Some(ClassifiedCommand::External(left)), None) => { (Some(ClassifiedCommand::External(left)), None) => {
run_external_command(left, ctx, input, true)? run_external_command(left, ctx, input, true).await?
} }
(Some(ClassifiedCommand::External(left)), _) => { (Some(ClassifiedCommand::External(left)), _) => {
run_external_command(left, ctx, input, false)? run_external_command(left, ctx, input, false).await?
} }
(None, _) => break, (None, _) => break,