fix: integrate git hooks with pre-commit by dhruvxvaishnav · Pull Request #17 · canonical/gitlance

dhruvxvaishnav · 2026-08-11T11:24:20Z

Closes #9

Summary

Copying commit-msg and pre-push directly into .git/hooks can replace hooks that contributors already use.

This branch is based on #13, which includes the commit-message support from #12. The change on top is limited to pre-commit integration:

reuse the existing commit-msg hook through pre-commit
use a small pre-push adapter with the existing --not-on-remotes mode
select the pushed ref from PRE_COMMIT_TO_REF or PRE_COMMIT_LOCAL_BRANCH
correctly handle non-checked-out branches and skip non-branch refs
use pre-commit install from task install-githooks
preserve existing hooks through pre-commit's legacy-hook support
document installation and uninstall behavior

The Rust source changes displayed in the combined PR diff are inherited from #12 and #13. This contribution commit does not modify Gitlance's Rust code.

Testing

cargo fmt --all -- --check
cargo clippy --locked --all-targets --all-features -- -D warnings
cargo test --locked — 47 unit and 19 integration tests passed
reuse lint
cargo llvm-cov --locked report --fail-under-lines 70 — 85.12% line coverage
pre-commit validate-config .pre-commit-config.yaml
repeated pre-commit install
actual existing commit-msg and configured pre-push hook runs
non-checked-out branch selection
non-branch ref skipping

agherzan

Most of the implementation scope is already covered in a PR. The missing bit was only the pre-commit integration. Can you validate it by having only those changes on top of #12 and #13? Also, keep the git log as informative as the PR description as that is what we are ending up with in git history.

dhruvxvaishnav · 2026-08-11T11:53:09Z

Thanks for the feedback @agherzan. I’ve updated the branch on top of #13, which includes #12, and limited my commit to the pre-commit integration. I also expanded the commit message to include the implementation rationale and validation details. All checks are passing locally. Ready for another review when you have time.

agherzan

Any reason not use the pre-commit install directly? I'm not sure I understand why you need the changes in gitlance.

Some small extra comments:

keep git log lines at resonable lengths.
SOB should have your name and email

dhruvxvaishnav · 2026-08-11T12:18:27Z

Thanks, that makes sense. I added the Gitlance changes only to translate pre-commit's pre-push environment variables, but after rechecking #12 and #13, I agree this can be simpler. I'll use pre-commit install directly and configure it to call the existing --message-file and --not-on-remotes functionality, then remove the Gitlance source changes.

I'll also wrap the commit message at reasonable line lengths and update the author and sign-off to:
Dhruv Vaishnav <dhruvvaishav687@gmail.com>

dhruvxvaishnav · 2026-08-11T12:24:26Z

Updated as discussed. The contribution commit now uses pre-commit install directly and only changes .pre-commit-config.yaml, Taskfile.yml, and CONTRIBUTING.md; there are no Gitlance source or test changes on top of #12/#13.

I also wrapped the commit message at 72 columns and updated both the author and sign-off to Dhruv Vaishnav <dhruvvaishav687@gmail.com>. Both configured hook stages and the full validation suite pass locally.

Copilot

Pull request overview

This PR introduces pre-commit–based installation and configuration for local Git hooks, and extends the gitlance CLI to support commit-msg style validation (message-file mode) and pre-push style validation of only unpublished commits (not-on-remotes mode).

Changes:

Add --message-file and --not-on-remotes CLI modes and supporting git utilities (remote_tracking_refs, get_commits_excluding).
Add integration/units tests covering message-file mode and not-on-remotes commit selection.
Add pre-commit configuration + Taskfile/docs updates to install hooks via pre-commit install and document install/uninstall behavior.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/integration_tests.rs	Refactors test harness and adds integration coverage for `--message-file` and `--not-on-remotes`.
Taskfile.yml	Switches `install-githooks` to `pre-commit install` with preconditions.
src/main.rs	Adds CLI flags and implements message-file + not-on-remotes commit selection flow.
src/lib.rs	Re-exports new git APIs used by the CLI and other crates.
src/git.rs	Adds commit creation from message files, comment stripping, remote-tracking ref listing, and exclude-based revwalk selection.
README.md	Documents direct CLI usage for message-file and not-on-remotes modes.
githooks/pre-push	Updates hook behavior for new-branch pushes to use not-on-remotes logic.
githooks/commit-msg	Adds a standalone commit-msg hook script invoking `--message-file`.
CONTRIBUTING.md	Documents local hook installation via pre-commit and uninstall behavior.
.pre-commit-config.yaml	Adds pre-commit hooks for commit-msg and pre-push stages using gitlance.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

agherzan · 2026-08-11T12:39:34Z

+    hooks:
+      - id: gitlance-commit-msg
+        name: Validate commit message with gitlance
+        entry: gitlance --message-file


I would use the existing hook. Any disadvantages to that?

No functional disadvantage here. Pre-commit passes the commit-message filename as the first argument, which matches what the existing githooks/commit-msg script expects. I used the binary directly to avoid an extra shell layer, but reusing the existing hook is clearer and avoids duplicating its behavior. I'll switch this entry to the existing hook.

agherzan · 2026-08-11T12:41:02Z

+        entry: gitlance --head HEAD --not-on-remotes
+        language: system
+        stages: [pre-push]
+        always_run: true


Could you explain why you added this?

Gitlance validates commits rather than files, so this keeps the hook running for pushes with no changed files, such as an empty commit.

Gitlance supports now validating files as well. If you mean a git log entry in a file. Did you find a limitation?

agherzan · 2026-08-11T12:41:44Z

+        language: system
+        stages: [pre-push]
+        always_run: true
+        pass_filenames: false


Could you explain why you added this?

The adapter gets the pushed ref from pre-commit's environment. Changed filenames are not inputs to Gitlance, so they should not be passed.

I'm not sure I understand. Why is that different in the other hook?

agherzan · 2026-08-11T12:42:39Z

+    preconditions:
+      - sh: pre-commit --version
+        msg: "pre-commit is required: https://pre-commit.com/#install"
+      - sh: gitlance --not-on-remotes --head HEAD --help


Help should be enough.

Agreed, gitlance --help is enough. I'll simplify it.

agherzan

I have merged the PRs that you use here. Could you please rebase it?

Copying commit-msg and pre-push directly into .git/hooks can replace hooks that contributors already use. Configure both stages through pre-commit and install its dispatcher from the existing task. Pre-commit preserves prior hooks as legacy hooks, runs them alongside Gitlance, and keeps repeated installation idempotent. Reuse the commit-msg hook and the --not-on-remotes mode added by canonical#12 and canonical#13. Pre-commit passes the prepared message file to the existing hook. For pre-push, use a small adapter to select the pushed ref from PRE_COMMIT_TO_REF or PRE_COMMIT_LOCAL_BRANCH. This validates a branch that is not checked out and skips non-branch refs without changing the Gitlance CLI. Document the local setup and uninstall workflow. Closes: canonical#9 Signed-off-by: Dhruv Vaishnav <dhruvvaishav687@gmail.com>

dhruvxvaishnav · 2026-08-20T05:00:56Z

Thanks @agherzan. I’ve rebased the branch onto the current main. The PR now contains only the pre-commit integration commit on top of the merged #12 and #13 changes, and GitHub reports the branch as mergeable.

agherzan · 2026-08-22T09:23:29Z

+task install-githooks
+```
+
+The installation is idempotent. If a `commit-msg` or `pre-push` hook already


How does it behave when installing the gitlance hooks after they were updated locally? Does it update the hooks? Does it ignore the updates as it already has them?

agherzan · 2026-08-22T09:26:59Z

@@ -0,0 +1,24 @@
+#!/usr/bin/env python3


Gitlance supports taking advantage of Git's commit file, which is exposed to hooks before creating the commit. Does this script fill any gap in that implementation?

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

.pre-commit-config.yaml:17

The pre-commit pre-push implementation consumes the Git stdin stream and exposes only one non-deleted ref through PRE_COMMIT_TO_REF/PRE_COMMIT_LOCAL_BRANCH. This adapter therefore validates at most one ref from a multi-ref push; if the first ref is a tag it returns success here and any later branch ref is never checked. The pre-commit integration needs a way to process every pushed ref, or this hook must be installed outside the single-ref pre-commit adapter.

        entry: python githooks/pre_commit_pre_push.py

Taskfile.yml:30

pre-commit install does not replace an existing non-pre-commit hook unless overwrite is requested, so this task will fail on exactly the repositories with an existing commit-msg or pre-push hook instead of moving it to the legacy hook and chaining it. Invoke the legacy-hook installation path with pre-commit install --overwrite so the documented preservation behavior actually occurs.

      - pre-commit install

Taskfile.yml:30

pre-commit install refuses to run when Git has core.hooksPath configured. The previous task resolved the effective hooks directory with git rev-parse --git-path hooks, so this change makes task install-githooks fail for contributors who use a custom hooks path instead of installing the requested hooks. Handle or document this case explicitly, rather than silently regressing the existing path support.

      - pre-commit install

githooks/pre_commit_pre_push.py:20

Unlike the existing githooks/commit-msg and githooks/pre-push scripts, this adapter does not handle a missing gitlance executable. If the binary is removed or the hook is run from an environment without Cargo's bin directory, subprocess.call raises FileNotFoundError and pre-commit prints a traceback instead of the actionable failure/bypass message. Catch this case and return a normal nonzero hook result.

    return subprocess.call(
        ["gitlance", "--head", head, "--not-on-remotes"],
    )

githooks/pre_commit_pre_push.py:15

Allowing the literal HEAD bypasses the non-branch filter for an explicit tag push such as git push origin HEAD:refs/tags/v1: pre-commit reports the local ref as HEAD, so this adapter invokes Gitlance instead of skipping the tag. Check PRE_COMMIT_REMOTE_BRANCH as well (or otherwise distinguish HEAD pushed to a tag) before accepting the ref.

    if local_branch != "HEAD" and not local_branch.startswith("refs/heads/"):
        return 0

+    head = os.environ.get("PRE_COMMIT_TO_REF") or local_branch
+    return subprocess.call(
+        ["gitlance", "--head", head, "--not-on-remotes"],


agherzan requested changes Aug 11, 2026

View reviewed changes

dhruvxvaishnav force-pushed the fix/flexible-pre-push-hooks branch from 8d6cf24 to 4f9098f Compare August 11, 2026 11:43

dhruvxvaishnav changed the title ~~fix: manage pre-push hooks with pre-commit~~ fix: integrate git hooks with pre-commit Aug 11, 2026

agherzan requested changes Aug 11, 2026

View reviewed changes

dhruvxvaishnav force-pushed the fix/flexible-pre-push-hooks branch from 4f9098f to ee632a6 Compare August 11, 2026 12:23

agherzan requested a lite review from Copilot August 11, 2026 12:26

Copilot started reviewing on behalf of agherzan August 11, 2026 12:27 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

Comment thread .pre-commit-config.yaml Outdated

Comment thread src/main.rs

Comment thread src/git.rs

Comment thread src/main.rs

dhruvxvaishnav force-pushed the fix/flexible-pre-push-hooks branch from ee632a6 to 266711e Compare August 11, 2026 12:40

agherzan requested changes Aug 11, 2026

View reviewed changes

dhruvxvaishnav force-pushed the fix/flexible-pre-push-hooks branch from 266711e to 1e94c46 Compare August 11, 2026 12:50

agherzan requested a review from panasheMuriro August 18, 2026 12:20

agherzan reviewed Aug 19, 2026

View reviewed changes

agherzan self-assigned this Aug 19, 2026

agherzan added the enhancement New feature or request label Aug 19, 2026

dhruvxvaishnav force-pushed the fix/flexible-pre-push-hooks branch from 1e94c46 to 288c1df Compare August 20, 2026 04:58

agherzan reviewed Aug 22, 2026

View reviewed changes

agherzan requested a lite review from Copilot August 22, 2026 09:28

Copilot started reviewing on behalf of agherzan August 22, 2026 09:28 View session

Copilot AI reviewed Aug 22, 2026

View reviewed changes

Comment thread githooks/pre_commit_pre_push.py

Comment on lines +17 to +19

head = os.environ.get("PRE_COMMIT_TO_REF") or local_branch

return subprocess.call(

["gitlance", "--head", head, "--not-on-remotes"],

Uh oh!

Conversation

dhruvxvaishnav commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

agherzan left a comment

Choose a reason for hiding this comment

Uh oh!

dhruvxvaishnav commented Aug 11, 2026

Uh oh!

agherzan left a comment

Choose a reason for hiding this comment

Uh oh!

dhruvxvaishnav commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhruvxvaishnav commented Aug 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agherzan left a comment

Choose a reason for hiding this comment

Uh oh!

dhruvxvaishnav commented Aug 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dhruvxvaishnav commented Aug 11, 2026 •

edited

Loading

dhruvxvaishnav commented Aug 11, 2026 •

edited

Loading