From efbf4f48c62a9779e838ada8a4538983c7151e17 Mon Sep 17 00:00:00 2001
From: Jason Gedge <jason@gedge.ca>
Date: Sun, 29 Mar 2020 21:15:55 -0400
Subject: [PATCH] =?UTF-8?q?Fix=20poor=20message=20for=20executable=20that?=
 =?UTF-8?q?=20user=20doesn't=20have=20permissi=E2=80=A6=20(#1535)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 Cargo.lock                                     | 11 -----------
 crates/nu-cli/Cargo.toml                       |  1 -
 crates/nu-cli/src/cli.rs                       |  2 +-
 .../nu-cli/src/commands/classified/external.rs | 18 ++++++++++--------
 .../nu-cli/src/commands/classified/pipeline.rs |  4 ++--
 5 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 6e4a2fc494..54490f2678 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2304,7 +2304,6 @@ dependencies = [
  "umask",
  "unicode-xid",
  "users",
- "which",
 ]
 
 [[package]]
@@ -4288,16 +4287,6 @@ dependencies = [
  "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]]
 name = "widestring"
 version = "0.4.0"
diff --git a/crates/nu-cli/Cargo.toml b/crates/nu-cli/Cargo.toml
index 3b4baee3d4..f2211a2890 100644
--- a/crates/nu-cli/Cargo.toml
+++ b/crates/nu-cli/Cargo.toml
@@ -87,7 +87,6 @@ trash = "1.0.0"
 typetag = "0.1.4"
 umask = "0.1"
 unicode-xid = "0.2.0"
-which = "3.1.1"
 
 clipboard = { version = "0.5", optional = true }
 starship = { version = "0.38.0", optional = true }
diff --git a/crates/nu-cli/src/cli.rs b/crates/nu-cli/src/cli.rs
index fd639a7706..4dafc61a49 100644
--- a/crates/nu-cli/src/cli.rs
+++ b/crates/nu-cli/src/cli.rs
@@ -644,7 +644,7 @@ async fn process_line(
                 {
                     if dunce::canonicalize(name).is_ok()
                         && PathBuf::from(name).is_dir()
-                        && which::which(name).is_err()
+                        && ichwh::which(name).await.unwrap_or(None).is_none()
                         && args.list.is_empty()
                     {
                         // Here we work differently if we're in Windows because of the expected Windows behavior
diff --git a/crates/nu-cli/src/commands/classified/external.rs b/crates/nu-cli/src/commands/classified/external.rs
index 1e67975824..eef6b1b338 100644
--- a/crates/nu-cli/src/commands/classified/external.rs
+++ b/crates/nu-cli/src/commands/classified/external.rs
@@ -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,
     context: &mut Context,
     input: Option<InputStream>,
@@ -102,7 +102,7 @@ pub(crate) fn run_external_command(
 ) -> Result<Option<InputStream>, ShellError> {
     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(
             "Command not found",
             "command not found",
@@ -633,22 +633,22 @@ fn spawn(
         Ok(Some(stream.to_input_stream()))
     } else {
         Err(ShellError::labeled_error(
-            "Command not found",
-            "command not found",
+            "Failed to spawn process",
+            "failed to spawn",
             &command.name_tag,
         ))
     }
 }
 
-fn did_find_command(name: &str) -> bool {
+async fn did_find_command(name: &str) -> bool {
     #[cfg(not(windows))]
     {
-        which::which(name).is_ok()
+        ichwh::which(name).await.unwrap_or(None).is_some()
     }
 
     #[cfg(windows)]
     {
-        if which::which(name).is_ok() {
+        if ichwh::which(name).await.unwrap_or(None).is_some() {
             true
         } else {
             let cmd_builtins = [
@@ -738,7 +738,9 @@ mod tests {
 
         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(())
     }
diff --git a/crates/nu-cli/src/commands/classified/pipeline.rs b/crates/nu-cli/src/commands/classified/pipeline.rs
index d530671955..e81df7db62 100644
--- a/crates/nu-cli/src/commands/classified/pipeline.rs
+++ b/crates/nu-cli/src/commands/classified/pipeline.rs
@@ -35,11 +35,11 @@ pub(crate) async fn run_pipeline(
             }
 
             (Some(ClassifiedCommand::External(left)), None) => {
-                run_external_command(left, ctx, input, true)?
+                run_external_command(left, ctx, input, true).await?
             }
 
             (Some(ClassifiedCommand::External(left)), _) => {
-                run_external_command(left, ctx, input, false)?
+                run_external_command(left, ctx, input, false).await?
             }
 
             (None, _) => break,