diff --git a/CHANGELOG.md b/CHANGELOG.md index 423aea3ee8..c7caec869d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * 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)) * yanking commit ranges no longer generates incorrect dotted range notations, but lists each individual commit [[@naseschwarz](https://github.com/naseschwarz)] (https://github.com/gitui-org/gitui/issues/2576) ## [0.27.0] - 2024-01-14 diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index 3e82cf6f8d..6fc672134f 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -76,11 +76,28 @@ pub fn hooks_prepare_commit_msg( mod tests { use super::*; use crate::sync::tests::repo_init; + 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,7 +130,7 @@ 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 = @@ -143,7 +160,7 @@ 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 @@ -174,4 +191,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().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/git2-hooks/src/hookspath.rs b/git2-hooks/src/hookspath.rs index 3648676ee2..af4f8af0a7 100644 --- a/git2-hooks/src/hookspath.rs +++ b/git2-hooks/src/hookspath.rs @@ -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()) } @@ -232,3 +259,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 + ); + } +}