Add configurable skipMergeCommits parameter to Git info command#1381
Open
GabriRuflex wants to merge 2 commits into
Open
Add configurable skipMergeCommits parameter to Git info command#1381GabriRuflex wants to merge 2 commits into
GabriRuflex wants to merge 2 commits into
Conversation
265d31c to
b5d14bc
Compare
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
b5d14bc to
d12b21d
Compare
Author
Author
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.
Author
|
All fixed and tested |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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