Run git-hooks more correctly

CHANGELOG.md

@@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
+* execute git-hooks without shell (on *nix) [[@Joshix](https://github.com/Joshix-1)] ([#2483](https://github.com/extrawurst/gitui/pull/2483))
 
 ## [0.27.0] - 2024-01-14
 

asyncgit/src/sync/hooks.rs

@@ -74,8 +74,15 @@ pub fn hooks_prepare_commit_msg(
 
 #[cfg(test)]
 mod tests {
+	use git2::Repository;
+	use tempfile::TempDir;
+
 	use super::*;
-	use crate::sync::tests::repo_init;
+	use crate::sync::tests::repo_init_with_prefix;
+
+	fn repo_init() -> Result<(TempDir, Repository)> {
+		repo_init_with_prefix("gitui $#  ")
+	}
 
 	#[test]
 	fn test_post_commit_hook_reject_in_subfolder() {
@@ -120,7 +127,7 @@ mod tests {
 			crate::sync::utils::repo_work_dir(repo_path).unwrap();
 
 		let hook = b"#!/bin/sh
-	echo $(pwd)
+	echo \"$(pwd)\"
 	exit 1
 	        ";
 
@@ -146,7 +153,7 @@ mod tests {
 		let root = repo.path().parent().unwrap();
 
 		let hook = b"#!/bin/sh
-	echo 'msg' > $1
+	echo 'msg' > \"$1\"
 	echo 'rejected'
 	exit 1
 	        ";

asyncgit/src/sync/mod.rs

@@ -144,11 +144,19 @@ pub mod tests {
 
 	///
 	pub fn repo_init() -> Result<(TempDir, Repository)> {
+		repo_init_with_prefix("gitui")
+	}
+
+	///
+	#[inline]
+	pub fn repo_init_with_prefix(
+		prefix: &'static str,
+	) -> Result<(TempDir, Repository)> {
 		init_log();
 
 		sandbox_config_files();
 
-		let td = TempDir::new()?;
+		let td = TempDir::with_prefix(prefix)?;
 		let repo = Repository::init(td.path())?;
 		{
 			let mut config = repo.config()?;

git2-hooks/src/hookspath.rs

@@ -3,7 +3,7 @@ use git2::Repository;
 use crate::{error::Result, HookResult, HooksError};
 
 use std::{
-	env,
+	ffi::OsStr,
 	path::{Path, PathBuf},
 	process::Command,
 	str::FromStr,
@@ -108,29 +108,45 @@ impl HookPaths {
 	/// see <https://git-scm.com/docs/githooks>
 	pub fn run_hook(&self, args: &[&str]) -> Result<HookResult> {
 		let hook = self.hook.clone();
-
-		let arg_str = format!("{:?} {}", hook, args.join(" "));
-		// Use -l to avoid "command not found" on Windows.
-		let bash_args =
-			vec!["-l".to_string(), "-c".to_string(), arg_str];
-
 		log::trace!("run hook '{:?}' in '{:?}'", hook, self.pwd);
 
-		let git_shell = find_bash_executable()
-			.or_else(find_default_unix_shell)
-			.unwrap_or_else(|| "bash".into());
-		let output = Command::new(git_shell)
-			.args(bash_args)
-			.with_no_window()
-			.current_dir(&self.pwd)
-			// This call forces Command to handle the Path environment correctly on windows,
-			// the specific env set here does not matter
-			// see https://github.com/rust-lang/rust/issues/37519
-			.env(
-				"DUMMY_ENV_TO_FIX_WINDOWS_CMD_RUNS",
-				"FixPathHandlingOnWindows",
+		let run_command = |command: &mut Command| {
+			command.current_dir(&self.pwd).with_no_window().output()
+		};
+		let output = if cfg!(windows) {
+			let command = {
+				let mut os_str = std::ffi::OsString::new();
+				os_str.push("'");
+				os_str.push(hook.as_os_str()); // TODO: this doesn't work if `hook` contains single-quotes
+				os_str.push("'");
+				os_str.push(" \"$@\"");
+				os_str
+			};
+			let shell_args = {
+				let mut shell_args = vec![
+					OsStr::new("-l"), // Use -l to avoid "command not found"
+					OsStr::new("-c"),
+					command.as_os_str(),
+					hook.as_os_str(),
+				];
+				shell_args.extend(args.iter().map(OsStr::new));
+				shell_args
+			};
+
+			run_command(
+				Command::new(sh_on_windows())
+					.args(shell_args)
+					// This call forces Command to handle the Path environment correctly on windows,
+					// the specific env set here does not matter
+					// see https://github.com/rust-lang/rust/issues/37519
+					.env(
+						"DUMMY_ENV_TO_FIX_WINDOWS_CMD_RUNS",
+						"FixPathHandlingOnWindows",
+					),
 			)
-			.output()?;
+		} else {
+			run_command(Command::new(&hook).args(args))
+		}?;
 
 		if output.status.success() {
 			Ok(HookResult::Ok { hook })
@@ -150,6 +166,30 @@ impl HookPaths {
 	}
 }
 
+/// Get the path to the sh.exe bundled with Git for Windows
+fn sh_on_windows() -> PathBuf {
+	if cfg!(windows) {
+		Command::new("where.exe")
+			.arg("git")
+			.output()
+			.ok()
+			.map(|out| {
+				PathBuf::from(Into::<String>::into(
+					String::from_utf8_lossy(&out.stdout),
+				))
+			})
+			.as_deref()
+			.and_then(Path::parent)
+			.and_then(Path::parent)
+			.map(|p| p.join("usr/bin/sh.exe"))
+			.filter(|p| p.exists())
+			.unwrap_or_else(|| "sh".into())
+	} else {
+		debug_assert!(false, "should only be called on windows");
+		"sh".into()
+	}
+}
+
 #[cfg(unix)]
 fn is_executable(path: &Path) -> bool {
 	use std::os::unix::fs::PermissionsExt;
@@ -168,40 +208,12 @@ fn is_executable(path: &Path) -> bool {
 }
 
 #[cfg(windows)]
-/// windows does not consider bash scripts to be executable so we consider everything
+/// windows does not consider shell scripts to be executable so we consider everything
 /// to be executable (which is not far from the truth for windows platform.)
 const fn is_executable(_: &Path) -> bool {
 	true
 }
 
-// Find bash.exe, and avoid finding wsl's bash.exe on Windows.
-// None for non-Windows.
-fn find_bash_executable() -> Option<PathBuf> {
-	if cfg!(windows) {
-		Command::new("where.exe")
-			.arg("git")
-			.output()
-			.ok()
-			.map(|out| {
-				PathBuf::from(Into::<String>::into(
-					String::from_utf8_lossy(&out.stdout),
-				))
-			})
-			.as_deref()
-			.and_then(Path::parent)
-			.and_then(Path::parent)
-			.map(|p| p.join("usr/bin/bash.exe"))
-			.filter(|p| p.exists())
-	} else {
-		None
-	}
-}
-
-// Find default shell on Unix-like OS.
-fn find_default_unix_shell() -> Option<PathBuf> {
-	env::var_os("SHELL").map(PathBuf::from)
-}
-
 trait CommandExt {
 	/// The process is a console application that is being run without a
 	/// console window. Therefore, the console handle for the application is

git2-hooks/src/lib.rs

@@ -282,7 +282,7 @@ exit 0
 
 		let hook = br#"#!/bin/sh
 COMMIT_MSG="$(cat "$1")"
-printf "$COMMIT_MSG" | sed 's/sth/shell_command/g' >"$1"
+printf "$COMMIT_MSG" | sed 's/sth/shell_command/g' > "$1"
 exit 0
         "#;
 
@@ -499,7 +499,7 @@ sys.exit(1)
 		let (_td, repo) = repo_init();
 
 		let hook = b"#!/bin/sh
-echo 'msg' > $1
+echo 'msg' > \"$1\"
 echo 'rejected'
 exit 1
         ";
@@ -525,7 +525,7 @@ exit 1
 		let (_td, repo) = repo_init();
 
 		let hook = b"#!/bin/sh
-echo 'msg' > $1
+echo 'msg' > \"$1\"
 exit 0
         ";
 
@@ -565,7 +565,7 @@ exit 0
 		let (_td, repo) = repo_init();
 
 		let hook = b"#!/bin/sh
-echo msg:$2 > $1
+echo \"msg:$2\" > \"$1\"
 exit 0
         ";
 
@@ -589,7 +589,7 @@ exit 0
 		let (_td, repo) = repo_init();
 
 		let hook = b"#!/bin/sh
-echo $2,$3 > $1
+echo \"$2,$3\" > \"$1\"
 echo 'rejected'
 exit 2
         ";