Invoke hooks using bash explicitly

To run correctly on Windows, all invocations of hooks need to be
executed as an argument to bash.  This changes the code to build
commands using get_bash() with the `-c` flag.

When discovering which bash to use, we are careful on Windows to pick
"git-bash" aka the bash.exe that comes with git.  This is neccesary as
git-bash uses different path translations than regular bash.exe, and
using the wrong one would result in git.exe not being able to discover
the git-branchless.exe binary correctly (when invoked in hook).

Author: waych

src/core/rewrite.rs

@@ -11,7 +11,7 @@ use fn_error_context::context;
 use indicatif::{ProgressBar, ProgressStyle};
 
 use crate::core::formatting::printable_styled_string;
-use crate::util::{run_git, wrap_git_error, GitExecutable};
+use crate::util::{get_bash, run_git, wrap_git_error, GitExecutable};
 
 use super::eventlog::{
     Event, EventCursor, EventReplayer, EventTransactionId, BRANCHLESS_TRANSACTION_ID_ENV_VAR,
@@ -385,21 +385,25 @@ fn post_rebase_in_memory(
     rewritten_oids: &[(git2::Oid, git2::Oid)],
     event_tx_id: EventTransactionId,
 ) -> anyhow::Result<()> {
-    let post_rewrite_hook_path = repo
+    let hooks_path = repo
         .config()?
         .get_path("core.hooksPath")
-        .unwrap_or_else(|_| repo.path().join("hooks"))
-        .join("post-rewrite");
-    if post_rewrite_hook_path.exists() {
-        let mut child = Command::new(post_rewrite_hook_path.as_path())
-            .arg("rebase")
+        .unwrap_or_else(|_| repo.path().join("hooks"));
+    if hooks_path.join("post-rewrite").exists() {
+        let mut command = Command::new(get_bash().context("bash needed to run post-rewrite")?);
+        let mut child = command
+            .arg("-c")
+            // Always use Unix pathing here as this is an arg to bash.
+            // Prefix with ./ as hooks aren't in the PATH.
+            .arg("./post-rewrite rebase")
             .env(BRANCHLESS_TRANSACTION_ID_ENV_VAR, event_tx_id.to_string())
+            .current_dir(&hooks_path)
             .stdin(Stdio::piped())
             .spawn()
             .with_context(|| {
                 format!(
                     "Invoking post-rewrite hook at: {:?}",
-                    post_rewrite_hook_path.as_path()
+                    hooks_path.join("post-rewrite").as_path()
                 )
             })?;
 

src/testing.rs

@@ -7,7 +7,7 @@ use std::path::{Path, PathBuf};
 use std::process::Command;
 use std::str::FromStr;
 
-use crate::util::{wrap_git_error, GitExecutable, GitVersion};
+use crate::util::{get_bash, wrap_git_error, GitExecutable, GitVersion};
 use anyhow::Context;
 use fn_error_context::context;
 
@@ -99,11 +99,16 @@ impl Git {
             .git_executable
             .parent()
             .expect("Unable to find git path parent");
-        std::env::join_paths(vec![
+        let bash = get_bash().expect("bash missing?");
+        let bash_path = bash.parent().unwrap();
+        std::env::join_paths(
+            vec![
                 // For Git to be able to launch `git-branchless`.
                 branchless_path,
                 // For our hooks to be able to call back into `git`.
                 git_path,
+                // For branchless to manually invoke bash when needed.
+                bash_path,
             ]
             .iter()
             .map(|path| path.to_str().expect("Unable to decode path component")),

src/util.rs

@@ -2,6 +2,7 @@
 
 use std::collections::{HashMap, HashSet};
 use std::convert::TryInto;
+use std::env;
 use std::io::{stderr, stdout, Write};
 use std::path::PathBuf;
 use std::process::Command;
@@ -274,6 +275,42 @@ impl FromStr for GitVersion {
     }
 }
 
+/// Returns a path for a given file, searching through PATH to find it.
+pub fn get_from_path(exe_name: &str) -> Option<PathBuf> {
+    env::var_os("PATH").and_then(|paths| {
+        env::split_paths(&paths).find_map(|dir| {
+            let bash_path = dir.join(exe_name);
+            if bash_path.is_file() {
+                Some(bash_path.to_path_buf())
+            } else {
+                None
+            }
+        })
+    })
+}
+
+/// Returns the path to bash.
+pub fn get_bash() -> Option<PathBuf> {
+    let exe_name = if cfg!(target_os = "windows") {
+        "bash.exe"
+    } else {
+        "bash"
+    };
+    // If we are on Windows, first look for git.exe, and try to use it's bash, otherwise it won't
+    // be able to find git-branchless correctly.
+    if cfg!(target_os = "windows") {
+        // Git is typically installed at C:\Program Files\Git\cmd\git.exe with the cmd\ directory
+        // in the path, however git-bash is usually not in PATH and is in bin\ directory:
+        let git_path = get_from_path("git.exe").expect("Couldn't find git.exe");
+        let git_dir = git_path.parent().unwrap().parent().unwrap();
+        let git_bash = git_dir.join("bin").join(exe_name);
+        if git_bash.is_file() {
+            return Some(git_bash);
+        }
+    }
+    get_from_path(exe_name)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

tests/core/test_hooks.rs

@@ -1,4 +1,6 @@
+use anyhow::Context;
 use branchless::testing::with_git;
+use branchless::util::get_bash;
 use std::process::Command;
 
 fn preprocess_stderr(stderr: String) -> String {
@@ -166,8 +168,11 @@ fn test_pre_auto_gc() -> anyhow::Result<()> {
         // `pre-auto-gc` hook to be invoked at all. We'll just invoke the hook
         // directly to make sure that it's installed properly.
         std::env::set_current_dir(&git.repo_path)?;
-        let hook_path = git.repo_path.join(".git").join("hooks").join("pre-auto-gc");
-        let output = Command::new(hook_path)
+        let output = Command::new(get_bash().context("bash needed to run pre-auto-gc")?)
+            .arg("-c")
+            // Always use a unix style path here, as we are handing it to bash (even on Windows).
+            .arg("./.git/hooks/pre-auto-gc")
+            .current_dir(&git.repo_path)
             .env("PATH", git.get_path_for_env())
             .output()?;