From 1d1fc8d4bc53a49277ff31ba7a6d469abfa782a2 Mon Sep 17 00:00:00 2001 From: Andrei Gherzan Date: Fri, 17 Jul 2026 13:21:49 +0100 Subject: [PATCH] feat: add --message-file for commit-msg hook support Enables validation of commit messages before the git object exists, allowing gitlance to work as a commit-msg hook. Comment lines are ignored, matching how a commit message is interpreted. The new mode/argument is mutually exclusive with the ref-based options (--base, --head, --repo, --skip-merge-commits), which are only meaningful when validating existing commits. Signed-off-by: Andrei Gherzan --- README.md | 25 ++++++ Taskfile.yml | 2 + githooks/commit-msg | 25 ++++++ src/git.rs | 65 ++++++++++++++++ src/main.rs | 103 +++++++++++++------------ tests/integration_tests.rs | 153 ++++++++++++++++++++++++++++++++----- 6 files changed, 306 insertions(+), 67 deletions(-) create mode 100755 githooks/commit-msg diff --git a/README.md b/README.md index 0e36ff8..eec49bb 100644 --- a/README.md +++ b/README.md @@ -224,6 +224,31 @@ steps: run: exit 1 ``` +## CLI Usage + +gitlance can also be run directly as a command-line tool, independent of GitHub +Actions. + +### Validating a Single Commit Message + +Instead of a commit range, gitlance can validate a single commit message read +from a file, using `--message-file`. This makes it convenient to use in a local +`commit-msg` git hook, where git passes the path to the prepared message as the +first argument (`$1`): + +```bash +#!/bin/sh +# .git/hooks/commit-msg +gitlance --message-file "$1" +``` + +To validate a message file directly (for example the last prepared message), +pass its path explicitly: + +```bash +gitlance --message-file .git/COMMIT_EDITMSG +``` + ## Installation ### GitHub Action diff --git a/Taskfile.yml b/Taskfile.yml index 7e1a115..83ff7bf 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -27,6 +27,8 @@ tasks: cmds: - cp githooks/pre-push "{{.HOOKS_DIR}}/" - chmod +x "{{.HOOKS_DIR}}/pre-push" + - cp githooks/commit-msg "{{.HOOKS_DIR}}/" + - chmod +x "{{.HOOKS_DIR}}/commit-msg" - echo "✓ Git hooks installed to {{.HOOKS_DIR}}" setup: diff --git a/githooks/commit-msg b/githooks/commit-msg new file mode 100755 index 0000000..6cbe31a --- /dev/null +++ b/githooks/commit-msg @@ -0,0 +1,25 @@ +#!/bin/bash + +# SPDX-FileCopyrightText: Canonical Ltd. +# +# SPDX-License-Identifier: Apache-2.0 + +set -e + +# Check that gitlance is available +if ! command -v gitlance >/dev/null 2>&1; then + echo "Error: gitlance is not installed or not in PATH" + echo "To bypass: git commit --no-verify" + exit 1 +fi + +message_file="$1" + +if ! gitlance --message-file "$message_file"; then + echo + echo "Commit rejected: Message validation failed" + echo "To bypass: git commit --no-verify" + exit 1 +fi + +exit 0 diff --git a/src/git.rs b/src/git.rs index d55e205..cfb1445 100644 --- a/src/git.rs +++ b/src/git.rs @@ -12,6 +12,34 @@ pub struct Commit { pub message: String, } +impl Commit { + /// Creates a Commit from a raw commit message (no SHA available). + /// + /// Comment lines are stripped, matching how git treats a commit message. + pub fn from_message(message: String) -> Self { + Self { + sha: "-".to_string(), + message: strip_comments(&message), + } + } +} + +/// The git "scissors" line. Everything from this line onward is discarded by +/// git when interpreting a commit message (e.g. with `commit --verbose`). +const SCISSORS: &str = "# ------------------------ >8 ------------------------"; + +/// Strips content the way git interprets a commit message: everything from the +/// scissors line onward is dropped, along with any comment lines (starting with +/// `#`, ignoring leading whitespace). +fn strip_comments(message: &str) -> String { + message + .lines() + .take_while(|line| line.trim_start() != SCISSORS) + .filter(|line| !line.trim_start().starts_with('#')) + .collect::>() + .join("\n") +} + /// Opens a git repository at the specified path pub fn open_repo(repo_path: &str) -> Result { Repository::open(repo_path) @@ -99,6 +127,43 @@ mod tests { use crate::test_utils::*; use tempfile::TempDir; + #[test] + fn test_strip_comments_hash_not_at_line_start_is_kept() { + assert_eq!(strip_comments("fix: issue #42"), "fix: issue #42"); + } + + #[test] + fn test_strip_comments_indented_hash_is_comment() { + // Git treats leading-whitespace `#` lines as comments too. + assert_eq!(strip_comments("feat: x\n # indented"), "feat: x"); + } + + #[test] + fn test_strip_comments_all_comments_yields_empty() { + assert_eq!(strip_comments("# only\n# comments"), ""); + } + + #[test] + fn test_strip_comments_drops_content_after_scissors() { + let raw = "feat: subject\n\nBody\n# ------------------------ >8 ------------------------\ndiff --git a/x b/x\nWIP leftover"; + assert_eq!(strip_comments(raw), "feat: subject\n\nBody"); + } + + #[test] + fn test_strip_comments_indented_scissors_is_honored() { + let raw = + "feat: subject\n # ------------------------ >8 ------------------------\nignored"; + assert_eq!(strip_comments(raw), "feat: subject"); + } + + #[test] + fn test_from_message_strips_comment_lines() { + let raw = "# Please enter the commit message\nfeat: subject\n\n# a comment\nBody\n"; + let commit = Commit::from_message(raw.to_string()); + assert_eq!(commit.sha, "-"); + assert_eq!(commit.message, "feat: subject\n\nBody"); + } + #[test] fn test_open_repo_success() { let temp_dir = TempDir::new().expect("Failed to create temp dir"); diff --git a/src/main.rs b/src/main.rs index 197918a..d88b239 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,12 +22,16 @@ struct Cli { head: Option, /// Git repository path - #[arg(long, global = true, default_value = ".")] - repo: String, + #[arg(long, global = true)] + repo: Option, /// Skip merge commits in validation (default: false, all commits are checked) #[arg(long, global = true)] skip_merge_commits: bool, + + /// Validate a single commit message from file (e.g. .git/COMMIT_EDITMSG) + #[arg(long, global = true, conflicts_with_all = ["base", "head", "skip_merge_commits", "repo"])] + message_file: Option, } #[derive(Subcommand)] @@ -65,57 +69,58 @@ fn main() { // Determine which checks to run let command = cli.command.unwrap_or(Commands::All); - // Get refs, with environment variable fallback - let base = cli.base.or_else(|| std::env::var("BASE_REF").ok()); - let head = cli.head.or_else(|| std::env::var("HEAD_REF").ok()); - - // Validate both refs before proceeding - let mut has_errors = false; - - if base.is_none() { - output::error( - "Missing base reference. Provide --base or set BASE_REF environment variable", - ); - has_errors = true; - } - - if head.is_none() { - output::error( - "Missing head reference. Provide --head or set HEAD_REF environment variable", - ); - has_errors = true; - } - - if has_errors { - exit(1); - } - - let base = base.expect("base should be validated above"); - let head = head.expect("head should be validated above"); - - // Open repository - let repo = match git::open_repo(&cli.repo) { - Ok(r) => r, - Err(e) => { - output::error(&format!("Failed to open repository: {}", e)); - exit(1); + // Get commits based on input mode + let commits = if let Some(path) = cli.message_file { + match std::fs::read_to_string(&path) { + Ok(message) => vec![git::Commit::from_message(message)], + Err(e) => { + output::error(&format!("Failed to read '{}': {}", path.display(), e)); + exit(1); + } } - }; - - // Get commits in range - let commits = match git::get_commits_in_range(&repo, &base, &head, cli.skip_merge_commits) { - Ok(commits) => commits, - Err(e) => { - output::error(&format!("Failed to get commits: {}", e)); - exit(1); + } else { + let base = cli.base.or_else(|| std::env::var("BASE_REF").ok()); + let head = cli.head.or_else(|| std::env::var("HEAD_REF").ok()); + + let (base, head) = match (base, head) { + (Some(b), Some(h)) => (b, h), + (None, None) => { + output::error( + "Provide --base and --head (or set BASE_REF/HEAD_REF), or use --message-file", + ); + exit(1); + } + (None, Some(_)) => { + output::error("Missing --base (or BASE_REF)"); + exit(1); + } + (Some(_), None) => { + output::error("Missing --head (or HEAD_REF)"); + exit(1); + } + }; + + let repo = match git::open_repo(cli.repo.as_deref().unwrap_or(".")) { + Ok(r) => r, + Err(e) => { + output::error(&format!("Failed to open repository: {}", e)); + exit(1); + } + }; + + match git::get_commits_in_range(&repo, &base, &head, cli.skip_merge_commits) { + Ok(commits) if commits.is_empty() => { + output::notice("No commits found in the specified range"); + exit(0); + } + Ok(commits) => commits, + Err(e) => { + output::error(&format!("Failed to get commits: {}", e)); + exit(1); + } } }; - if commits.is_empty() { - output::notice("No commits found in the specified range"); - exit(0); - } - // Display the commits being tested display_commits(&commits); diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 1664210..6610792 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -11,9 +11,10 @@ mod tests { /// Arguments can be omitted (None) to test error cases. fn run_check( check: Option<&str>, - repo_path: &str, + repo_path: Option<&str>, base: Option<&str>, head: Option<&str>, + message_file: Option<&str>, ) -> bool { use assert_cmd::Command; @@ -23,7 +24,9 @@ mod tests { cmd.arg(c); } - cmd.args(["--repo", repo_path]); + if let Some(path) = repo_path { + cmd.args(["--repo", path]); + } if let Some(r) = base { cmd.args(["--base", r]); @@ -31,6 +34,9 @@ mod tests { if let Some(r) = head { cmd.args(["--head", r]); } + if let Some(f) = message_file { + cmd.args(["--message-file", f]); + } cmd.ok().is_ok() } @@ -52,7 +58,13 @@ mod tests { let sha1 = create_commit(&repo_path, message); assert!( - run_check(Some("all"), &repo_path, Some(&base_sha), Some(&sha1)), + run_check( + Some("all"), + Some(&repo_path), + Some(&base_sha), + Some(&sha1), + None + ), "Expected all checks to pass" ); } @@ -72,7 +84,13 @@ mod tests { let sha1 = create_commit(&repo_path, "feat: add feature"); assert!( - !run_check(Some("all"), &repo_path, Some(&base_sha), Some(&sha1)), + !run_check( + Some("all"), + Some(&repo_path), + Some(&base_sha), + Some(&sha1), + None + ), "Expected all checks to fail when one check fails" ); } @@ -91,7 +109,7 @@ mod tests { let _ = create_commit(&repo_path, "initial"); assert!( - !run_check(None, &repo_path, None, Some("abc123")), + !run_check(None, Some(&repo_path), None, Some("abc123"), None), "Expected check to fail without base ref" ); } @@ -108,7 +126,7 @@ mod tests { let _ = create_commit(&repo_path, "initial"); assert!( - !run_check(None, &repo_path, Some("abc123"), None), + !run_check(None, Some(&repo_path), Some("abc123"), None, None), "Expected check to fail without head ref" ); } @@ -125,7 +143,7 @@ mod tests { let base = create_commit(&repo_path, "initial"); assert!( - !run_check(None, &repo_path, Some(&base), Some("invalid")), + !run_check(None, Some(&repo_path), Some(&base), Some("invalid"), None), "Expected check to fail with invalid ref" ); } @@ -146,7 +164,7 @@ mod tests { // Run without explicit subcommand - should still succeed with default "all" assert!( - run_check(None, &repo_path, Some(&base_sha), Some(&sha1)), + run_check(None, Some(&repo_path), Some(&base_sha), Some(&sha1), None), "Expected default to 'all' checks" ); } @@ -167,7 +185,13 @@ mod tests { let sha1 = create_commit(&repo_path, "feat: normal commit"); assert!( - run_check(Some("wip-fixup"), &repo_path, Some(&base_sha), Some(&sha1)), + run_check( + Some("wip-fixup"), + Some(&repo_path), + Some(&base_sha), + Some(&sha1), + None + ), "Expected wip-fixup check to pass for normal commit" ); } @@ -189,9 +213,10 @@ mod tests { assert!( run_check( Some("signed-off-by"), - &repo_path, + Some(&repo_path), Some(&base_sha), - Some(&sha1) + Some(&sha1), + None, ), "Expected signed-off-by check to pass" ); @@ -213,9 +238,10 @@ mod tests { assert!( run_check( Some("conventional-commits"), - &repo_path, + Some(&repo_path), Some(&base_sha), - Some(&sha1) + Some(&sha1), + None, ), "Expected conventional-commits check to pass" ); @@ -237,7 +263,13 @@ mod tests { // Using the same SHA for base and head should result in no commits assert!( - run_check(None, &repo_path, Some(&base_sha), Some(&base_sha)), + run_check( + None, + Some(&repo_path), + Some(&base_sha), + Some(&base_sha), + None + ), "Expected success with empty commit range" ); } @@ -247,9 +279,10 @@ mod tests { assert!( !run_check( None, - "/nonexistent/repo/path", + Some("/nonexistent/repo/path"), Some("abc123"), - Some("def456") + Some("def456"), + None, ), "Expected check to fail with invalid repository path" ); @@ -270,7 +303,13 @@ mod tests { let _sha1 = create_commit(&repo_path, message); assert!( - run_check(Some("all"), &repo_path, Some(&base), Some("HEAD")), + run_check( + Some("all"), + Some(&repo_path), + Some(&base), + Some("HEAD"), + None + ), "Expected check to pass with HEAD reference" ); } @@ -292,8 +331,86 @@ mod tests { let _sha2 = create_commit(&repo_path, message2); assert!( - run_check(Some("all"), &repo_path, Some("HEAD~2"), Some("HEAD")), + run_check( + Some("all"), + Some(&repo_path), + Some("HEAD~2"), + Some("HEAD"), + None + ), "Expected check to pass with HEAD~2 and HEAD references" ); } + + // ===== Message File Tests ===== + + #[test] + fn test_message_file_all_checks_pass() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let file_path = temp_dir.path().join("COMMIT_EDITMSG"); + let message = "feat: add feature\n\nSigned-off-by: Test User "; + std::fs::write(&file_path, message).expect("Failed to write message file"); + + assert!( + run_check( + Some("all"), + None, + None, + None, + Some(file_path.to_str().unwrap()) + ), + "Expected all checks to pass with message file" + ); + } + + #[test] + fn test_message_file_wip_check_fails() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let file_path = temp_dir.path().join("COMMIT_EDITMSG"); + std::fs::write(&file_path, "WIP add feature").expect("Failed to write message file"); + + assert!( + !run_check( + Some("wip-fixup"), + None, + None, + None, + Some(file_path.to_str().unwrap()) + ), + "Expected WIP check to fail" + ); + } + + #[test] + fn test_message_file_nonexistent() { + assert!( + !run_check( + Some("all"), + None, + None, + None, + Some("/nonexistent/path/COMMIT_EDITMSG") + ), + "Expected failure with nonexistent message file" + ); + } + + #[test] + fn test_message_file_empty_is_rejected() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let file_path = temp_dir.path().join("COMMIT_EDITMSG"); + std::fs::write(&file_path, "# only a comment\n\n \n") + .expect("Failed to write message file"); + + assert!( + !run_check( + Some("all"), + None, + None, + None, + Some(file_path.to_str().unwrap()) + ), + "Expected failure with empty commit message" + ); + } }