Skip to content

Commit

Permalink
[JENKINS-16993] Use "git log" to determine changed files.
Browse files Browse the repository at this point in the history
The output of "git show" included commits that were not introduced since
the last build, so could falsely trigger build in the presence of
"includedRegions".

A benefit of not including extraneous commits is the size of the output
is reduced, and therefore less heap required.

This can still trigger extra builds as "-m" means the changes between all
legs of a merge commit are displayed.  However, without this, conflict
resolution changed as part of the merge commit would be skipped.
  • Loading branch information
nwholloway committed Mar 24, 2013
1 parent 8d80b11 commit 340da90
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 4 deletions.
Expand Up @@ -326,12 +326,12 @@ public List<String> showRevision(ObjectId from, ObjectId to) throws GitException
StringWriter writer = new StringWriter();

if (from != null){
writer.write(launchCommand("show", "--no-abbrev", "--format=raw", "-M", "--raw",
writer.write(launchCommand("log", "--full-history", "--no-abbrev", "--format=raw", "-M", "-m", "--raw",
from.name() + ".." + to.name()));
writer.write("\\n");
} else {
writer.write(launchCommand("log", "--full-history", "--no-abbrev", "--format=raw", "-M", "-m", "--raw",
"-1", to.name()));
}

writer.write(launchCommand("whatchanged", "--no-abbrev", "-M", "-m", "--pretty=raw", "-1", to.name()));

String result = writer.toString();
List<String> revShow = new ArrayList<String>();
Expand Down
60 changes: 60 additions & 0 deletions src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestCase.java
@@ -1,6 +1,7 @@
package org.jenkinsci.plugins.gitclient;

import com.google.common.base.Function;
import com.google.common.base.Predicate;
import com.google.common.collect.Collections2;
import hudson.Launcher;
import hudson.model.TaskListener;
Expand All @@ -15,6 +16,7 @@

import java.io.*;
import java.util.Collection;
import java.util.List;
import java.util.Set;

/**
Expand Down Expand Up @@ -318,6 +320,64 @@ public void test_push() throws Exception {
assertEquals(sha1, remoteSha1);
}

public void test_show_revision_for_merge() throws Exception {
launchCommand("git init");
launchCommand("git fetch https://github.com/jenkinsci/git-client-plugin.git");
ObjectId from = ObjectId.fromString("45e76942914664ee19f31d90e6f2edbfe0d13a46");
ObjectId to = ObjectId.fromString("b53374617e85537ec46f86911b5efe3e4e2fa54b");

List<String> revisionDetails = git.showRevision(from, to);

Collection<String> commits = Collections2.filter(revisionDetails, new Predicate<String>() {
public boolean apply(String detail) {
return detail.startsWith("commit ");
}
});
assertEquals(3, commits.size());
assertTrue(commits.contains("commit 4f2964e476776cf59be3e033310f9177bedbf6a8"));
// Merge commit is duplicated as have to capture changes that may have been made as part of merge
assertTrue(commits.contains("commit b53374617e85537ec46f86911b5efe3e4e2fa54b (from 4f2964e476776cf59be3e033310f9177bedbf6a8)"));
assertTrue(commits.contains("commit b53374617e85537ec46f86911b5efe3e4e2fa54b (from 45e76942914664ee19f31d90e6f2edbfe0d13a46)"));

Collection<String> diffs = Collections2.filter(revisionDetails, new Predicate<String>() {
public boolean apply(String detail) {
return detail.startsWith(":");
}
});
Collection<String> paths = Collections2.transform(diffs, new Function<String, String>() {
public String apply(String diff) {
return diff.substring(diff.indexOf('\t')+1);
}
});

assertTrue(paths.contains(".gitignore"));
// Some irrelevant changes will be listed due to merge commit
assertTrue(paths.contains("pom.xml"));
assertTrue(paths.contains("src/main/java/hudson/plugins/git/GitAPI.java"));
assertTrue(paths.contains("src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java"));
assertTrue(paths.contains("src/main/java/org/jenkinsci/plugins/gitclient/Git.java"));
assertTrue(paths.contains("src/main/java/org/jenkinsci/plugins/gitclient/GitClient.java"));
assertTrue(paths.contains("src/main/java/org/jenkinsci/plugins/gitclient/JGitAPIImpl.java"));
assertTrue(paths.contains("src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestCase.java"));
assertTrue(paths.contains("src/test/java/org/jenkinsci/plugins/gitclient/JGitAPIImplTest.java"));
// Previous implementation included other commits, and listed irrelevant changes
assertFalse(paths.contains("README.md"));
}

public void test_show_revision_for_single_commit() throws Exception {
launchCommand("git init");
launchCommand("git fetch https://github.com/jenkinsci/git-client-plugin.git");
ObjectId to = ObjectId.fromString("51de9eda47ca8dcf03b2af58dfff7355585f0d0c");
List<String> revisionDetails = git.showRevision(null, to);
Collection<String> commits = Collections2.filter(revisionDetails, new Predicate<String>() {
public boolean apply(String detail) {
return detail.startsWith("commit ");
}
});
assertEquals(1, commits.size());
assertTrue(commits.contains("commit 51de9eda47ca8dcf03b2af58dfff7355585f0d0c"));
}

private String launchCommand(String args) throws IOException, InterruptedException {
return launchCommand(repo, args);
}
Expand Down
Expand Up @@ -39,6 +39,8 @@ public static Test suite() {
suite.addTest(TestSuite.createTest(JGitAPIImplTest.class, "test_hasGitRepo_without_git_directory"));
suite.addTest(TestSuite.createTest(JGitAPIImplTest.class, "test_hasGitRepo_with_invalid_git_repo"));
suite.addTest(TestSuite.createTest(JGitAPIImplTest.class, "test_hasGitRepo_with_valid_git_repo"));
//suite.addTest(TestSuite.createTest(JGitAPIImplTest.class, "test_show_revision_for_merge"));
//suite.addTest(TestSuite.createTest(JGitAPIImplTest.class, "test_show_revision_for_single_commit"));

return suite;
}
Expand Down

0 comments on commit 340da90

Please sign in to comment.