refactor: central hook path and metadata handling

Author: jpgrayson

src/hook.rs

@@ -9,7 +9,9 @@ use anyhow::{anyhow, Context, Result};
 use crate::wrap::Message;
 
 /// Find path to hook script given a hook name.
-fn get_hook_path(repo: &gix::Repository, hook_name: &str) -> Result<PathBuf> {
+///
+/// Returns None if the hook script is not found or is not executable.
+fn get_hook_path(repo: &gix::Repository, hook_name: &str) -> Result<Option<PathBuf>> {
     let config = repo.config_snapshot();
     let hooks_path =
         if let Some(core_hooks_path) = config.trusted_path("core.hookspath").transpose()? {
@@ -27,7 +29,20 @@ fn get_hook_path(repo: &gix::Repository, hook_name: &str) -> Result<PathBuf> {
             // No core.hookspath, use default .git/hooks location
             Cow::Owned(repo.common_dir().join("hooks"))
         };
-    Ok(hooks_path.join(hook_name))
+    let hook_path = hooks_path.join(hook_name);
+
+    let hook_meta = match std::fs::metadata(&hook_path) {
+        Ok(meta) => meta,
+        Err(_) => return Ok(None), // ignore missing hook
+    };
+
+    if !is_executable(&hook_meta) {
+        return Ok(None);
+    }
+
+    let hook_path = gix::path::realpath(hook_path)?;
+
+    Ok(Some(hook_path))
 }
 
 /// Run the git `pre-commit` hook script.
@@ -40,25 +55,17 @@ fn get_hook_path(repo: &gix::Repository, hook_name: &str) -> Result<PathBuf> {
 /// existing, not being a file, or not being executable.
 pub(crate) fn run_pre_commit_hook(repo: &gix::Repository, use_editor: bool) -> Result<bool> {
     let hook_name = "pre-commit";
-    let hook_path = get_hook_path(repo, hook_name)?;
-    let hook_meta = match std::fs::metadata(&hook_path) {
-        Ok(meta) => meta,
-        Err(_) => return Ok(false), // ignore missing hook
-    };
-
-    if !hook_meta.is_file() {
-        return Ok(false);
-    }
-
-    // Ignore non-executable hooks
-    if !is_executable(&hook_meta) {
+    let hook_path = if let Some(hook_path) = get_hook_path(repo, hook_name)? {
+        hook_path
+    } else {
         return Ok(false);
-    }
+    };
 
-    let mut hook_command = std::process::Command::new(gix::path::realpath(hook_path)?);
     let workdir = repo
         .work_dir()
         .expect("should not get this far with a bare repo");
+
+    let mut hook_command = std::process::Command::new(hook_path);
     if !use_editor {
         hook_command.env("GIT_EDITOR", ":");
     }
@@ -95,20 +102,11 @@ pub(crate) fn run_commit_msg_hook<'repo>(
     use_editor: bool,
 ) -> Result<Message<'repo>> {
     let hook_name = "commit-msg";
-    let hook_path = get_hook_path(repo, hook_name)?;
-    let hook_meta = match std::fs::metadata(&hook_path) {
-        Ok(meta) => meta,
-        Err(_) => return Ok(message), // ignore missing hook
-    };
-
-    if !hook_meta.is_file() {
-        return Ok(message);
-    }
-
-    // Ignore non-executable hooks
-    if !is_executable(&hook_meta) {
+    let hook_path = if let Some(hook_path) = get_hook_path(repo, hook_name)? {
+        hook_path
+    } else {
         return Ok(message);
-    }
+    };
 
     let mut msg_file = tempfile::NamedTempFile::new()?;
     msg_file.write_all(message.raw_bytes())?;
@@ -118,7 +116,7 @@ pub(crate) fn run_commit_msg_hook<'repo>(
 
     // TODO: when git runs this hook, it only sets GIT_INDEX_FILE and sometimes
     // GIT_EDITOR. So author and committer vars are not clearly required.
-    let mut hook_command = std::process::Command::new(gix::path::realpath(hook_path)?);
+    let mut hook_command = std::process::Command::new(hook_path);
     hook_command.env("GIT_INDEX_FILE", &index_path);
     if !use_editor {
         hook_command.env("GIT_EDITOR", ":");