Skip to content

Commit d12b21d

Browse files
committed
Add skipMergeCommits parameter to Git info command
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 #1327
1 parent 184ae7f commit d12b21d

5 files changed

Lines changed: 249 additions & 9 deletions

File tree

maven-scm-api/src/main/java/org/apache/maven/scm/CommandParameter.java

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,17 @@ public class CommandParameter implements Serializable {
7979
*/
8080
public static final CommandParameter SCM_SHORT_REVISION_LENGTH = new CommandParameter("shortRevisionLength");
8181

82+
/**
83+
* Parameter used only for the Git SCM {@code info} command to control whether merge commits are skipped
84+
* (adds <code>--no-merges</code> to the underlying <code>git log</code> invocation).
85+
* Defaults to {@code true} (skip merge commits) to keep backward compatibility. Set to {@code false} to
86+
* include merge commits, e.g. so that the reported revision matches the actual {@code HEAD} in merge-based
87+
* release workflows.
88+
*
89+
* @since 2.2.2
90+
*/
91+
public static final CommandParameter SCM_SKIP_MERGE_COMMITS = new CommandParameter("skipMergeCommits");
92+
8293
/**
8394
* Parameter to force add.
8495
*

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

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,20 @@ public class GitInfoCommand extends AbstractCommand implements GitCommand {
4747

4848
public static final int NO_REVISION_LENGTH = -1;
4949

50+
/** Default value applied when the {@link CommandParameter#SCM_SKIP_MERGE_COMMITS} parameter is absent. */
51+
public static final boolean DEFAULT_SKIP_MERGE_COMMITS = true;
52+
5053
@Override
5154
protected ScmResult executeCommand(
5255
ScmProviderRepository repository, ScmFileSet fileSet, CommandParameters parameters) throws ScmException {
5356

57+
boolean skipMergeCommits = isSkipMergeCommits(parameters);
58+
5459
Commandline baseCli = GitCommandLineUtils.getBaseGitCommandLine(fileSet.getBasedir(), "log");
5560
baseCli.createArg().setValue("-1"); // only most recent commit matters
56-
baseCli.createArg().setValue("--no-merges"); // skip merge commits
61+
if (skipMergeCommits) {
62+
baseCli.createArg().setValue("--no-merges"); // skip merge commits
63+
}
5764
baseCli.addArg(GitInfoConsumer.getFormatArgument());
5865

5966
List<InfoItem> infoItems = new LinkedList<>();
@@ -64,7 +71,9 @@ protected ScmResult executeCommand(
6471
for (File scmFile : fileSet.getFileList()) {
6572
baseCli = GitCommandLineUtils.getBaseGitCommandLine(fileSet.getBasedir(), "log");
6673
baseCli.createArg().setValue("-1"); // only most recent commit matters
67-
baseCli.createArg().setValue("--no-merges"); // skip merge commits
74+
if (skipMergeCommits) {
75+
baseCli.createArg().setValue("--no-merges"); // skip merge commits
76+
}
6877
baseCli.addArg(GitInfoConsumer.getFormatArgument());
6978
// Insert a separator to make sure that files aren't interpreted as part of the version spec
7079
baseCli.createArg().setValue("--");
@@ -102,4 +111,21 @@ private static int getRevisionLength(final CommandParameters parameters) throws
102111
return parameters.getInt(CommandParameter.SCM_SHORT_REVISION_LENGTH, NO_REVISION_LENGTH);
103112
}
104113
}
114+
115+
/**
116+
* Whether merge commits should be skipped (i.e. whether {@code --no-merges} should be added).
117+
*
118+
* @param parameters the command parameters
119+
* @return {@link #DEFAULT_SKIP_MERGE_COMMITS} if parameter {@link CommandParameter#SCM_SKIP_MERGE_COMMITS}
120+
* (or the whole {@code parameters}) is absent, and otherwise the requested value
121+
* @throws ScmException if the parameter has the wrong type
122+
* @since 2.2.2
123+
*/
124+
private static boolean isSkipMergeCommits(final CommandParameters parameters) throws ScmException {
125+
if (parameters == null) {
126+
return DEFAULT_SKIP_MERGE_COMMITS;
127+
} else {
128+
return parameters.getBoolean(CommandParameter.SCM_SKIP_MERGE_COMMITS, DEFAULT_SKIP_MERGE_COMMITS);
129+
}
130+
}
105131
}

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
import static org.apache.maven.scm.provider.git.GitScmTestUtils.GIT_COMMAND_LINE;
3434
import static org.junit.jupiter.api.Assertions.assertEquals;
35+
import static org.junit.jupiter.api.Assertions.assertFalse;
3536
import static org.junit.jupiter.api.Assertions.assertNotNull;
3637
import static org.junit.jupiter.api.Assertions.assertTrue;
3738

@@ -92,6 +93,40 @@ void testInfoCommandWithNegativeShortRevision() throws Exception {
9293
"revision should not be short");
9394
}
9495

96+
@Test
97+
void testInfoCommandSkipsMergeCommitsByDefault() throws Exception {
98+
checkSystemCmdPresence(GIT_COMMAND_LINE);
99+
100+
GitScmTestUtils.initRepo("src/test/resources/git/info", getRepositoryRoot(), getWorkingCopy());
101+
102+
ScmProvider provider = getScmManager().getProviderByUrl(getScmUrl());
103+
ScmProviderRepository repository = provider.makeProviderScmRepository(getRepositoryRoot());
104+
assertNotNull(repository);
105+
InfoScmResult result = provider.info(repository, new ScmFileSet(getRepositoryRoot()), new CommandParameters());
106+
assertNotNull(result);
107+
assertTrue(
108+
result.getCommandLine().contains("--no-merges"),
109+
"merge commits must be skipped by default (--no-merges present)");
110+
}
111+
112+
@Test
113+
void testInfoCommandIncludeMergeCommits() throws Exception {
114+
checkSystemCmdPresence(GIT_COMMAND_LINE);
115+
116+
GitScmTestUtils.initRepo("src/test/resources/git/info", getRepositoryRoot(), getWorkingCopy());
117+
118+
ScmProvider provider = getScmManager().getProviderByUrl(getScmUrl());
119+
ScmProviderRepository repository = provider.makeProviderScmRepository(getRepositoryRoot());
120+
assertNotNull(repository);
121+
CommandParameters commandParameters = new CommandParameters();
122+
commandParameters.setString(CommandParameter.SCM_SKIP_MERGE_COMMITS, Boolean.FALSE.toString());
123+
InfoScmResult result = provider.info(repository, new ScmFileSet(getRepositoryRoot()), commandParameters);
124+
assertNotNull(result);
125+
assertFalse(
126+
result.getCommandLine().contains("--no-merges"),
127+
"merge commits must be included when skipMergeCommits=false (no --no-merges)");
128+
}
129+
95130
@Test
96131
void testInfoCommandWithZeroShortRevision() throws Exception {
97132
checkSystemCmdPresence(GIT_COMMAND_LINE);

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

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.util.List;
2525

2626
import org.apache.commons.lang3.StringUtils;
27+
import org.apache.maven.scm.CommandParameter;
2728
import org.apache.maven.scm.CommandParameters;
2829
import org.apache.maven.scm.ScmException;
2930
import org.apache.maven.scm.ScmFileSet;
@@ -42,6 +43,7 @@
4243
import org.eclipse.jgit.revwalk.RevCommit;
4344
import org.eclipse.jgit.revwalk.RevSort;
4445
import org.eclipse.jgit.revwalk.RevWalk;
46+
import org.eclipse.jgit.revwalk.filter.RevFilter;
4547
import org.eclipse.jgit.treewalk.filter.AndTreeFilter;
4648
import org.eclipse.jgit.treewalk.filter.PathFilter;
4749
import org.eclipse.jgit.treewalk.filter.TreeFilter;
@@ -54,6 +56,7 @@ public class JGitInfoCommand extends AbstractCommand implements GitCommand {
5456
protected ScmResult executeCommand(
5557
ScmProviderRepository repository, ScmFileSet fileSet, CommandParameters parameters) throws ScmException {
5658
File basedir = fileSet.getBasedir();
59+
boolean skipMergeCommits = isSkipMergeCommits(parameters);
5760
Git git = null;
5861
try {
5962
git = JGitUtils.openRepo(basedir);
@@ -64,13 +67,13 @@ protected ScmResult executeCommand(
6467

6568
List<InfoItem> infoItems = new LinkedList<>();
6669
if (fileSet.getFileList().isEmpty()) {
67-
RevCommit headCommit = git.getRepository().parseCommit(objectId);
68-
infoItems.add(getInfoItem(headCommit, fileSet.getBasedir()));
70+
RevCommit commit = getMostRecentCommit(git.getRepository(), objectId, skipMergeCommits);
71+
infoItems.add(getInfoItem(commit, fileSet.getBasedir()));
6972
} else {
7073
// iterate over all files
7174
for (File file : JGitUtils.getWorkingCopyRelativePaths(
7275
git.getRepository().getWorkTree(), fileSet)) {
73-
infoItems.add(getInfoItem(git.getRepository(), objectId, file));
76+
infoItems.add(getInfoItem(git.getRepository(), objectId, file, skipMergeCommits));
7477
}
7578
}
7679
return new InfoScmResult(infoItems, new ScmResult("JGit.resolve(HEAD)", "", objectId.toString(), true));
@@ -81,11 +84,30 @@ protected ScmResult executeCommand(
8184
}
8285
}
8386

84-
protected InfoItem getInfoItem(Repository repository, ObjectId headObjectId, File file) throws IOException {
85-
RevCommit commit = getMostRecentCommitForPath(repository, headObjectId, JGitUtils.toNormalizedFilePath(file));
87+
protected InfoItem getInfoItem(Repository repository, ObjectId headObjectId, File file, boolean skipMergeCommits)
88+
throws IOException {
89+
RevCommit commit = getMostRecentCommitForPath(
90+
repository, headObjectId, JGitUtils.toNormalizedFilePath(file), skipMergeCommits);
8691
return getInfoItem(commit, file);
8792
}
8893

94+
/**
95+
* Returns the most recent commit reachable from {@code headObjectId}, optionally ignoring merge commits
96+
* (mimics {@code git log -1 --no-merges} when {@code skipMergeCommits} is {@code true}).
97+
*/
98+
private RevCommit getMostRecentCommit(Repository repository, ObjectId headObjectId, boolean skipMergeCommits)
99+
throws IOException {
100+
try (RevWalk revWalk = new RevWalk(repository)) {
101+
RevCommit headCommit = revWalk.parseCommit(headObjectId);
102+
if (!skipMergeCommits) {
103+
return headCommit;
104+
}
105+
revWalk.markStart(headCommit);
106+
revWalk.setRevFilter(RevFilter.NO_MERGES);
107+
return revWalk.next();
108+
}
109+
}
110+
89111
protected InfoItem getInfoItem(RevCommit fileCommit, File file) {
90112
InfoItem infoItem = new InfoItem();
91113
infoItem.setPath(file.getPath());
@@ -100,16 +122,34 @@ protected InfoItem getInfoItem(RevCommit fileCommit, File file) {
100122
return infoItem;
101123
}
102124

103-
private RevCommit getMostRecentCommitForPath(Repository repository, ObjectId headObjectId, String path)
104-
throws IOException {
125+
private RevCommit getMostRecentCommitForPath(
126+
Repository repository, ObjectId headObjectId, String path, boolean skipMergeCommits) throws IOException {
105127
RevCommit latestCommit = null;
106128
try (RevWalk revWalk = new RevWalk(repository)) {
107129
RevCommit headCommit = revWalk.parseCommit(headObjectId);
108130
revWalk.markStart(headCommit);
109131
revWalk.sort(RevSort.COMMIT_TIME_DESC);
132+
if (skipMergeCommits) {
133+
revWalk.setRevFilter(RevFilter.NO_MERGES);
134+
}
110135
revWalk.setTreeFilter(AndTreeFilter.create(PathFilter.create(path), TreeFilter.ANY_DIFF));
111136
latestCommit = revWalk.next();
112137
}
113138
return latestCommit;
114139
}
140+
141+
/**
142+
* Whether merge commits should be skipped for the {@code info} command.
143+
*
144+
* @param parameters the command parameters (may be {@code null})
145+
* @return {@code true} if parameter {@link CommandParameter#SCM_SKIP_MERGE_COMMITS} (or the whole
146+
* {@code parameters}) is absent, and otherwise the requested value
147+
* @throws ScmException if the parameter has the wrong type
148+
*/
149+
private static boolean isSkipMergeCommits(CommandParameters parameters) throws ScmException {
150+
if (parameters == null) {
151+
return true;
152+
}
153+
return parameters.getBoolean(CommandParameter.SCM_SKIP_MERGE_COMMITS, true);
154+
}
115155
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.maven.scm.provider.git.jgit.command.info;
20+
21+
import java.io.File;
22+
import java.nio.charset.StandardCharsets;
23+
import java.nio.file.Files;
24+
25+
import org.apache.maven.scm.CommandParameter;
26+
import org.apache.maven.scm.CommandParameters;
27+
import org.apache.maven.scm.ScmFileSet;
28+
import org.apache.maven.scm.command.info.InfoScmResult;
29+
import org.eclipse.jgit.api.Git;
30+
import org.eclipse.jgit.api.MergeCommand;
31+
import org.eclipse.jgit.api.MergeResult;
32+
import org.eclipse.jgit.lib.ObjectId;
33+
import org.eclipse.jgit.lib.StoredConfig;
34+
import org.eclipse.jgit.revwalk.RevCommit;
35+
import org.junit.jupiter.api.Test;
36+
import org.junit.jupiter.api.io.TempDir;
37+
38+
import static org.junit.jupiter.api.Assertions.assertEquals;
39+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
40+
import static org.junit.jupiter.api.Assertions.assertNotNull;
41+
42+
/**
43+
* Unit tests for the {@code skipMergeCommits} handling of {@link JGitInfoCommand}.
44+
* A repository whose {@code HEAD} is a merge commit is built with the JGit API, then the
45+
* {@code info} command is invoked with and without the {@link CommandParameter#SCM_SKIP_MERGE_COMMITS} flag.
46+
*/
47+
class JGitInfoCommandTest {
48+
49+
@TempDir
50+
File workDir;
51+
52+
@Test
53+
void includesMergeCommitWhenSkipMergeCommitsIsFalse() throws Exception {
54+
ObjectId mergeCommit = buildRepositoryWithMergeHead();
55+
56+
CommandParameters parameters = new CommandParameters();
57+
parameters.setString(CommandParameter.SCM_SKIP_MERGE_COMMITS, Boolean.FALSE.toString());
58+
59+
InfoScmResult result = info(parameters);
60+
61+
assertNotNull(result);
62+
assertEquals(
63+
mergeCommit.getName(),
64+
result.getInfoItems().get(0).getRevision(),
65+
"HEAD merge commit must be reported when skipMergeCommits=false");
66+
}
67+
68+
@Test
69+
void skipsMergeCommitByDefault() throws Exception {
70+
ObjectId mergeCommit = buildRepositoryWithMergeHead();
71+
72+
InfoScmResult result = info(new CommandParameters());
73+
74+
assertNotNull(result);
75+
assertNotEquals(
76+
mergeCommit.getName(),
77+
result.getInfoItems().get(0).getRevision(),
78+
"merge commit must be skipped by default, a non-merge commit must be reported");
79+
}
80+
81+
private InfoScmResult info(CommandParameters parameters) throws Exception {
82+
// executeCommand is package-private accessible and ignores the repository argument
83+
return (InfoScmResult) new JGitInfoCommand().executeCommand(null, new ScmFileSet(workDir), parameters);
84+
}
85+
86+
/**
87+
* Builds a repository whose {@code HEAD} is a no-fast-forward merge commit (two parents).
88+
*
89+
* @return the id of the merge commit which is now {@code HEAD}
90+
*/
91+
private ObjectId buildRepositoryWithMergeHead() throws Exception {
92+
try (Git git = Git.init().setDirectory(workDir).call()) {
93+
StoredConfig config = git.getRepository().getConfig();
94+
config.setString("user", null, "name", "Test User");
95+
config.setString("user", null, "email", "test@example.com");
96+
config.setBoolean("commit", null, "gpgsign", false);
97+
config.save();
98+
99+
commit(git, "a.txt");
100+
String mainBranch = git.getRepository().getBranch();
101+
102+
git.checkout().setCreateBranch(true).setName("feature").call();
103+
commit(git, "b.txt");
104+
105+
git.checkout().setName(mainBranch).call();
106+
commit(git, "c.txt");
107+
108+
MergeResult mergeResult = git.merge()
109+
.include(git.getRepository().resolve("feature"))
110+
.setFastForward(MergeCommand.FastForwardMode.NO_FF)
111+
.setMessage("merge feature")
112+
.call();
113+
return mergeResult.getNewHead();
114+
}
115+
}
116+
117+
private RevCommit commit(Git git, String fileName) throws Exception {
118+
File file = new File(git.getRepository().getWorkTree(), fileName);
119+
Files.write(file.toPath(), ("content of " + fileName).getBytes(StandardCharsets.UTF_8));
120+
git.add().addFilepattern(fileName).call();
121+
return git.commit()
122+
.setMessage("add " + fileName)
123+
.setAuthor("Test User", "test@example.com")
124+
.setCommitter("Test User", "test@example.com")
125+
.setSign(false)
126+
.call();
127+
}
128+
}

0 commit comments

Comments
 (0)