diff --git a/CHANGELOG.md b/CHANGELOG.md index 012b60e5ff..7b386b72be 100644 --- a/CHANGELOG.md +++ b/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)) ### Added * support loading custom syntax highlighting themes from a file [[@acuteenvy](https://github.com/acuteenvy)] ([#2565](https://github.com/gitui-org/gitui/pull/2565)) @@ -24,6 +25,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * set the terminal title to `gitui ({repo_path})` [[@acuteenvy](https://github.com/acuteenvy)] ([#2462](https://github.com/gitui-org/gitui/issues/2462)) * respect `.mailmap` [[@acuteenvy](https://github.com/acuteenvy)] ([#2406](https://github.com/gitui-org/gitui/issues/2406)) +### Fixes +* Resolve `core.hooksPath` relative to `GIT_WORK_TREE` [[@naseschwarz](https://github.com/naseschwarz)] ([#2571](https://github.com/gitui-org/gitui/issues/2571)) + ## [0.27.0] - 2024-01-14 **new: manage remotes** diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index 3e82cf6f8d..f0d1e06e3b 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -74,13 +74,37 @@ 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 $# ") + } + use std::fs::File; + use std::io::Write; + use std::path::Path; + + fn create_hook_in_path(path: &Path, hook_script: &[u8]) { + File::create(path).unwrap().write_all(hook_script).unwrap(); + + #[cfg(unix)] + { + std::process::Command::new("chmod") + .arg("+x") + .arg(path) + // .current_dir(path) + .output() + .unwrap(); + } + } #[test] fn test_post_commit_hook_reject_in_subfolder() { let (_td, repo) = repo_init().unwrap(); - let root = repo.path().parent().unwrap(); + let root = repo.workdir().unwrap(); let hook = b"#!/bin/sh echo 'rejected' @@ -113,14 +137,14 @@ mod tests { #[cfg(unix)] fn test_pre_commit_workdir() { let (_td, repo) = repo_init().unwrap(); - let root = repo.path().parent().unwrap(); + let root = repo.workdir().unwrap(); let repo_path: &RepoPath = &root.as_os_str().to_str().unwrap().into(); let workdir = crate::sync::utils::repo_work_dir(repo_path).unwrap(); let hook = b"#!/bin/sh - echo $(pwd) + echo \"$(pwd)\" exit 1 "; @@ -143,10 +167,10 @@ mod tests { #[test] fn test_hooks_commit_msg_reject_in_subfolder() { let (_td, repo) = repo_init().unwrap(); - let root = repo.path().parent().unwrap(); + let root = repo.workdir().unwrap(); let hook = b"#!/bin/sh - echo 'msg' > $1 + echo 'msg' > \"$1\" echo 'rejected' exit 1 "; @@ -174,4 +198,37 @@ mod tests { assert_eq!(msg, String::from("msg\n")); } + + #[test] + fn test_hooks_commit_msg_reject_in_hooks_folder_githooks_moved_absolute( + ) { + let (_td, repo) = repo_init_with_prefix("bla '").unwrap(); + let root = repo.workdir().unwrap(); + let mut config = repo.config().unwrap(); + + const HOOKS_DIR: &'static str = "my_hooks"; + config.set_str("core.hooksPath", HOOKS_DIR).unwrap(); + + let hook = b"#!/bin/sh + echo 'msg' > \"$1\" + echo 'rejected' + exit 1 + "; + let hooks_folder = root.join(HOOKS_DIR); + std::fs::create_dir_all(&hooks_folder).unwrap(); + create_hook_in_path(&hooks_folder.join("commit-msg"), hook); + + let mut msg = String::from("test"); + let res = hooks_commit_msg( + &hooks_folder.to_str().unwrap().into(), + &mut msg, + ) + .unwrap(); + assert_eq!( + res, + HookResult::NotOk(String::from("rejected\n")) + ); + + assert_eq!(msg, String::from("msg\n")); + } } diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index c52a556aad..683aacd0ef 100644 --- a/asyncgit/src/sync/mod.rs +++ b/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()?; diff --git a/git2-hooks/src/hookspath.rs b/git2-hooks/src/hookspath.rs index 3648676ee2..57d54fa102 100644 --- a/git2-hooks/src/hookspath.rs +++ b/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, @@ -41,16 +41,8 @@ impl HookPaths { if let Some(config_path) = Self::config_hook_path(repo)? { let hooks_path = PathBuf::from(config_path); - let hook = hooks_path.join(hook); - - let hook = shellexpand::full( - hook.as_os_str() - .to_str() - .ok_or(HooksError::PathToString)?, - )?; - - let hook = PathBuf::from_str(hook.as_ref()) - .map_err(|_| HooksError::PathToString)?; + let hook = + Self::expand_path(&hooks_path.join(hook), &pwd)?; return Ok(Self { git: git_dir, @@ -66,6 +58,41 @@ impl HookPaths { }) } + /// Expand path according to the rule of githooks and config + /// core.hooksPath + fn expand_path(path: &Path, pwd: &Path) -> Result { + let hook_expanded = shellexpand::full( + path.as_os_str() + .to_str() + .ok_or(HooksError::PathToString)?, + )?; + let hook_expanded = PathBuf::from_str(hook_expanded.as_ref()) + .map_err(|_| HooksError::PathToString)?; + + // `man git-config`: + // + // > A relative path is taken as relative to the + // > directory where the hooks are run (see the + // > "DESCRIPTION" section of githooks[5]). + // + // `man githooks`: + // + // > Before Git invokes a hook, it changes its + // > working directory to either $GIT_DIR in a bare + // > repository or the root of the working tree in a + // > non-bare repository. + // + // I.e. relative paths in core.hooksPath in non-bare + // repositories are always relative to GIT_WORK_TREE. + Ok({ + if hook_expanded.is_absolute() { + hook_expanded + } else { + pwd.join(hook_expanded) + } + }) + } + fn config_hook_path(repo: &Repository) -> Result> { Ok(repo.config()?.get_string(CONFIG_HOOKS_PATH).ok()) } @@ -108,29 +135,45 @@ impl HookPaths { /// see pub fn run_hook(&self, args: &[&str]) -> Result { 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 +193,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::::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 +235,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 { - if cfg!(windows) { - Command::new("where.exe") - .arg("git") - .output() - .ok() - .map(|out| { - PathBuf::from(Into::::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 { - 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 @@ -232,3 +271,35 @@ impl CommandExt for Command { self } } + +#[cfg(test)] +mod test { + use super::HookPaths; + use std::path::Path; + + #[test] + fn test_hookspath_relative() { + assert_eq!( + HookPaths::expand_path( + &Path::new("pre-commit"), + &Path::new("example_git_root"), + ) + .unwrap(), + Path::new("example_git_root").join("pre-commit") + ); + } + + #[test] + fn test_hookspath_absolute() { + let absolute_hook = + std::env::current_dir().unwrap().join("pre-commit"); + assert_eq!( + HookPaths::expand_path( + &absolute_hook, + &Path::new("example_git_root"), + ) + .unwrap(), + absolute_hook + ); + } +} diff --git a/git2-hooks/src/lib.rs b/git2-hooks/src/lib.rs index 2a458856d7..d8d5f796e4 100644 --- a/git2-hooks/src/lib.rs +++ b/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 ";