From 015c17d8e77013c0337b13a047d519d1acdb72e9 Mon Sep 17 00:00:00 2001 From: Naseschwarz Date: Tue, 18 Mar 2025 23:38:40 +0100 Subject: [PATCH 1/2] Resolve core.hooksPath relative to GIT_WORK_TREE git supports relative values in core.hooksPath. `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. There is a further exception; I believe this is not considered for path resolution: > An exception are hooks triggered during a push (pre-receive, update, > post-receive, post-update, push-to-checkout) which are always executed > in $GIT_DIR. --- CHANGELOG.md | 3 ++ asyncgit/src/sync/hooks.rs | 50 +++++++++++++++++++++++ git2-hooks/src/hookspath.rs | 79 ++++++++++++++++++++++++++++++++----- 3 files changed, 122 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 012b60e5ff..850d444c1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,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..f6f7e1c999 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -76,6 +76,23 @@ 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() { @@ -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.path().parent().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 + ); + } +} From 892bced35a351dc62edf575116219c2d829f7d8d Mon Sep 17 00:00:00 2001 From: Naseschwarz Date: Sun, 23 Mar 2025 13:56:40 +0100 Subject: [PATCH 2/2] Favor Repository::workdir() over path().parent() This more clearly errors in case of bare repositories instead of using the parent directory of the bare repository. --- asyncgit/src/sync/hooks.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index f6f7e1c999..6fc672134f 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -97,7 +97,7 @@ mod tests { #[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' @@ -130,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 = @@ -160,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 @@ -196,7 +196,7 @@ mod tests { fn test_hooks_commit_msg_reject_in_hooks_folder_githooks_moved_absolute( ) { let (_td, repo) = repo_init().unwrap(); - let root = repo.path().parent().unwrap(); + let root = repo.workdir().unwrap(); let mut config = repo.config().unwrap(); const HOOKS_DIR: &'static str = "my_hooks";