From 6fb24008599cb5388f601a5afc7496d70d9e8d8c Mon Sep 17 00:00:00 2001
From: 0xzhzh <178814591+0xzhzh@users.noreply.github.com>
Date: Thu, 24 Oct 2024 23:42:28 -0700
Subject: [PATCH 1/6] Add which function for finding executables in PATH

Closes #2109 (but with a function name that is shorter and more familiar)
---
 Cargo.toml          |  1 +
 README.md           | 16 ++++++++++++++++
 src/function.rs     | 11 +++++++++++
 tests/lib.rs        |  1 +
 tests/which_exec.rs | 42 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 71 insertions(+)
 create mode 100644 tests/which_exec.rs

diff --git a/Cargo.toml b/Cargo.toml
index f74367d02f..8a516d9935 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -51,6 +51,7 @@ tempfile = "3.0.0"
 typed-arena = "2.0.1"
 unicode-width = "0.2.0"
 uuid = { version = "1.0.0", features = ["v4"] }
+which = "6.0.0"
 
 [dev-dependencies]
 executable-path = "1.0.0"
diff --git a/README.md b/README.md
index 535b2bc402..82b94272ce 100644
--- a/README.md
+++ b/README.md
@@ -1597,6 +1597,22 @@ $ just
   name `key`, returning `default` if it is not present.
 - `env(key)`<sup>1.15.0</sup> — Alias for `env_var(key)`.
 - `env(key, default)`<sup>1.15.0</sup> — Alias for `env_var_or_default(key, default)`.
+- `which(exe)`<sup>1.37.0</sup> — Retrieves the full path of `exe` according
+  to the `PATH`. Returns an empty string if no executable named `exe` exists.
+
+```just
+bash := which("bash")
+nexist := which("does-not-exist")
+
+@test:
+    echo "bash: '{{bash}}'"
+    echo "nexist: '{{nexist}}'"
+```
+
+```console
+bash: '/bin/bash'
+nexist: ''
+```
 
 #### Invocation Information
 
diff --git a/src/function.rs b/src/function.rs
index a714a8d0fd..ff9c93b6a6 100644
--- a/src/function.rs
+++ b/src/function.rs
@@ -110,6 +110,7 @@ pub(crate) fn get(name: &str) -> Option<Function> {
     "uppercase" => Unary(uppercase),
     "uuid" => Nullary(uuid),
     "without_extension" => Unary(without_extension),
+    "which" => Unary(which_exec),
     _ => return None,
   };
   Some(function)
@@ -667,6 +668,16 @@ fn uuid(_context: Context) -> FunctionResult {
   Ok(uuid::Uuid::new_v4().to_string())
 }
 
+fn which_exec(_context: Context, s: &str) -> FunctionResult {
+  let path = which::which(s).unwrap_or_default();
+  path.to_str().map(str::to_string).ok_or_else(|| {
+    format!(
+      "unable to convert which executable path to string: {}",
+      path.display()
+    )
+  })
+}
+
 fn without_extension(_context: Context, path: &str) -> FunctionResult {
   let parent = Utf8Path::new(path)
     .parent()
diff --git a/tests/lib.rs b/tests/lib.rs
index ec71c665da..0552851f69 100644
--- a/tests/lib.rs
+++ b/tests/lib.rs
@@ -110,6 +110,7 @@ mod timestamps;
 mod undefined_variables;
 mod unexport;
 mod unstable;
+mod which_exec;
 #[cfg(windows)]
 mod windows;
 #[cfg(target_family = "windows")]
diff --git a/tests/which_exec.rs b/tests/which_exec.rs
new file mode 100644
index 0000000000..1b685e96a8
--- /dev/null
+++ b/tests/which_exec.rs
@@ -0,0 +1,42 @@
+use super::*;
+
+fn make_path() -> TempDir {
+  let tmp = temptree! {
+    "hello.exe": "#!/usr/bin/env bash\necho hello\n",
+  };
+
+  #[cfg(not(windows))]
+  {
+    let exe = tmp.path().join("hello.exe");
+    let perms = std::os::unix::fs::PermissionsExt::from_mode(0o755);
+    fs::set_permissions(exe, perms).unwrap();
+  }
+
+  tmp
+}
+
+#[test]
+fn finds_executable() {
+  let tmp = make_path();
+  let mut path = env::current_dir().unwrap();
+  path.push("bin");
+  Test::new()
+    .justfile(r#"p := which("hello.exe")"#)
+    .env("PATH", tmp.path().to_str().unwrap())
+    .args(["--evaluate", "p"])
+    .stdout(format!("{}", tmp.path().join("hello.exe").display()))
+    .run();
+}
+
+#[test]
+fn prints_empty_string_for_missing_executable() {
+  let tmp = make_path();
+  let mut path = env::current_dir().unwrap();
+  path.push("bin");
+  Test::new()
+    .justfile(r#"p := which("goodbye.exe")"#)
+    .env("PATH", tmp.path().to_str().unwrap())
+    .args(["--evaluate", "p"])
+    .stdout("")
+    .run();
+}

From f1980b5150f7720aac4139daa1cbeaa86cb0b616 Mon Sep 17 00:00:00 2001
From: 0xzhzh <178814591+0xzhzh@users.noreply.github.com>
Date: Sun, 3 Nov 2024 18:53:11 -0800
Subject: [PATCH 2/6] Change version of which function to master branch

---
 README.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.md b/README.md
index 82b94272ce..5b165b02e8 100644
--- a/README.md
+++ b/README.md
@@ -1597,7 +1597,7 @@ $ just
   name `key`, returning `default` if it is not present.
 - `env(key)`<sup>1.15.0</sup> — Alias for `env_var(key)`.
 - `env(key, default)`<sup>1.15.0</sup> — Alias for `env_var_or_default(key, default)`.
-- `which(exe)`<sup>1.37.0</sup> — Retrieves the full path of `exe` according
+- `which(exe)`<sup>master</sup> — Retrieves the full path of `exe` according
   to the `PATH`. Returns an empty string if no executable named `exe` exists.
 
 ```just

From 0c6f5e8376eb3c3ed2024637d2dd05fff8298638 Mon Sep 17 00:00:00 2001
From: 0xzhzh <178814591+0xzhzh@users.noreply.github.com>
Date: Sun, 3 Nov 2024 23:28:42 -0800
Subject: [PATCH 3/6] Use internal implementation of which()

---
 Cargo.lock      | 11 +++++++++
 Cargo.toml      |  3 ++-
 justfile        |  2 ++
 src/function.rs | 64 ++++++++++++++++++++++++++++++++++++++++++-------
 4 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 80a7276d9d..1ba1463bd6 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -498,6 +498,15 @@ dependencies = [
  "cc",
 ]
 
+[[package]]
+name = "is_executable"
+version = "1.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d4a1b5bad6f9072935961dfbf1cced2f3d129963d091b6f69f007fe04e758ae2"
+dependencies = [
+ "winapi",
+]
+
 [[package]]
 name = "is_terminal_polyfill"
 version = "1.70.1"
@@ -535,8 +544,10 @@ dependencies = [
  "dirs",
  "dotenvy",
  "edit-distance",
+ "either",
  "executable-path",
  "heck",
+ "is_executable",
  "lexiclean",
  "libc",
  "num_cpus",
diff --git a/Cargo.toml b/Cargo.toml
index 8a516d9935..bda81c5ec1 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -30,7 +30,9 @@ derivative = "2.0.0"
 dirs = "5.0.1"
 dotenvy = "0.15"
 edit-distance = "2.0.0"
+either = "1.13.0"
 heck = "0.5.0"
+is_executable = "1.0.4"
 lexiclean = "0.0.1"
 libc = "0.2.0"
 num_cpus = "1.15.0"
@@ -51,7 +53,6 @@ tempfile = "3.0.0"
 typed-arena = "2.0.1"
 unicode-width = "0.2.0"
 uuid = { version = "1.0.0", features = ["v4"] }
-which = "6.0.0"
 
 [dev-dependencies]
 executable-path = "1.0.0"
diff --git a/justfile b/justfile
index d0ff455c67..cf65778280 100755
--- a/justfile
+++ b/justfile
@@ -6,6 +6,8 @@ alias t := test
 
 log := "warn"
 
+bingus := which("bash")
+
 export JUST_LOG := log
 
 [group: 'dev']
diff --git a/src/function.rs b/src/function.rs
index ff9c93b6a6..25c328b9c5 100644
--- a/src/function.rs
+++ b/src/function.rs
@@ -1,5 +1,6 @@
 use {
   super::*,
+  either::Either,
   heck::{
     ToKebabCase, ToLowerCamelCase, ToShoutyKebabCase, ToShoutySnakeCase, ToSnakeCase, ToTitleCase,
     ToUpperCamelCase,
@@ -110,7 +111,7 @@ pub(crate) fn get(name: &str) -> Option<Function> {
     "uppercase" => Unary(uppercase),
     "uuid" => Nullary(uuid),
     "without_extension" => Unary(without_extension),
-    "which" => Unary(which_exec),
+    "which" => Unary(which),
     _ => return None,
   };
   Some(function)
@@ -668,14 +669,59 @@ fn uuid(_context: Context) -> FunctionResult {
   Ok(uuid::Uuid::new_v4().to_string())
 }
 
-fn which_exec(_context: Context, s: &str) -> FunctionResult {
-  let path = which::which(s).unwrap_or_default();
-  path.to_str().map(str::to_string).ok_or_else(|| {
-    format!(
-      "unable to convert which executable path to string: {}",
-      path.display()
-    )
-  })
+fn which(context: Context, s: &str) -> FunctionResult {
+  use is_executable::IsExecutable;
+
+  let cmd = PathBuf::from(s);
+
+  let path_var;
+  let candidates = match cmd.components().count() {
+    0 => Err("empty command string".to_string())?,
+    1 => {
+      // cmd is a regular command
+      path_var = env::var_os("PATH").ok_or("Environment variable `PATH` is not set")?;
+      Either::Left(env::split_paths(&path_var).map(|path| path.join(cmd.clone())))
+    }
+    _ => {
+      // cmd contains a path separator, treat it as a path
+      Either::Right(iter::once(cmd))
+    }
+  };
+
+  for mut candidate in candidates.into_iter() {
+    if candidate.is_relative() {
+      // This candidate is a relative path, either because the user invoked `which("./rel/path")`,
+      // or because there was a relative path in `PATH`. Resolve it to an absolute path.
+      let cwd = context
+        .evaluator
+        .context
+        .search
+        .justfile
+        .parent()
+        .ok_or_else(|| {
+          format!(
+            "Could not resolve absolute path from `{}` relative to the justfile directory. Justfile `{}` had no parent.",
+            candidate.display(),
+            context.evaluator.context.search.justfile.display()
+          )
+        })?;
+      let mut cwd = PathBuf::from(cwd);
+      cwd.push(candidate);
+      candidate = cwd;
+    }
+
+    if candidate.is_executable() {
+      return candidate.to_str().map(str::to_string).ok_or_else(|| {
+        format!(
+          "Executable path is not valid unicode: {}",
+          candidate.display()
+        )
+      });
+    }
+  }
+
+  // No viable candidates; return an empty string
+  Ok(String::new())
 }
 
 fn without_extension(_context: Context, path: &str) -> FunctionResult {

From 389b2ae4af5910ff2fceefedab8ee668db4115f1 Mon Sep 17 00:00:00 2001
From: 0xzhzh <178814591+0xzhzh@users.noreply.github.com>
Date: Sun, 1 Dec 2024 17:47:48 -0500
Subject: [PATCH 4/6] Add tests for internal implementation of which()

---
 tests/which_exec.rs | 162 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 145 insertions(+), 17 deletions(-)

diff --git a/tests/which_exec.rs b/tests/which_exec.rs
index 1b685e96a8..edc09d723d 100644
--- a/tests/which_exec.rs
+++ b/tests/which_exec.rs
@@ -1,25 +1,46 @@
 use super::*;
 
-fn make_path() -> TempDir {
-  let tmp = temptree! {
-    "hello.exe": "#!/usr/bin/env bash\necho hello\n",
-  };
+trait TempDirExt {
+  fn executable(self, file: impl AsRef<Path>) -> Self;
+}
 
-  #[cfg(not(windows))]
-  {
-    let exe = tmp.path().join("hello.exe");
-    let perms = std::os::unix::fs::PermissionsExt::from_mode(0o755);
-    fs::set_permissions(exe, perms).unwrap();
-  }
+impl TempDirExt for TempDir {
+  fn executable(self, file: impl AsRef<Path>) -> Self {
+    let file = self.path().join(file.as_ref());
+
+    // Make sure it exists first, as a sanity check.
+    assert!(
+      file.exists(),
+      "executable file does not exist: {}",
+      file.display()
+    );
+
+    // Windows uses file extensions to determine whether a file is executable.
+    // Other systems don't care. To keep these tests cross-platform, just make
+    // sure all executables end with ".exe" suffix.
+    assert!(
+      file.extension() == Some("exe".as_ref()),
+      "executable file does not end with .exe: {}",
+      file.display()
+    );
+
+    #[cfg(not(windows))]
+    {
+      let perms = std::os::unix::fs::PermissionsExt::from_mode(0o755);
+      fs::set_permissions(file, perms).unwrap();
+    }
 
-  tmp
+    self
+  }
 }
 
 #[test]
 fn finds_executable() {
-  let tmp = make_path();
-  let mut path = env::current_dir().unwrap();
-  path.push("bin");
+  let tmp = temptree! {
+    "hello.exe": "#!/usr/bin/env bash\necho hello\n",
+  }
+  .executable("hello.exe");
+
   Test::new()
     .justfile(r#"p := which("hello.exe")"#)
     .env("PATH", tmp.path().to_str().unwrap())
@@ -30,9 +51,11 @@ fn finds_executable() {
 
 #[test]
 fn prints_empty_string_for_missing_executable() {
-  let tmp = make_path();
-  let mut path = env::current_dir().unwrap();
-  path.push("bin");
+  let tmp = temptree! {
+    "hello.exe": "#!/usr/bin/env bash\necho hello\n",
+  }
+  .executable("hello.exe");
+
   Test::new()
     .justfile(r#"p := which("goodbye.exe")"#)
     .env("PATH", tmp.path().to_str().unwrap())
@@ -40,3 +63,108 @@ fn prints_empty_string_for_missing_executable() {
     .stdout("")
     .run();
 }
+
+#[test]
+fn skips_non_executable_files() {
+  let tmp = temptree! {
+    "hello.exe": "#!/usr/bin/env bash\necho hello\n",
+    "hi": "just some regular file",
+  }
+  .executable("hello.exe");
+
+  Test::new()
+    .justfile(r#"p := which("hi")"#)
+    .env("PATH", tmp.path().to_str().unwrap())
+    .args(["--evaluate", "p"])
+    .stdout("")
+    .run();
+}
+
+#[test]
+fn supports_multiple_paths() {
+  let tmp1 = temptree! {
+    "hello1.exe": "#!/usr/bin/env bash\necho hello\n",
+  }
+  .executable("hello1.exe");
+
+  let tmp2 = temptree! {
+    "hello2.exe": "#!/usr/bin/env bash\necho hello\n",
+  }
+  .executable("hello2.exe");
+
+  let path =
+    env::join_paths([tmp1.path().to_str().unwrap(), tmp2.path().to_str().unwrap()]).unwrap();
+
+  Test::new()
+    .justfile(r#"p := which("hello1.exe")"#)
+    .env("PATH", path.to_str().unwrap())
+    .args(["--evaluate", "p"])
+    .stdout(format!("{}", tmp1.path().join("hello1.exe").display()))
+    .run();
+
+  Test::new()
+    .justfile(r#"p := which("hello2.exe")"#)
+    .env("PATH", path.to_str().unwrap())
+    .args(["--evaluate", "p"])
+    .stdout(format!("{}", tmp2.path().join("hello2.exe").display()))
+    .run();
+}
+
+#[test]
+fn supports_shadowed_executables() {
+  let tmp1 = temptree! {
+    "shadowed.exe": "#!/usr/bin/env bash\necho hello\n",
+  }
+  .executable("shadowed.exe");
+
+  let tmp2 = temptree! {
+    "shadowed.exe": "#!/usr/bin/env bash\necho hello\n",
+  }
+  .executable("shadowed.exe");
+
+  // which should never resolve to this directory, no matter where or how many
+  // times it appears in PATH, because the "shadowed" file is not executable.
+  let dummy = if cfg!(windows) {
+    temptree! {
+      "shadowed": "#!/usr/bin/env bash\necho hello\n",
+    }
+  } else {
+    temptree! {
+      "shadowed.exe": "#!/usr/bin/env bash\necho hello\n",
+    }
+  };
+
+  // This PATH should give priority to tmp1/shadowed.exe
+  let tmp1_path = env::join_paths([
+    dummy.path().to_str().unwrap(),
+    tmp1.path().to_str().unwrap(),
+    dummy.path().to_str().unwrap(),
+    tmp2.path().to_str().unwrap(),
+    dummy.path().to_str().unwrap(),
+  ])
+  .unwrap();
+
+  // This PATH should give priority to tmp2/shadowed.exe
+  let tmp2_path = env::join_paths([
+    dummy.path().to_str().unwrap(),
+    tmp2.path().to_str().unwrap(),
+    dummy.path().to_str().unwrap(),
+    tmp1.path().to_str().unwrap(),
+    dummy.path().to_str().unwrap(),
+  ])
+  .unwrap();
+
+  Test::new()
+    .justfile(r#"p := which("shadowed.exe")"#)
+    .env("PATH", tmp1_path.to_str().unwrap())
+    .args(["--evaluate", "p"])
+    .stdout(format!("{}", tmp1.path().join("shadowed.exe").display()))
+    .run();
+
+  Test::new()
+    .justfile(r#"p := which("shadowed.exe")"#)
+    .env("PATH", tmp2_path.to_str().unwrap())
+    .args(["--evaluate", "p"])
+    .stdout(format!("{}", tmp2.path().join("shadowed.exe").display()))
+    .run();
+}

From 2a535c0fbe70712c4dcdd52e27a6aa5966656074 Mon Sep 17 00:00:00 2001
From: 0xzhzh <178814591+0xzhzh@users.noreply.github.com>
Date: Sun, 1 Dec 2024 18:00:27 -0500
Subject: [PATCH 5/6] Remove stray addition to justfile

---
 justfile | 2 --
 1 file changed, 2 deletions(-)

diff --git a/justfile b/justfile
index cf65778280..d0ff455c67 100755
--- a/justfile
+++ b/justfile
@@ -6,8 +6,6 @@ alias t := test
 
 log := "warn"
 
-bingus := which("bash")
-
 export JUST_LOG := log
 
 [group: 'dev']

From 34f2ea607d4758349844a8e8635e4f90e97cd171 Mon Sep 17 00:00:00 2001
From: 0xzhzh <178814591+0xzhzh@users.noreply.github.com>
Date: Fri, 13 Dec 2024 15:14:21 -0800
Subject: [PATCH 6/6] Remove dependency on either

---
 Cargo.lock      | 1 -
 Cargo.toml      | 1 -
 src/function.rs | 7 +++----
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 1ba1463bd6..8958927e23 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -544,7 +544,6 @@ dependencies = [
  "dirs",
  "dotenvy",
  "edit-distance",
- "either",
  "executable-path",
  "heck",
  "is_executable",
diff --git a/Cargo.toml b/Cargo.toml
index bda81c5ec1..8c5be83add 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -30,7 +30,6 @@ derivative = "2.0.0"
 dirs = "5.0.1"
 dotenvy = "0.15"
 edit-distance = "2.0.0"
-either = "1.13.0"
 heck = "0.5.0"
 is_executable = "1.0.4"
 lexiclean = "0.0.1"
diff --git a/src/function.rs b/src/function.rs
index 25c328b9c5..795a5297fd 100644
--- a/src/function.rs
+++ b/src/function.rs
@@ -1,6 +1,5 @@
 use {
   super::*,
-  either::Either,
   heck::{
     ToKebabCase, ToLowerCamelCase, ToShoutyKebabCase, ToShoutySnakeCase, ToSnakeCase, ToTitleCase,
     ToUpperCamelCase,
@@ -680,15 +679,15 @@ fn which(context: Context, s: &str) -> FunctionResult {
     1 => {
       // cmd is a regular command
       path_var = env::var_os("PATH").ok_or("Environment variable `PATH` is not set")?;
-      Either::Left(env::split_paths(&path_var).map(|path| path.join(cmd.clone())))
+      env::split_paths(&path_var).map(|path| path.join(cmd.clone())).collect()
     }
     _ => {
       // cmd contains a path separator, treat it as a path
-      Either::Right(iter::once(cmd))
+      vec![cmd]
     }
   };
 
-  for mut candidate in candidates.into_iter() {
+  for mut candidate in candidates {
     if candidate.is_relative() {
       // This candidate is a relative path, either because the user invoked `which("./rel/path")`,
       // or because there was a relative path in `PATH`. Resolve it to an absolute path.