From aaf4b019af340e6ba1fa34f8f0c98bbaa32bc287 Mon Sep 17 00:00:00 2001 From: Andrei Gherzan Date: Fri, 17 Jul 2026 13:21:49 +0100 Subject: [PATCH 1/2] 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 | 21 +++++ Taskfile.yml | 2 + githooks/commit-msg | 25 ++++++ src/git.rs | 65 ++++++++++++++++ src/main.rs | 103 +++++++++++++------------ tests/integration_tests.rs | 153 ++++++++++++++++++++++++++++++++----- 6 files changed, 302 insertions(+), 67 deletions(-) create mode 100755 githooks/commit-msg diff --git a/README.md b/README.md index 0e36ff8..894a305 100644 --- a/README.md +++ b/README.md @@ -224,6 +224,27 @@ 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: + +```bash +gitlance --message-file .git/COMMIT_EDITMSG +``` + +A specific check can also be selected: + +```bash +gitlance conventional-commits --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" + ); + } } From bfdffb4d789eaf9280f81257911040fda8ddddc5 Mon Sep 17 00:00:00 2001 From: Andrei Gherzan Date: Wed, 22 Jul 2026 23:35:40 +0100 Subject: [PATCH 2/2] feat: validate only commits not yet on any remote When pushing a new branch, the pre-push hook derived its base from a single remote's HEAD. If that remote was behind (e.g. a fork whose main is a stale copy of upstream), it re-validated commits already reviewed and merged elsewhere, and could reject a push over commits the author never touched. Base validation on every remote-tracking ref instead: a commit already present on any remote has been published and does not need rechecking. Excluding a set of refs rather than a single base also lets the root commit be validated, which a single exclusive base can never reach. Closes: #10 Signed-off-by: Andrei Gherzan --- README.md | 11 +++ githooks/pre-push | 41 +++++---- src/git.rs | 165 ++++++++++++++++++++++++++++++++++--- src/lib.rs | 4 +- src/main.rs | 49 ++++++++++- tests/integration_tests.rs | 92 +++++++++++++++++++-- 6 files changed, 324 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 894a305..0911ae3 100644 --- a/README.md +++ b/README.md @@ -245,6 +245,17 @@ A specific check can also be selected: gitlance conventional-commits --message-file .git/COMMIT_EDITMSG ``` +### Validating Unpublished Commits + +With `--not-on-remotes`, gitlance validates only the commits reachable from +`--head` that are not yet present on any remote. This makes it convenient to use +in a local `pre-push` git hook. It exits successfully when there is nothing new +to check: + +```bash +gitlance --head --not-on-remotes +``` + ## Installation ### GitHub Action diff --git a/githooks/pre-push b/githooks/pre-push index 5c5996b..d49ab5a 100755 --- a/githooks/pre-push +++ b/githooks/pre-push @@ -34,35 +34,34 @@ do continue fi - # Determine base commit + # Run gitlance + echo "Checking git commit logs with gitlance:" + echo if [ "$remote_oid" = "0000000000000000000000000000000000000000" ]; then - # New branch - try to find common ancestor with remote HEAD - base="" - if remote_head=$(git ls-remote "$remote" HEAD 2>/dev/null | awk '{print $1}') && [ -n "$remote_head" ]; then - base=$(git merge-base "$remote_head" "$local_oid" 2>/dev/null) || true - fi - # Fall back to root commit if no common ancestor found - if [ -z "$base" ]; then - base=$(git rev-list --max-parents=0 "$local_oid" | head -1) + # New branch: validate commits not yet present on any remote. gitlance + # derives the set from remote-tracking refs, which is more reliable than + # a single remote's HEAD (a stale remote would re-check known commits). + if ! gitlance --head "$local_oid" --not-on-remotes; then + echo "" + echo "Push rejected: Commit validation failed" + echo "To fix: git commit --amend, or git rebase -i" + echo "To bypass: git push --no-verify" + exit 1 fi else - # Existing branch - find common ancestor to only check divergent commits + # Existing branch: check only commits that diverge from the remote. if ! base=$(git merge-base "$remote_oid" "$local_oid"); then echo "Error: Failed to find merge-base between remote and local commits" echo "To bypass: git push --no-verify" exit 1 fi - fi - - # Run gitlance - echo "Checking git commit logs with gitlance:" - echo - if ! gitlance --base "$base" --head "$local_oid"; then - echo "" - echo "Push rejected: Commit validation failed" - echo "To fix: git commit --amend, or git rebase -i $base" - echo "To bypass: git push --no-verify" - exit 1 + if ! gitlance --base "$base" --head "$local_oid"; then + echo "" + echo "Push rejected: Commit validation failed" + echo "To fix: git commit --amend, or git rebase -i $base" + echo "To bypass: git push --no-verify" + exit 1 + fi fi done diff --git a/src/git.rs b/src/git.rs index cfb1445..d2b8806 100644 --- a/src/git.rs +++ b/src/git.rs @@ -63,21 +63,46 @@ pub fn resolve_ref(repo: &Repository, refspec: &str) -> Result }) } -/// Gets all commits in the range [base, head] -/// Returns commits from base (exclusive) to head (inclusive) +/// Lists all remote-tracking refs (`refs/remotes/*`) in the repository. /// -/// Accepts any valid git revision specification for base and head: +/// These represent everything already pushed to or fetched from any remote, +/// and are used to determine which commits are genuinely new. +pub fn remote_tracking_refs(repo: &Repository) -> Result, CheckError> { + let refs = repo + .references_glob("refs/remotes/*") + .map_err(|e| CheckError::Git(format!("Failed to list remote-tracking refs: {}", e)))?; + + let mut names = Vec::new(); + for reference in refs { + let reference = reference + .map_err(|e| CheckError::Git(format!("Failed to read remote-tracking ref: {}", e)))?; + let name = reference + .name() + .map_err(|e| CheckError::Git(format!("Remote-tracking ref has invalid name: {}", e)))?; + names.push(name.to_string()); + } + + Ok(names) +} + +/// Gets all commits reachable from `head` but not from any of `excludes`. +/// +/// This is the core commit-selection routine. Each exclude ref is hidden from +/// the revwalk, so the result contains only commits unique to `head`. Passing a +/// single base gives a `base..head` range; passing all remote-tracking refs +/// gives the commits that are not yet on any remote. +/// +/// Accepts any valid git revision specification for `head` and each exclude: /// - Full/short SHAs, branches, tags, HEAD~n, etc. /// /// If `skip_merge_commits` is true, merge commits (commits with more than one parent) /// are excluded from the results. -pub fn get_commits_in_range( +pub fn get_commits_excluding( repo: &Repository, - base: &str, head: &str, + excludes: &[impl AsRef], skip_merge_commits: bool, ) -> Result, CheckError> { - let base_oid = resolve_ref(repo, base)?; let head_oid = resolve_ref(repo, head)?; let mut revwalk = repo @@ -89,10 +114,14 @@ pub fn get_commits_in_range( .push(head_oid) .map_err(|e| CheckError::Git(format!("Failed to push head to revwalk: {}", e)))?; - // Don't include the base commit itself - revwalk - .hide(base_oid) - .map_err(|e| CheckError::Git(format!("Failed to hide base in revwalk: {}", e)))?; + // Hide each exclude ref so its commits (and their ancestors) are omitted + for exclude in excludes { + let exclude = exclude.as_ref(); + let exclude_oid = resolve_ref(repo, exclude)?; + revwalk.hide(exclude_oid).map_err(|e| { + CheckError::Git(format!("Failed to hide '{}' in revwalk: {}", exclude, e)) + })?; + } let mut commits = Vec::new(); @@ -121,6 +150,23 @@ pub fn get_commits_in_range( Ok(commits) } +/// Gets all commits in the range [base, head] +/// Returns commits from base (exclusive) to head (inclusive) +/// +/// Accepts any valid git revision specification for base and head: +/// - Full/short SHAs, branches, tags, HEAD~n, etc. +/// +/// If `skip_merge_commits` is true, merge commits (commits with more than one parent) +/// are excluded from the results. +pub fn get_commits_in_range( + repo: &Repository, + base: &str, + head: &str, + skip_merge_commits: bool, +) -> Result, CheckError> { + get_commits_excluding(repo, head, &[base], skip_merge_commits) +} + #[cfg(test)] mod tests { use super::*; @@ -442,4 +488,103 @@ mod tests { let commit_clone = commit.clone(); assert_eq!(commit_clone.sha, commit.sha); } + + #[test] + fn test_remote_tracking_refs_lists_remote_refs() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_repo( + temp_dir + .path() + .to_str() + .expect("Failed to convert temp dir path to string"), + ); + let repo = open_repo(&repo_path).expect("Failed to open repo"); + + let base_sha = create_commit(&repo_path, "initial"); + + // Simulate a fetched remote-tracking ref by writing it directly. + run_cmd( + &repo_path, + "git", + &["update-ref", "refs/remotes/origin/main", &base_sha], + ); + + let refs = remote_tracking_refs(&repo).expect("Failed to list remote-tracking refs"); + assert_eq!(refs, vec!["refs/remotes/origin/main".to_string()]); + } + + #[test] + fn test_get_commits_excluding_empty_excludes_includes_root() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_repo( + temp_dir + .path() + .to_str() + .expect("Failed to convert temp dir path to string"), + ); + let repo = open_repo(&repo_path).expect("Failed to open repo"); + + let _base_sha = create_commit(&repo_path, "initial"); + let _sha1 = create_commit(&repo_path, "second commit"); + + // With no excludes, every commit reachable from head is returned, + // including the root commit that has no parent. + let commits = get_commits_excluding(&repo, "HEAD", &[] as &[&str], false) + .expect("Failed to get commits"); + + assert_eq!(commits.len(), 2); + assert_eq!(commits[0].message.trim(), "second commit"); + assert_eq!(commits[1].message.trim(), "initial"); + } + + #[test] + fn test_get_commits_excluding_head_fully_excluded_is_empty() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_repo( + temp_dir + .path() + .to_str() + .expect("Failed to convert temp dir path to string"), + ); + let repo = open_repo(&repo_path).expect("Failed to open repo"); + + let _base_sha = create_commit(&repo_path, "initial"); + let _sha1 = create_commit(&repo_path, "second commit"); + + // Excluding head itself leaves nothing new. + let commits = + get_commits_excluding(&repo, "HEAD", &["HEAD"], false).expect("Failed to get commits"); + + assert!(commits.is_empty()); + } + + #[test] + fn test_get_commits_excluding_via_remote_tracking_refs() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_repo( + temp_dir + .path() + .to_str() + .expect("Failed to convert temp dir path to string"), + ); + let repo = open_repo(&repo_path).expect("Failed to open repo"); + + let _base_sha = create_commit(&repo_path, "initial"); + let sha1 = create_commit(&repo_path, "second commit"); + let _sha2 = create_commit(&repo_path, "third commit"); + + // Pretend the first two commits are already published on a remote. + run_cmd( + &repo_path, + "git", + &["update-ref", "refs/remotes/origin/main", &sha1], + ); + + let excludes = remote_tracking_refs(&repo).expect("Failed to list remote-tracking refs"); + let commits = + get_commits_excluding(&repo, "HEAD", &excludes, false).expect("Failed to get commits"); + + assert_eq!(commits.len(), 1); + assert_eq!(commits[0].message.trim(), "third commit"); + } } diff --git a/src/lib.rs b/src/lib.rs index 449d0f4..027d5c4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,7 +11,9 @@ pub mod output; pub mod test_utils; pub use error::CheckError; -pub use git::{get_commits_in_range, open_repo, resolve_ref}; +pub use git::{ + get_commits_excluding, get_commits_in_range, open_repo, remote_tracking_refs, resolve_ref, +}; /// Length to abbreviate SHAs in output messages const SHA_ABBREV_LEN: usize = 8; diff --git a/src/main.rs b/src/main.rs index d88b239..954f571 100644 --- a/src/main.rs +++ b/src/main.rs @@ -29,8 +29,13 @@ struct Cli { #[arg(long, global = true)] skip_merge_commits: bool, + /// Validate only commits not yet present on any remote (requires --head, + /// mutually exclusive with --base). Exits successfully when nothing is new. + #[arg(long, global = true, conflicts_with = "base")] + not_on_remotes: 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"])] + #[arg(long, global = true, conflicts_with_all = ["base", "head", "skip_merge_commits", "repo", "not_on_remotes"])] message_file: Option, } @@ -78,6 +83,48 @@ fn main() { exit(1); } } + } else if cli.not_on_remotes { + // Validate only commits reachable from head but not from any remote. + let head = cli.head.or_else(|| std::env::var("HEAD_REF").ok()); + + let head = match head { + Some(h) => h, + 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); + } + }; + + let excludes = match git::remote_tracking_refs(&repo) { + Ok(refs) => refs, + Err(e) => { + output::error(&format!("Failed to list remote-tracking refs: {}", e)); + exit(1); + } + }; + + match git::get_commits_excluding(&repo, &head, &excludes, cli.skip_merge_commits) { + // An empty result means every commit is already on a remote, so + // there is nothing new to validate. This is a clean pass, not an + // error: the revwalk semantics make emptiness a precise signal. + Ok(commits) if commits.is_empty() => { + println!("No new commits to check."); + exit(0); + } + 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()); diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 6610792..13797e3 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -7,15 +7,16 @@ mod tests { use gitlance::test_utils::*; use tempfile::TempDir; - /// Runs the binary with specific check and arguments (for integration testing). - /// Arguments can be omitted (None) to test error cases. - fn run_check( + /// Builds and runs the binary with the given arguments, returning the raw + /// process output. Any argument can be omitted (None) to test error cases. + fn run( check: Option<&str>, repo_path: Option<&str>, base: Option<&str>, head: Option<&str>, message_file: Option<&str>, - ) -> bool { + not_on_remotes: bool, + ) -> std::process::Output { use assert_cmd::Command; let mut cmd = Command::cargo_bin("gitlance").expect("Failed to find binary"); @@ -37,8 +38,25 @@ mod tests { if let Some(f) = message_file { cmd.args(["--message-file", f]); } + if not_on_remotes { + cmd.arg("--not-on-remotes"); + } - cmd.ok().is_ok() + cmd.output().expect("Failed to run binary") + } + + /// Runs the binary and reports whether it exited successfully. + /// Arguments can be omitted (None) to test error cases. + fn run_check( + check: Option<&str>, + repo_path: Option<&str>, + base: Option<&str>, + head: Option<&str>, + message_file: Option<&str>, + ) -> bool { + run(check, repo_path, base, head, message_file, false) + .status + .success() } // ===== All Checks Tests ===== @@ -342,6 +360,70 @@ mod tests { ); } + // ===== Not-on-remotes Tests ===== + + #[test] + fn test_not_on_remotes_checks_only_unpublished_commits() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_repo( + temp_dir + .path() + .to_str() + .expect("Failed to convert temp dir path to string"), + ); + + let published = create_commit(&repo_path, "chore: initial"); + let message = "feat: add feature\n\nSigned-off-by: Test User "; + let _new = create_commit(&repo_path, message); + + // Mark the first commit as already present on a remote. + run_cmd( + &repo_path, + "git", + &["update-ref", "refs/remotes/origin/main", &published], + ); + + let output = run(None, Some(&repo_path), None, Some("HEAD"), None, true); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert!(output.status.success(), "Expected checks to pass"); + assert!( + stdout.contains("Testing 1 commit"), + "Expected only the unpublished commit to be checked, got: {}", + stdout + ); + } + + #[test] + fn test_not_on_remotes_passes_when_nothing_new() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_repo( + temp_dir + .path() + .to_str() + .expect("Failed to convert temp dir path to string"), + ); + + let head = create_commit(&repo_path, "chore: initial"); + + // Every commit is already on a remote. + run_cmd( + &repo_path, + "git", + &["update-ref", "refs/remotes/origin/main", &head], + ); + + let output = run(None, Some(&repo_path), None, Some("HEAD"), None, true); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert!(output.status.success(), "Expected a clean pass"); + assert!( + stdout.contains("No new commits to check"), + "Expected clean-pass message, got: {}", + stdout + ); + } + // ===== Message File Tests ===== #[test]