From 518ed1f7b912c7104b02a19551956de31cce945d Mon Sep 17 00:00:00 2001 From: Rail Sharipov Date: Wed, 6 Nov 2024 16:53:42 -0700 Subject: [PATCH 1/9] add include merge commits option to changelog implementation --- .../jenkinsci/plugins/gitclient/ChangelogCommand.java | 8 ++++++++ .../org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java | 10 ++++++++++ .../org/jenkinsci/plugins/gitclient/JGitAPIImpl.java | 9 ++++++++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/ChangelogCommand.java b/src/main/java/org/jenkinsci/plugins/gitclient/ChangelogCommand.java index 65548b8dc2..2c1ba8ff58 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/ChangelogCommand.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/ChangelogCommand.java @@ -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 diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java index 1f1959135f..fba6ac9de8 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java @@ -1252,6 +1252,7 @@ public ChangelogCommand changelog() { private Integer n = null; private Writer out = null; + private boolean includeMergeCommits = false; @Override public ChangelogCommand excludes(String rev) { @@ -1287,6 +1288,12 @@ 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 */ @@ -1304,6 +1311,9 @@ public void execute() throws GitException, InterruptedException { if (n != null) { args.add("-n").add(n); } + if (includeMergeCommits) { + args.add("-m"); + } for (String rev : this.revs) { args.add(rev); } diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java index d2fa3bf7e7..ffed13c3c7 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java @@ -1263,6 +1263,7 @@ public ChangelogCommand changelog() throws GitException { private RevWalk walk = new RevWalk(or); private Writer out; private boolean hasIncludedRev = false; + private boolean includeMergeCommits = false; @Override public ChangelogCommand excludes(String rev) throws GitException { @@ -1317,6 +1318,12 @@ public ChangelogCommand max(int n) { return this; } + @Override + public ChangelogCommand includeMergeCommits(boolean include) { + this.includeMergeCommits = include; + return this; + } + private void closeResources() { walk.close(); or.close(); @@ -1348,7 +1355,7 @@ public void execute() throws GitException { } for (RevCommit commit : walk) { // git whatachanged doesn't show the merge commits unless -m is given - if (commit.getParentCount() > 1) { + if (!includeMergeCommits && commit.getParentCount() > 1) { continue; } From 79eb22f78855331888166fd8f3d4bf8f45c26eea Mon Sep 17 00:00:00 2001 From: Rail Sharipov Date: Mon, 11 Nov 2024 15:37:12 -0700 Subject: [PATCH 2/9] add tests --- .../plugins/gitclient/JGitAPIImpl.java | 36 ++++- .../plugins/gitclient/GitAPITestUpdate.java | 5 +- .../plugins/gitclient/GitClientTest.java | 125 ++++++++++++++++++ 3 files changed, 156 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java index ffed13c3c7..d417b7c653 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java @@ -116,6 +116,7 @@ import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.revwalk.filter.MaxCountRevFilter; +import org.eclipse.jgit.revwalk.filter.AndRevFilter; import org.eclipse.jgit.revwalk.filter.RevFilter; import org.eclipse.jgit.storage.file.FileRepositoryBuilder; import org.eclipse.jgit.submodule.SubmoduleWalk; @@ -1261,9 +1262,11 @@ public ChangelogCommand changelog() throws GitException { private Repository repo = getRepository(); private ObjectReader or = repo.newObjectReader(); private RevWalk walk = new RevWalk(or); + private RevFilter currentFilter = RevFilter.NO_MERGES; private Writer out; private boolean hasIncludedRev = false; private boolean includeMergeCommits = false; + private Integer maxCount = null; @Override public ChangelogCommand excludes(String rev) throws GitException { @@ -1314,13 +1317,15 @@ public ChangelogCommand to(Writer w) { @Override public ChangelogCommand max(int n) { - walk.setRevFilter(MaxCountRevFilter.create(n)); + this.maxCount = n; + updateRevFilter(); return this; } @Override public ChangelogCommand includeMergeCommits(boolean include) { this.includeMergeCommits = include; + updateRevFilter(); return this; } @@ -1335,6 +1340,30 @@ public void abort() { closeResources(); } + private void updateRevFilter() { + List filters = new ArrayList<>(); + + if (!includeMergeCommits) { + filters.add(RevFilter.NO_MERGES); + } else { + filters.add(RevFilter.NO_MERGES.negate()); + } + + if (maxCount != null) { + filters.add(MaxCountRevFilter.create(maxCount)); + } + + if (filters.isEmpty()) { + currentFilter = RevFilter.ALL; + } else if (filters.size() == 1) { + currentFilter = filters.get(0); + } else { + currentFilter = AndRevFilter.create(filters); + } + + walk.setRevFilter(currentFilter); + } + /** Execute the changelog command. Assumed that this is * only performed once per instance of this object. * Resources opened by this ChangelogCommand object are @@ -1354,11 +1383,6 @@ public void execute() throws GitException { this.includes("HEAD"); } for (RevCommit commit : walk) { - // git whatachanged doesn't show the merge commits unless -m is given - if (!includeMergeCommits && commit.getParentCount() > 1) { - continue; - } - formatter.format(commit, null, pw, true); } } catch (IOException e) { diff --git a/src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestUpdate.java b/src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestUpdate.java index 359681f0fc..34b8217353 100644 --- a/src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestUpdate.java +++ b/src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestUpdate.java @@ -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(""))); } diff --git a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java index 51997bcdcf..7357ffb719 100644 --- a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java +++ b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java @@ -3023,6 +3023,131 @@ 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 { + w.init(); + w.commitEmpty("init"); + + w.git.branch("branch-1"); + w.git.checkout().ref("branch-1").execute(); + w.touch("file-1", "content-1"); + w.git.add("file-1"); + w.git.commit("commit-1"); + String branch1Commit = w.git.revParse("HEAD").name(); + + w.git.branch("branch-2"); + w.git.checkout().ref("branch-2").execute(); + w.touch("file-2", "content-2"); + w.git.add("file-2"); + w.git.commit("commit-2"); + String branch2Commit = w.git.revParse("HEAD").name(); + + // Merge branch-1 into branch-2 + String mergeMessage = "Merge branch-2 into branch-1"; + w.git.checkout().ref("branch-1").execute(); + w.git.merge() + .setMessage(mergeMessage) + .setGitPluginFastForwardMode(MergeCommand.GitPluginFastForwardMode.NO_FF) + .setRevisionToMerge(w.git.getHeadRev(w.repoPath(), "branch-2")) + .execute(); + + // Get changelog with merge commits included + StringWriter writer = new StringWriter(); + w.git.changelog() + .includeMergeCommits(true) + .to(writer) + .execute(); + String changelog = writer.toString(); + + assertThat(changelog, containsString("commit " + w.git.revParse("HEAD").name())); + assertThat(changelog, containsString(mergeMessage)); + assertThat(changelog, containsString("parent " + branch1Commit)); + assertThat(changelog, containsString("commit-1")); + assertThat(changelog, containsString("commit-2")); + + // Get changelog with merge commits excluded + writer = new StringWriter(); + w.git.changelog() + .includeMergeCommits(false) + .to(writer) + .execute(); + changelog = writer.toString(); + + assertThat(changelog, not(containsString(mergeMessage))); + assertThat(changelog, containsString("commit " + branch1Commit)); + assertThat(changelog, containsString("commit-1")); + assertThat(changelog, containsString("commit-2")); + } + + /** + * Tests changelog with merge commits from branches with divergent histories. + */ + @Test + public void testChangelogWithMergeCommitsAndDivergentHistory() throws Exception { + w.init(); + w.commitEmpty("init"); + + // Create branch-1 and add file-1 + w.git.branch("branch-1"); + w.git.checkout().ref("branch-1").execute(); + w.touch("file-1", "content-1"); + w.git.add("file-1"); + w.git.commit("add file-1"); + ObjectId branch1FirstCommit = w.git.revParse("HEAD"); + + // Create branch-2 from branch-1 and add file-2 + w.git.branch("branch-2"); + w.git.checkout().ref("branch-2").execute(); + w.touch("file-2", "content-2"); + w.git.add("file-2"); + w.git.commit("add file-2"); + ObjectId branch2Commit = w.git.revParse("HEAD"); + + // Add more commits to branch-1 + w.git.checkout().ref("branch-1").execute(); + w.touch("file-3", "content-3"); + w.git.add("file-3"); + w.git.commit("add file-3"); + w.touch("file-4", "content-4"); + w.git.add("file-4"); + w.git.commit("add file-4"); + ObjectId branch1FinalCommit = w.git.revParse("HEAD"); + + // Merge branch-2 into branch-1 + String mergeMessage = "Merge branch-2 into branch-1"; + w.git.merge() + .setMessage(mergeMessage) + .setGitPluginFastForwardMode(MergeCommand.GitPluginFastForwardMode.NO_FF) + .setRevisionToMerge(w.git.getHeadRev(w.repoPath(), "branch-2")) + .execute(); + ObjectId mergeCommit = w.git.revParse("HEAD"); + + // Get changelog with merge commits included + StringWriter writer = new StringWriter(); + w.git.changelog() + .includeMergeCommits(true) + .to(writer) + .execute(); + String changelog = writer.toString(); + + assertThat(changelog, containsString("commit " + mergeCommit.name())); + assertThat(changelog, containsString("parent " + branch1FinalCommit.name() + " " + branch2Commit.name())); + assertThat(changelog, containsString(mergeMessage)); + + // Get changelog with merge commits excluded + writer = new StringWriter(); + w.git.changelog() + .includeMergeCommits(false) + .to(writer) + .execute(); + changelog = writer.toString(); + + assertThat(changelog, not(containsString(mergeMessage))); + } + @Test @Issue("JENKINS-55284") // Pull must overwrite existing tags public void testFetchOverwritesExistingTags() throws Exception { From e6950cc81301aa03119f7bc0f5c9182ef2750c60 Mon Sep 17 00:00:00 2001 From: Rail Sharipov Date: Mon, 11 Nov 2024 15:40:44 -0700 Subject: [PATCH 3/9] Revert "[FIXED JENKINS-27097] use ISO-8601 in changelog" This reverts commit de49c9bf21f560ac3851c4f7ea3a3597a114ac8e. --- .../plugins/gitclient/CliGitAPIImpl.java | 11 +++++------ .../plugins/gitclient/JGitAPIImpl.java | 18 +++++++----------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java index fba6ac9de8..167400c445 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java @@ -1243,13 +1243,7 @@ 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 revs = new ArrayList<>(); - private Integer n = null; private Writer out = null; private boolean includeMergeCommits = false; @@ -1301,6 +1295,7 @@ public void abort() { @Override public void execute() throws GitException, InterruptedException { +<<<<<<< HEAD ArgumentListBuilder args = new ArgumentListBuilder(gitExe, "whatchanged", "--no-abbrev", "-M"); if (isAtLeastVersion(1, 8, 3, 0)) { args.add("--format=" + RAW); @@ -1309,6 +1304,10 @@ public void execute() throws GitException, InterruptedException { args.add("--format=raw"); } if (n != null) { +======= + ArgumentListBuilder args = new ArgumentListBuilder(gitExe, "whatchanged", "--no-abbrev", "-M", "--pretty=raw"); + if (n!=null) +>>>>>>> parent of de49c9bf ([FIXED JENKINS-27097] use ISO-8601 in changelog) args.add("-n").add(n); } if (includeMergeCommits) { diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java index d417b7c653..f39234de78 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java @@ -62,8 +62,10 @@ 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 javax.annotation.Nullable; + import org.eclipse.jgit.api.AddNoteCommand; import org.eclipse.jgit.api.CommitCommand; import org.eclipse.jgit.api.CreateBranchCommand.SetupUpstreamMode; @@ -1419,8 +1421,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. * @@ -1442,14 +1442,10 @@ void format(RevCommit commit, RevCommit parent, PrintWriter pw, Boolean useRawOu } pw.printf("tree %s\n", commit.getTree().name()); - 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())); + for (RevCommit p : commit.getParents()) + pw.printf("parent %s\n",p.name()); + 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(); From aecc1d47dfe6e2678571f359c423ee9e156ca4be Mon Sep 17 00:00:00 2001 From: Rail Sharipov Date: Mon, 11 Nov 2024 15:46:31 -0700 Subject: [PATCH 4/9] update --- .../jenkinsci/plugins/gitclient/CliGitAPIImpl.java | 13 +------------ .../jenkinsci/plugins/gitclient/GitClientTest.java | 3 ++- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java index 167400c445..44a7814e91 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java @@ -1295,19 +1295,8 @@ public void abort() { @Override public void execute() throws GitException, InterruptedException { -<<<<<<< HEAD - 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"); - } - if (n != null) { -======= - ArgumentListBuilder args = new ArgumentListBuilder(gitExe, "whatchanged", "--no-abbrev", "-M", "--pretty=raw"); + ArgumentListBuilder args = new ArgumentListBuilder(gitExe, "whatchanged", "--no-abbrev", "-M", "--format=raw"); if (n!=null) ->>>>>>> parent of de49c9bf ([FIXED JENKINS-27097] use ISO-8601 in changelog) args.add("-n").add(n); } if (includeMergeCommits) { diff --git a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java index 7357ffb719..b3051aa1c0 100644 --- a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java +++ b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java @@ -3134,7 +3134,8 @@ public void testChangelogWithMergeCommitsAndDivergentHistory() throws Exception String changelog = writer.toString(); assertThat(changelog, containsString("commit " + mergeCommit.name())); - assertThat(changelog, containsString("parent " + branch1FinalCommit.name() + " " + branch2Commit.name())); + assertThat(changelog, containsString("parent " + branch1FinalCommit.name())); + assertThat(changelog, containsString("parent " + branch2Commit.name())); assertThat(changelog, containsString(mergeMessage)); // Get changelog with merge commits excluded From a04c370cae1127bb9c0be5181ddb9e13662c3835 Mon Sep 17 00:00:00 2001 From: Rail Sharipov Date: Mon, 11 Nov 2024 15:48:42 -0700 Subject: [PATCH 5/9] update --- .../java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java index 44a7814e91..80ecdbb2cf 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java @@ -1296,7 +1296,7 @@ public void abort() { @Override public void execute() throws GitException, InterruptedException { ArgumentListBuilder args = new ArgumentListBuilder(gitExe, "whatchanged", "--no-abbrev", "-M", "--format=raw"); - if (n!=null) + if (n != null) { args.add("-n").add(n); } if (includeMergeCommits) { From 7e14ab0e8f1a7315a8879ca90356fafca460c7b5 Mon Sep 17 00:00:00 2001 From: Rail Sharipov Date: Wed, 13 Nov 2024 20:52:49 -0700 Subject: [PATCH 6/9] update tests --- .../plugins/gitclient/JGitAPIImpl.java | 17 +- .../plugins/gitclient/GitClientTest.java | 150 +++++++++--------- 2 files changed, 84 insertions(+), 83 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java index f39234de78..aa892a235e 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java @@ -1264,7 +1264,7 @@ public ChangelogCommand changelog() throws GitException { private Repository repo = getRepository(); private ObjectReader or = repo.newObjectReader(); private RevWalk walk = new RevWalk(or); - private RevFilter currentFilter = RevFilter.NO_MERGES; + private RevFilter revFilter = RevFilter.NO_MERGES; private Writer out; private boolean hasIncludedRev = false; private boolean includeMergeCommits = false; @@ -1347,8 +1347,6 @@ private void updateRevFilter() { if (!includeMergeCommits) { filters.add(RevFilter.NO_MERGES); - } else { - filters.add(RevFilter.NO_MERGES.negate()); } if (maxCount != null) { @@ -1356,14 +1354,12 @@ private void updateRevFilter() { } if (filters.isEmpty()) { - currentFilter = RevFilter.ALL; + revFilter = RevFilter.ALL; } else if (filters.size() == 1) { - currentFilter = filters.get(0); + revFilter = filters.get(0); } else { - currentFilter = AndRevFilter.create(filters); + revFilter = AndRevFilter.create(filters); } - - walk.setRevFilter(currentFilter); } /** Execute the changelog command. Assumed that this is @@ -1384,6 +1380,8 @@ public void execute() throws GitException { /* If no rev has been included, assume HEAD */ this.includes("HEAD"); } + walk.setRevFilter(revFilter); + for (RevCommit commit : walk) { formatter.format(commit, null, pw, true); } @@ -1442,8 +1440,9 @@ void format(RevCommit commit, RevCommit parent, PrintWriter pw, Boolean useRawOu } pw.printf("tree %s\n", commit.getTree().name()); - for (RevCommit p : commit.getParents()) + for (RevCommit p : commit.getParents()) { pw.printf("parent %s\n",p.name()); + } pw.printf("author %s\n", commit.getAuthorIdent().toExternalString()); pw.printf("committer %s\n", commit.getCommitterIdent().toExternalString()); diff --git a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java index b3051aa1c0..b566cbf9b1 100644 --- a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java +++ b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java @@ -3028,58 +3028,60 @@ public void testChangelogFirstLineTruncation() throws Exception { */ @Test public void testChangelogWithMergeCommitsAndLinearHistory() throws Exception { - w.init(); - w.commitEmpty("init"); - - w.git.branch("branch-1"); - w.git.checkout().ref("branch-1").execute(); - w.touch("file-1", "content-1"); - w.git.add("file-1"); - w.git.commit("commit-1"); - String branch1Commit = w.git.revParse("HEAD").name(); - - w.git.branch("branch-2"); - w.git.checkout().ref("branch-2").execute(); - w.touch("file-2", "content-2"); - w.git.add("file-2"); - w.git.commit("commit-2"); - String branch2Commit = w.git.revParse("HEAD").name(); - - // Merge branch-1 into branch-2 - String mergeMessage = "Merge branch-2 into branch-1"; - w.git.checkout().ref("branch-1").execute(); - w.git.merge() - .setMessage(mergeMessage) - .setGitPluginFastForwardMode(MergeCommand.GitPluginFastForwardMode.NO_FF) - .setRevisionToMerge(w.git.getHeadRev(w.repoPath(), "branch-2")) - .execute(); + 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(); - w.git.changelog() + gitClient.changelog() .includeMergeCommits(true) .to(writer) .execute(); String changelog = writer.toString(); - assertThat(changelog, containsString("commit " + w.git.revParse("HEAD").name())); + assertThat(changelog, containsString("commit " + gitClient.revParse("HEAD").name())); assertThat(changelog, containsString(mergeMessage)); - assertThat(changelog, containsString("parent " + branch1Commit)); - assertThat(changelog, containsString("commit-1")); - assertThat(changelog, containsString("commit-2")); + 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(); - w.git.changelog() + 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 " + branch1Commit)); - assertThat(changelog, containsString("commit-1")); - assertThat(changelog, containsString("commit-2")); + assertThat(changelog, containsString("commit " + branch2Commit.name())); + assertThat(changelog, containsString("parent " + branch1Commit.name())); + assertThat(changelog, containsString(branch1 + "-commit")); + assertThat(changelog, containsString(branch2 + "-commit")); } /** @@ -3087,66 +3089,66 @@ public void testChangelogWithMergeCommitsAndLinearHistory() throws Exception { */ @Test public void testChangelogWithMergeCommitsAndDivergentHistory() throws Exception { - w.init(); - w.commitEmpty("init"); - - // Create branch-1 and add file-1 - w.git.branch("branch-1"); - w.git.checkout().ref("branch-1").execute(); - w.touch("file-1", "content-1"); - w.git.add("file-1"); - w.git.commit("add file-1"); - ObjectId branch1FirstCommit = w.git.revParse("HEAD"); - - // Create branch-2 from branch-1 and add file-2 - w.git.branch("branch-2"); - w.git.checkout().ref("branch-2").execute(); - w.touch("file-2", "content-2"); - w.git.add("file-2"); - w.git.commit("add file-2"); - ObjectId branch2Commit = w.git.revParse("HEAD"); - - // Add more commits to branch-1 - w.git.checkout().ref("branch-1").execute(); - w.touch("file-3", "content-3"); - w.git.add("file-3"); - w.git.commit("add file-3"); - w.touch("file-4", "content-4"); - w.git.add("file-4"); - w.git.commit("add file-4"); - ObjectId branch1FinalCommit = w.git.revParse("HEAD"); - - // Merge branch-2 into branch-1 - String mergeMessage = "Merge branch-2 into branch-1"; - w.git.merge() - .setMessage(mergeMessage) - .setGitPluginFastForwardMode(MergeCommand.GitPluginFastForwardMode.NO_FF) - .setRevisionToMerge(w.git.getHeadRev(w.repoPath(), "branch-2")) - .execute(); - ObjectId mergeCommit = w.git.revParse("HEAD"); + 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(); - w.git.changelog() + 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(mergeMessage)); + assertThat(changelog, containsString(branch1 + "-commit")); + assertThat(changelog, containsString(branch2 + "-commit")); // Get changelog with merge commits excluded writer = new StringWriter(); - w.git.changelog() + 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 From d49a05ea708e1ede2580a9fb621d5f11364bd4bb Mon Sep 17 00:00:00 2001 From: Name Example Date: Thu, 14 Nov 2024 18:14:16 +0000 Subject: [PATCH 7/9] update methods --- .../jenkinsci/plugins/gitclient/JGitAPIImpl.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java index aa892a235e..a0e37f1e60 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java @@ -1320,14 +1320,12 @@ public ChangelogCommand to(Writer w) { @Override public ChangelogCommand max(int n) { this.maxCount = n; - updateRevFilter(); return this; } @Override public ChangelogCommand includeMergeCommits(boolean include) { this.includeMergeCommits = include; - updateRevFilter(); return this; } @@ -1342,23 +1340,21 @@ public void abort() { closeResources(); } - private void updateRevFilter() { + private RevFilter getRevFilter() { List filters = new ArrayList<>(); if (!includeMergeCommits) { filters.add(RevFilter.NO_MERGES); } - if (maxCount != null) { filters.add(MaxCountRevFilter.create(maxCount)); } - if (filters.isEmpty()) { - revFilter = RevFilter.ALL; + return RevFilter.ALL; } else if (filters.size() == 1) { - revFilter = filters.get(0); + return filters.get(0); } else { - revFilter = AndRevFilter.create(filters); + return AndRevFilter.create(filters); } } @@ -1380,7 +1376,7 @@ public void execute() throws GitException { /* If no rev has been included, assume HEAD */ this.includes("HEAD"); } - walk.setRevFilter(revFilter); + walk.setRevFilter(getRevFilter()); for (RevCommit commit : walk) { formatter.format(commit, null, pw, true); From 868412038349c3a59afb4c6cf72a22bf255f10e0 Mon Sep 17 00:00:00 2001 From: Rail Sharipov Date: Thu, 14 Nov 2024 12:11:32 -0700 Subject: [PATCH 8/9] fix spotless errors --- .../plugins/gitclient/CliGitAPIImpl.java | 3 ++- .../plugins/gitclient/JGitAPIImpl.java | 7 ++--- .../plugins/gitclient/GitClientTest.java | 27 +++++++------------ 3 files changed, 13 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java index 80ecdbb2cf..2b73130f79 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java @@ -1295,7 +1295,8 @@ public void abort() { @Override public void execute() throws GitException, InterruptedException { - ArgumentListBuilder args = new ArgumentListBuilder(gitExe, "whatchanged", "--no-abbrev", "-M", "--format=raw"); + ArgumentListBuilder args = + new ArgumentListBuilder(gitExe, "whatchanged", "--no-abbrev", "-M", "--format=raw"); if (n != null) { args.add("-n").add(n); } diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java index a0e37f1e60..8536e78a00 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java @@ -63,9 +63,6 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang.SystemUtils; import org.apache.sshd.common.util.security.SecurityUtils; - -import javax.annotation.Nullable; - import org.eclipse.jgit.api.AddNoteCommand; import org.eclipse.jgit.api.CommitCommand; import org.eclipse.jgit.api.CreateBranchCommand.SetupUpstreamMode; @@ -117,8 +114,8 @@ import org.eclipse.jgit.revwalk.RevSort; import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; -import org.eclipse.jgit.revwalk.filter.MaxCountRevFilter; 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; import org.eclipse.jgit.submodule.SubmoduleWalk; @@ -1437,7 +1434,7 @@ void format(RevCommit commit, RevCommit parent, PrintWriter pw, Boolean useRawOu pw.printf("tree %s\n", commit.getTree().name()); for (RevCommit p : commit.getParents()) { - pw.printf("parent %s\n",p.name()); + pw.printf("parent %s\n", p.name()); } pw.printf("author %s\n", commit.getAuthorIdent().toExternalString()); pw.printf("committer %s\n", commit.getCommitterIdent().toExternalString()); diff --git a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java index b566cbf9b1..bd44bd0bee 100644 --- a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java +++ b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java @@ -3055,13 +3055,11 @@ public void testChangelogWithMergeCommitsAndLinearHistory() throws Exception { // Get changelog with merge commits included StringWriter writer = new StringWriter(); - gitClient.changelog() - .includeMergeCommits(true) - .to(writer) - .execute(); + gitClient.changelog().includeMergeCommits(true).to(writer).execute(); String changelog = writer.toString(); - assertThat(changelog, containsString("commit " + gitClient.revParse("HEAD").name())); + assertThat( + changelog, containsString("commit " + gitClient.revParse("HEAD").name())); assertThat(changelog, containsString(mergeMessage)); assertThat(changelog, containsString("commit " + branch2Commit.name())); assertThat(changelog, containsString("parent " + branch1Commit.name())); @@ -3070,13 +3068,12 @@ public void testChangelogWithMergeCommitsAndLinearHistory() throws Exception { // Get changelog with merge commits excluded writer = new StringWriter(); - gitClient.changelog() - .includeMergeCommits(false) - .to(writer) - .execute(); + gitClient.changelog().includeMergeCommits(false).to(writer).execute(); changelog = writer.toString(); - assertThat(changelog, not(containsString("commit " + gitClient.revParse("HEAD").name()))); + 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())); @@ -3120,10 +3117,7 @@ public void testChangelogWithMergeCommitsAndDivergentHistory() throws Exception // Get changelog with merge commits included StringWriter writer = new StringWriter(); - gitClient.changelog() - .includeMergeCommits(true) - .to(writer) - .execute(); + gitClient.changelog().includeMergeCommits(true).to(writer).execute(); String changelog = writer.toString(); assertThat(changelog, containsString("commit " + mergeCommit.name())); @@ -3135,10 +3129,7 @@ public void testChangelogWithMergeCommitsAndDivergentHistory() throws Exception // Get changelog with merge commits excluded writer = new StringWriter(); - gitClient.changelog() - .includeMergeCommits(false) - .to(writer) - .execute(); + gitClient.changelog().includeMergeCommits(false).to(writer).execute(); changelog = writer.toString(); assertThat(changelog, not(containsString("commit " + mergeCommit.name()))); From ac0fb04ef74174b55e3c6759f4c0d74649bae39e Mon Sep 17 00:00:00 2001 From: Name Example Date: Thu, 14 Nov 2024 22:04:47 +0000 Subject: [PATCH 9/9] fix spotbug warnings --- .../plugins/gitclient/JGitAPIImpl.java | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java index 8536e78a00..c3693f07dd 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java @@ -1261,7 +1261,6 @@ public ChangelogCommand changelog() throws GitException { private Repository repo = getRepository(); private ObjectReader or = repo.newObjectReader(); private RevWalk walk = new RevWalk(or); - private RevFilter revFilter = RevFilter.NO_MERGES; private Writer out; private boolean hasIncludedRev = false; private boolean includeMergeCommits = false; @@ -1337,24 +1336,6 @@ public void abort() { closeResources(); } - private RevFilter getRevFilter() { - List filters = new ArrayList<>(); - - if (!includeMergeCommits) { - filters.add(RevFilter.NO_MERGES); - } - if (maxCount != null) { - filters.add(MaxCountRevFilter.create(maxCount)); - } - if (filters.isEmpty()) { - return RevFilter.ALL; - } else if (filters.size() == 1) { - return filters.get(0); - } else { - return AndRevFilter.create(filters); - } - } - /** Execute the changelog command. Assumed that this is * only performed once per instance of this object. * Resources opened by this ChangelogCommand object are @@ -1367,14 +1348,27 @@ public void execute() throws GitException { if (out == null) { throw new IllegalStateException(); // Match CliGitAPIImpl } + List 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"); } - walk.setRevFilter(getRevFilter()); - for (RevCommit commit : walk) { formatter.format(commit, null, pw, true); }