Cleanup hooks (#1972)

* cleanup errors
* cleaner repo structure
* added docs

6 files changed, 155 insertions, 147 deletions

diff --git a/Cargo.lock b/Cargo.lock
index f378e57e..80925180 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -714,7 +714,7 @@ dependencies = [
 
 [[package]]
 name = "git2-hooks"
-version = "0.1.0"
+version = "0.2.0"
 dependencies = [
  "git2",
  "git2-testing",
diff --git a/asyncgit/Cargo.toml b/asyncgit/Cargo.toml
index b055a89f..e6046872 100644
--- a/asyncgit/Cargo.toml
+++ b/asyncgit/Cargo.toml
@@ -17,7 +17,7 @@ crossbeam-channel = "0.5"
 easy-cast = "0.5"
 fuzzy-matcher = "0.3"
 git2 = "0.17"
-git2-hooks = { path = "../git2-hooks", version = "0.1" }
+git2-hooks = { path = "../git2-hooks", version = "0.2" }
 log = "0.4"
 # git2 = { path = "../../extern/git2-rs", features = ["vendored-openssl"]}
 # git2 = { git="https://github.com/extrawurst/git2-rs.git", rev="fc13dcc", features = ["vendored-openssl"]}
diff --git a/git2-hooks/Cargo.toml b/git2-hooks/Cargo.toml
index b7a197c7..c51ef57b 100644
--- a/git2-hooks/Cargo.toml
+++ b/git2-hooks/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "git2-hooks"
-version = "0.1.0"
+version = "0.2.0"
 authors = ["extrawurst <mail@rusticorn.com>"]
 edition = "2021"
 description = "adds git hooks support based on git2-rs"
diff --git a/git2-hooks/src/error.rs b/git2-hooks/src/error.rs
index bcd066d5..e8bce462 100644
--- a/git2-hooks/src/error.rs
+++ b/git2-hooks/src/error.rs
@@ -4,14 +4,6 @@ use thiserror::Error;
 #[derive(Error, Debug)]
 pub enum HooksError {
 	///
-	#[error("`{0}`")]
-	Generic(String),
-
-	///
-	#[error("git error:{0}")]
-	Git(#[from] git2::Error),
-
-	///
 	#[error("io error:{0}")]
 	Io(#[from] std::io::Error),
 
@@ -21,7 +13,7 @@ pub enum HooksError {
 
 	///
 	#[error("shellexpand error:{0}")]
-	Shell(#[from] shellexpand::LookupError<std::env::VarError>),
+	ShellExpand(#[from] shellexpand::LookupError<std::env::VarError>),
 }
 
 ///
diff --git a/git2-hooks/src/hookspath.rs b/git2-hooks/src/hookspath.rs
new file mode 100644
index 00000000..38f32c4e
--- /dev/null
+++ b/git2-hooks/src/hookspath.rs
@@ -0,0 +1,138 @@
+use git2::Repository;
+
+use crate::{error::Result, HookResult, HooksError};
+
+use std::{
+	path::Path, path::PathBuf, process::Command, str::FromStr,
+};
+
+pub struct HookPaths {
+	pub git: PathBuf,
+	pub hook: PathBuf,
+	pub pwd: PathBuf,
+}
+
+impl HookPaths {
+	pub fn new(repo: &Repository, hook: &str) -> Result<Self> {
+		let pwd = repo
+			.workdir()
+			.unwrap_or_else(|| repo.path())
+			.to_path_buf();
+
+		let git_dir = repo.path().to_path_buf();
+		let hooks_path = repo
+			.config()
+			.and_then(|config| config.get_string("core.hooksPath"))
+			.map_or_else(
+				|e| {
+					log::error!("hookspath error: {}", e);
+					repo.path().to_path_buf().join("hooks/")
+				},
+				PathBuf::from,
+			);
+
+		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)?;
+
+		Ok(Self {
+			git: git_dir,
+			hook,
+			pwd,
+		})
+	}
+
+	pub fn is_executable(&self) -> bool {
+		self.hook.exists() && is_executable(&self.hook)
+	}
+
+	/// this function calls hook scripts based on conventions documented here
+	/// see <https://git-scm.com/docs/githooks>
+	pub fn run_hook(&self, args: &[&str]) -> Result<HookResult> {
+		let arg_str = format!("{:?} {}", self.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 '{:?}'", self.hook, self.pwd);
+
+		let git_bash = find_bash_executable()
+			.unwrap_or_else(|| PathBuf::from("bash"));
+		let output = Command::new(git_bash)
+			.args(bash_args)
+			.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",
+			)
+			.output()?;
+
+		if output.status.success() {
+			Ok(HookResult::Ok)
+		} else {
+			let stderr =
+				String::from_utf8_lossy(&output.stderr).to_string();
+			let stdout =
+				String::from_utf8_lossy(&output.stdout).to_string();
+
+			Ok(HookResult::NotOk { stdout, stderr })
+		}
+	}
+}
+
+#[cfg(not(windows))]
+fn is_executable(path: &Path) -> bool {
+	use std::os::unix::fs::PermissionsExt;
+
+	let metadata = match path.metadata() {
+		Ok(metadata) => metadata,
+		Err(e) => {
+			log::error!("metadata error: {}", e);
+			return false;
+		}
+	};
+
+	let permissions = metadata.permissions();
+
+	permissions.mode() & 0o111 != 0
+}
+
+#[cfg(windows)]
+/// windows does not consider bash 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
+	}
+}
diff --git a/git2-hooks/src/lib.rs b/git2-hooks/src/lib.rs
index feddd3a4..4cf4c0bc 100644
--- a/git2-hooks/src/lib.rs
+++ b/git2-hooks/src/lib.rs
@@ -1,21 +1,29 @@
+//! git2-rs addon supporting git hooks
+//!
+//! most basic hook is: [`hooks_pre_commit`]. see also other `hooks_*` functions
+//!
+//! [`create_hook`] is useful to create git hooks from code (unittest make heavy usage of it)
 mod error;
+mod hookspath;
 
 use std::{
 	fs::File,
 	io::{Read, Write},
 	path::{Path, PathBuf},
 	process::Command,
-	str::FromStr,
 };
 
 pub use error::HooksError;
 use error::Result;
+use hookspath::HookPaths;
+
 use git2::Repository;
 
 pub const HOOK_POST_COMMIT: &str = "post-commit";
 pub const HOOK_PRE_COMMIT: &str = "pre-commit";
 pub const HOOK_COMMIT_MSG: &str = "commit-msg";
-pub const HOOK_COMMIT_MSG_TEMP_FILE: &str = "COMMIT_EDITMSG";
+
+const HOOK_COMMIT_MSG_TEMP_FILE: &str = "COMMIT_EDITMSG";
 
 ///
 #[derive(Debug, PartialEq, Eq)]
@@ -26,91 +34,7 @@ pub enum HookResult {
 	NotOk { stdout: String, stderr: String },
 }
 
-struct HookPaths {
-	git: PathBuf,
-	hook: PathBuf,
-	pwd: PathBuf,
-}
-
-impl HookPaths {
-	pub fn new(repo: &Repository, hook: &str) -> Result<Self> {
-		let pwd = repo
-			.workdir()
-			.unwrap_or_else(|| repo.path())
-			.to_path_buf();
-
-		let git_dir = repo.path().to_path_buf();
-		let hooks_path = repo
-			.config()
-			.and_then(|config| config.get_string("core.hooksPath"))
-			.map_or_else(
-				|e| {
-					log::error!("hookspath error: {}", e);
-					repo.path().to_path_buf().join("hooks/")
-				},
-				PathBuf::from,
-			);
-
-		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)?;
-
-		Ok(Self {
-			git: git_dir,
-			hook,
-			pwd,
-		})
-	}
-
-	pub fn is_executable(&self) -> bool {
-		self.hook.exists() && is_executable(&self.hook)
-	}
-
-	/// this function calls hook scripts based on conventions documented here
-	/// see <https://git-scm.com/docs/githooks>
-	pub fn run_hook(&self, args: &[&str]) -> Result<HookResult> {
-		let arg_str = format!("{:?} {}", self.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 '{:?}'", self.hook, self.pwd);
-
-		let git_bash = find_bash_executable()
-			.unwrap_or_else(|| PathBuf::from("bash"));
-		let output = Command::new(git_bash)
-			.args(bash_args)
-			.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",
-			)
-			.output()?;
-
-		if output.status.success() {
-			Ok(HookResult::Ok)
-		} else {
-			let stderr =
-				String::from_utf8_lossy(&output.stderr).to_string();
-			let stdout =
-				String::from_utf8_lossy(&output.stdout).to_string();
-
-			Ok(HookResult::NotOk { stdout, stderr })
-		}
-	}
-}
-
-/// helper method to create git hooks
+/// helper method to create git hooks programmatically (heavy used in unittests)
 pub fn create_hook(
 	r: &Repository,
 	hook: &str,
@@ -170,7 +94,6 @@ pub fn hooks_commit_msg(
 }
 
 /// this hook is documented here <https://git-scm.com/docs/githooks#_pre_commit>
-///
 pub fn hooks_pre_commit(repo: &Repository) -> Result<HookResult> {
 	let hook = HookPaths::new(repo, HOOK_PRE_COMMIT)?;
 
@@ -180,7 +103,8 @@ pub fn hooks_pre_commit(repo: &Repository) -> Result<HookResult> {
 		Ok(HookResult::Ok)
 	}
 }
-///
+
+/// this hook is documented here <https://git-scm.com/docs/githooks#_post_commit>
 pub fn hooks_post_commit(repo: &Repository) -> Result<HookResult> {
 	let hook = HookPaths::new(repo, HOOK_POST_COMMIT)?;
 
@@ -191,52 +115,6 @@ pub fn hooks_post_commit(repo: &Repository) -> Result<HookResult> {
 	}
 }
 
-#[cfg(not(windows))]
-fn is_executable(path: &Path) -> bool {
-	use std::os::unix::fs::PermissionsExt;
-	let metadata = match path.metadata() {
-		Ok(metadata) => metadata,
-		Err(e) => {
-			log::error!("metadata error: {}", e);
-			return false;
-		}
-	};
-
-	let permissions = metadata.permissions();
-
-	permissions.mode() & 0o111 != 0
-}
-
-#[cfg(windows)]
-/// windows does not consider bash 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
-	}
-}
-
 #[cfg(test)]
 mod tests {
 	use super::*;