Add configurable skipMergeCommits parameter to Git info command#1381
Add configurable skipMergeCommits parameter to Git info command#1381GabriRuflex wants to merge 3 commits into
Conversation
b5d14bc to
d12b21d
Compare
Hey @slachiewicz could you please take a look to this PR? |
There was a problem hiding this comment.
Pull request overview
Adds a new skipMergeCommits command parameter to make Git info revision selection configurable, addressing cases where always using --no-merges causes the reported revision to differ from HEAD in merge-based workflows.
Changes:
- Introduces new public
CommandParameter.SCM_SKIP_MERGE_COMMITS(defaulting totruefor backward compatibility). - Updates gitexe
GitInfoCommandto conditionally add--no-mergesbased on the parameter. - Updates JGit
JGitInfoCommandto apply merge-skipping consistently (including per-path lookups) and adds provider-level tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/test/java/org/apache/maven/scm/provider/git/jgit/command/info/JGitInfoCommandTest.java | Adds unit tests for default merge-skipping and opt-in merge inclusion behavior. |
| maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/info/JGitInfoCommand.java | Adds parameter handling to optionally apply RevFilter.NO_MERGES and align behavior with gitexe. |
| maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gitexe/src/test/java/org/apache/maven/scm/provider/git/gitexe/command/info/GitInfoCommandTest.java | Adds assertions verifying --no-merges default and its absence when disabled. |
| maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gitexe/src/main/java/org/apache/maven/scm/provider/git/gitexe/command/info/GitInfoCommand.java | Conditionally adds --no-merges based on the new parameter while preserving default behavior. |
| maven-scm-api/src/main/java/org/apache/maven/scm/CommandParameter.java | Adds the new SCM_SKIP_MERGE_COMMITS public parameter definition (@since 2.2.2). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
All fixed and tested |
|
@slachiewicz The Copilot review has been completed and all its remarks have been addressed. While the scope of this enhancement is intentionally limited, it is the prerequisite for fixing mojohaus/buildnumber-maven-plugin#229, where the --no-merges behavior introduced in #193 causes the plugin to generate a revision that no longer matches the actual SCM version for workflows relying on merge commits. Making this behavior configurable was also suggested by @kwin in that issue. I'd appreciate a review when someone has a chance. |
The Git "info" command always passed --no-merges to git log, so the reported revision could differ from the actual HEAD in merge-based release workflows (regression surfaced in buildnumber-maven-plugin#229). Introduce CommandParameter.SCM_SKIP_MERGE_COMMITS (default true, fully backward compatible) to make this behavior configurable. When set to false, merge commits are included so the reported revision matches HEAD. Both providers honor the flag: gitexe adds --no-merges conditionally, and the JGit provider now applies RevFilter.NO_MERGES (previously it never filtered merge commits, diverging from gitexe). Fixes apache#1327
63a7acc to
ab81a46
Compare
|
@kwin Done — the behavioral tests now live in GitInfoCommandTckTest so they run identically for both gitexe and JGit (the provider-specific tests were removed). |
What does this PR do?
The Git
infocommand always passed--no-mergestogit log, so the reported revision could differfrom the actual
HEADin merge-based release workflows. This surfaced downstream as a regression inbuildnumber-maven-plugin#229 (which
switched from
git rev-parse --verify HEADtogit log -1 --no-merges).As requested in #1327, this makes the behavior configurable via a new
CommandParameter.SCM_SKIP_MERGE_COMMITSparameter ("skipMergeCommits"):true— skip merge commits (current behavior, fully backward compatible).falseto include merge commits, so the reported revision matches the actualHEAD.Changes
maven-scm-api— new publicCommandParameter.SCM_SKIP_MERGE_COMMITS(@since 2.2.2).maven-scm-provider-gitexe—GitInfoCommandadds--no-mergesonly when the flag istrue(null-safe reader mirroring the existing
shortRevisionLengthhandling, plus aDEFAULT_SKIP_MERGE_COMMITSconstant).maven-scm-provider-jgit—JGitInfoCommandnow honors the same flag by applyingRevFilter.NO_MERGESfor both theHEADand the per-file lookups. Previously the JGit providernever filtered merge commits, diverging from gitexe; it is now consistent with it (with the
default
true, JGit skips merge commits as gitexe already did).Tests
GitInfoCommandTest(gitexe): asserts--no-mergesis present by default and absent whenskipMergeCommits=false.JGitInfoCommandTest: builds a repository whoseHEADis a no-fast-forward merge commit andverifies that the merge commit is reported when
skipMergeCommits=falseand skipped by default.nullparameters) keep passing, confirming thedefault behavior is unchanged for both providers.
Notes
This is the enabling change on the maven-scm side. To fully address buildnumber-maven-plugin#229, the
plugin will need to expose an option that sets
SCM_SKIP_MERGE_COMMITS = falseon theCommandParametersit passes toScmProvider#info.Following this checklist to help incorporate the contribution quickly and easily:
mvn verifyto make sure basic checks pass.mvn -Prun-its verify).Fixes #1327