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

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 <andrei.gherzan@canonical.com>

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: canonical#10 Signed-off-by: Andrei Gherzan <andrei.gherzan@canonical.com>

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:40:05Z

+        stages: [commit-msg]
+      - id: gitlance-pre-push
+        name: Validate commits with gitlance
+        entry: gitlance --head HEAD --not-on-remotes


This is fair. Same for the above.

Good catch. I replaced the hardcoded HEAD with a small pre-commit adapter that reads PRE_COMMIT_TO_REF, falling back to PRE_COMMIT_LOCAL_BRANCH or HEAD, and skips non-branch refs. This handles git push origin otherbranch without changing the Gitlance Rust code. I validated the non-checked-out branch path, tag skipping, and the installed pre-push hook.

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:40:05Z

+        stages: [commit-msg]
+      - id: gitlance-pre-push
+        name: Validate commits with gitlance
+        entry: gitlance --head HEAD --not-on-remotes


This is fair. Same for the above.

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.

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.

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.

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>

agherzan added 2 commits July 23, 2026 01:03

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

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

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dhruvxvaishnav commented Aug 11, 2026 •

edited

Loading

dhruvxvaishnav commented Aug 11, 2026 •

edited

Loading