Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-65032] Add option to include merge commits in changelogs #1214

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,14 @@ public interface ChangelogCommand extends GitCommand {
*/
ChangelogCommand max(int n);

/**
* Include merge commits in the changelog.
*
* @param include a boolean.
* @return a {@link org.jenkinsci.plugins.gitclient.ChangelogCommand} object.
*/
ChangelogCommand includeMergeCommits(boolean include);

/**
* Abort this ChangelogCommand without executing it, close any
* open resources. The JGit implementation of changelog
Expand Down
25 changes: 12 additions & 13 deletions src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -1243,15 +1243,10 @@ public void prune(RemoteConfig repository) throws GitException, InterruptedExcep
@Override
public ChangelogCommand changelog() {
return new ChangelogCommand() {

/** Equivalent to the git-log raw format but using ISO 8601 date format - also prevent to depend on git CLI future changes */
public static final String RAW =
"commit %H%ntree %T%nparent %P%nauthor %aN <%aE> %ai%ncommitter %cN <%cE> %ci%n%n%w(0,4,4)%B";

private final List<String> revs = new ArrayList<>();

private Integer n = null;
private Writer out = null;
private boolean includeMergeCommits = false;

@Override
public ChangelogCommand excludes(String rev) {
Expand Down Expand Up @@ -1287,23 +1282,27 @@ public ChangelogCommand max(int n) {
return this;
}

@Override
public ChangelogCommand includeMergeCommits(boolean include) {
this.includeMergeCommits = include;
return this;
}

@Override
public void abort() {
/* No cleanup needed to abort the CliGitAPIImpl ChangelogCommand */
}

@Override
public void execute() throws GitException, InterruptedException {
ArgumentListBuilder args = new ArgumentListBuilder(gitExe, "whatchanged", "--no-abbrev", "-M");
if (isAtLeastVersion(1, 8, 3, 0)) {
args.add("--format=" + RAW);
} else {
/* Ancient git versions don't support the required format string, use 'raw' and hope it works */
args.add("--format=raw");
}
ArgumentListBuilder args =
new ArgumentListBuilder(gitExe, "whatchanged", "--no-abbrev", "-M", "--format=raw");
if (n != null) {
args.add("-n").add(n);
}
if (includeMergeCommits) {
args.add("-m");
}
for (String rev : this.revs) {
args.add(rev);
}
Expand Down
41 changes: 27 additions & 14 deletions src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
import jenkins.util.SystemProperties;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.SystemUtils;
import org.apache.commons.lang.time.FastDateFormat;
import org.apache.sshd.common.util.security.SecurityUtils;
import org.eclipse.jgit.api.AddNoteCommand;
import org.eclipse.jgit.api.CommitCommand;
Expand Down Expand Up @@ -115,6 +114,7 @@
import org.eclipse.jgit.revwalk.RevSort;
import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.revwalk.filter.AndRevFilter;
import org.eclipse.jgit.revwalk.filter.MaxCountRevFilter;
import org.eclipse.jgit.revwalk.filter.RevFilter;
import org.eclipse.jgit.storage.file.FileRepositoryBuilder;
Expand Down Expand Up @@ -1263,6 +1263,8 @@ public ChangelogCommand changelog() throws GitException {
private RevWalk walk = new RevWalk(or);
private Writer out;
private boolean hasIncludedRev = false;
private boolean includeMergeCommits = false;
private Integer maxCount = null;

@Override
public ChangelogCommand excludes(String rev) throws GitException {
Expand Down Expand Up @@ -1313,7 +1315,13 @@ public ChangelogCommand to(Writer w) {

@Override
public ChangelogCommand max(int n) {
walk.setRevFilter(MaxCountRevFilter.create(n));
this.maxCount = n;
return this;
}

@Override
public ChangelogCommand includeMergeCommits(boolean include) {
this.includeMergeCommits = include;
return this;
}

Expand All @@ -1340,18 +1348,28 @@ public void execute() throws GitException {
if (out == null) {
throw new IllegalStateException(); // Match CliGitAPIImpl
}
List<RevFilter> filters = new ArrayList<>();

if (!includeMergeCommits) {
filters.add(RevFilter.NO_MERGES);
}
if (maxCount != null) {
filters.add(MaxCountRevFilter.create(maxCount));
}
if (filters.isEmpty()) {
walk.setRevFilter(RevFilter.ALL);
} else if (filters.size() == 1) {
walk.setRevFilter(filters.get(0));
} else {
walk.setRevFilter(AndRevFilter.create(filters));
}
try (PrintWriter pw = new PrintWriter(out, false)) {
RawFormatter formatter = new RawFormatter();
if (!hasIncludedRev) {
/* If no rev has been included, assume HEAD */
this.includes("HEAD");
}
for (RevCommit commit : walk) {
// git whatachanged doesn't show the merge commits unless -m is given
if (commit.getParentCount() > 1) {
continue;
}

formatter.format(commit, null, pw, true);
}
} catch (IOException e) {
Expand Down Expand Up @@ -1388,8 +1406,6 @@ private String statusOf(DiffEntry d) {
}
}

public static final String ISO_8601 = "yyyy-MM-dd'T'HH:mm:ssZ";

/**
* Formats a commit into the raw format.
*
Expand All @@ -1414,11 +1430,8 @@ void format(RevCommit commit, RevCommit parent, PrintWriter pw, Boolean useRawOu
for (RevCommit p : commit.getParents()) {
pw.printf("parent %s\n", p.name());
}
FastDateFormat iso = FastDateFormat.getInstance(ISO_8601);
PersonIdent a = commit.getAuthorIdent();
pw.printf("author %s <%s> %s\n", a.getName(), a.getEmailAddress(), iso.format(a.getWhen()));
PersonIdent c = commit.getCommitterIdent();
pw.printf("committer %s <%s> %s\n", c.getName(), c.getEmailAddress(), iso.format(c.getWhen()));
pw.printf("author %s\n", commit.getAuthorIdent().toExternalString());
pw.printf("committer %s\n", commit.getCommitterIdent().toExternalString());

// indent commit messages by 4 chars
String msg = commit.getFullMessage();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2038,11 +2038,8 @@ public void testChangelogWithMergeCommitAndMaxLogHistory() throws Exception {
.setRevisionToMerge(w.git.getHeadRev(w.repoPath(), "branch-1"))
.execute();

/* JGit handles merge commits in changelog differently than CLI git. See JENKINS-40023. */
int maxlimit = w.git instanceof CliGitAPIImpl ? 1 : 2;

StringWriter writer = new StringWriter();
w.git.changelog().max(maxlimit).to(writer).execute();
w.git.changelog().max(1).to(writer).execute();
assertThat(writer.toString(), is(not("")));
}

Expand Down
119 changes: 119 additions & 0 deletions src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3023,6 +3023,125 @@ public void testChangelogFirstLineTruncation() throws Exception {
assertThat(changelogStringWriter.toString(), containsString(padLinesWithSpaces(commitMessage, 4)));
}

/**
* Tests changelog with merge commits from branches with linear histories.
*/
@Test
public void testChangelogWithMergeCommitsAndLinearHistory() throws Exception {
final String branch1 = "changelog-branch-1";
final String branch2 = "changelog-branch-2";
final String mergeMessage = "Merge " + branch2 + " into " + branch1;

// Create initial commit first
ObjectId initialCommit = commitOneFile();

// Create first branch with a commit
gitClient.branch(branch1);
gitClient.checkout().ref(branch1).execute();
ObjectId branch1Commit = commitFile("file-1", "content-1", branch1 + "-commit");

// Create second branch with a commit
gitClient.branch(branch2);
gitClient.checkout().ref(branch2).execute();
ObjectId branch2Commit = commitFile("file-2", "content-2", branch2 + "-commit");

// Merge branch2 into branch1 with merge commit
gitClient.checkout().ref(branch1).execute();
MergeCommand mergeCommand = gitClient.merge();
mergeCommand.setMessage(mergeMessage);
mergeCommand.setGitPluginFastForwardMode(MergeCommand.GitPluginFastForwardMode.NO_FF);
mergeCommand.setRevisionToMerge(gitClient.getHeadRev(repoRoot.getAbsolutePath(), branch2));
mergeCommand.execute();

// Get changelog with merge commits included
StringWriter writer = new StringWriter();
gitClient.changelog().includeMergeCommits(true).to(writer).execute();
String changelog = writer.toString();

assertThat(
changelog, containsString("commit " + gitClient.revParse("HEAD").name()));
assertThat(changelog, containsString(mergeMessage));
assertThat(changelog, containsString("commit " + branch2Commit.name()));
assertThat(changelog, containsString("parent " + branch1Commit.name()));
assertThat(changelog, containsString(branch1 + "-commit"));
assertThat(changelog, containsString(branch2 + "-commit"));

// Get changelog with merge commits excluded
writer = new StringWriter();
gitClient.changelog().includeMergeCommits(false).to(writer).execute();
changelog = writer.toString();

assertThat(
changelog,
not(containsString("commit " + gitClient.revParse("HEAD").name())));
assertThat(changelog, not(containsString(mergeMessage)));
assertThat(changelog, containsString("commit " + branch2Commit.name()));
assertThat(changelog, containsString("parent " + branch1Commit.name()));
assertThat(changelog, containsString(branch1 + "-commit"));
assertThat(changelog, containsString(branch2 + "-commit"));
}

/**
* Tests changelog with merge commits from branches with divergent histories.
*/
@Test
public void testChangelogWithMergeCommitsAndDivergentHistory() throws Exception {
final String branch1 = "changelog-div-1";
final String branch2 = "changelog-div-2";
final String mergeMessage = "Merge " + branch2 + " into " + branch1;

ObjectId initialCommit = commitOneFile();

// Create first branch with a commit
gitClient.branch(branch1);
gitClient.checkout().ref(branch1).execute();
ObjectId branch1Commit = commitFile("file-1", "content-1", branch1 + "-commit");

// Create second branch from branch1 and add a commit
gitClient.branch(branch2);
gitClient.checkout().ref(branch2).execute();
ObjectId branch2Commit = commitFile("file-2", "content-2", branch2 + "-commit");

// Add more commits to branch1
gitClient.checkout().ref(branch1).execute();
commitFile("file-3", "content-3", branch1 + "-commit");
ObjectId branch1FinalCommit = commitFile("file-4", "content-4", branch1 + "-commit");

// Merge branch2 into branch1
MergeCommand mergeCommand = gitClient.merge();
mergeCommand.setMessage(mergeMessage);
mergeCommand.setGitPluginFastForwardMode(MergeCommand.GitPluginFastForwardMode.NO_FF);
mergeCommand.setRevisionToMerge(gitClient.getHeadRev(repoRoot.getAbsolutePath(), branch2));
mergeCommand.execute();
ObjectId mergeCommit = gitClient.revParse("HEAD");

// Get changelog with merge commits included
StringWriter writer = new StringWriter();
gitClient.changelog().includeMergeCommits(true).to(writer).execute();
String changelog = writer.toString();

assertThat(changelog, containsString("commit " + mergeCommit.name()));
assertThat(changelog, containsString(mergeMessage));
assertThat(changelog, containsString("parent " + branch1FinalCommit.name()));
assertThat(changelog, containsString("parent " + branch2Commit.name()));
assertThat(changelog, containsString(branch1 + "-commit"));
assertThat(changelog, containsString(branch2 + "-commit"));

// Get changelog with merge commits excluded
writer = new StringWriter();
gitClient.changelog().includeMergeCommits(false).to(writer).execute();
changelog = writer.toString();

assertThat(changelog, not(containsString("commit " + mergeCommit.name())));
assertThat(changelog, not(containsString(mergeMessage)));
assertThat(changelog, not(containsString("parent " + branch1FinalCommit.name())));
assertThat(changelog, not(containsString("parent " + branch2Commit.name())));
assertThat(changelog, containsString("commit " + branch1FinalCommit.name()));
assertThat(changelog, containsString("parent " + branch1Commit.name()));
assertThat(changelog, containsString(branch1 + "-commit"));
assertThat(changelog, containsString(branch2 + "-commit"));
}

@Test
@Issue("JENKINS-55284") // Pull must overwrite existing tags
public void testFetchOverwritesExistingTags() throws Exception {
Expand Down