Skip to content

Commit

Permalink
Merge pull request #196 from MarkEWaite/master-JENKINS-32258-many-rev…
Browse files Browse the repository at this point in the history 
…-parse-calls

Fix JENKINS-32258 - many rev-parse calls pruning remote tracking branches
  • Loading branch information
@MarkEWaite
MarkEWaite committed Jan 9, 2016
2 parents 3139a94 + 1a450a5 commit 89ab085
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 18 deletions.
38 changes: 20 additions & 18 deletions src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java
View file
Expand Up @@ -1774,29 +1774,31 @@ public void execute() throws GitException, InterruptedException {
}

/**
* parseBranches.
* Parse branch name and SHA1 from "fos" argument string.
*
* @param fos a {@link java.lang.String} object.
* Argument content must match "git branch -v --no-abbrev".
*
* One branch per line, two leading characters ignored on each
* line, the branch name (not allowed to contain spaces), one or
* more spaces, and the 40 character SHA1 of the commit that is
* the head of that branch. Text after the SHA1 is ignored.
*
* @param fos output of "git branch -v --no-abbrev"
* @return a {@link java.util.Set} object.
* @throws hudson.plugins.git.GitException if underlying git operation fails.
* @throws java.lang.InterruptedException if interrupted.
*/
protected Set<Branch> parseBranches(String fos) throws GitException, InterruptedException {
// TODO: git branch -a -v --abbrev=0 would do this in one shot..

private Set<Branch> parseBranches(String fos) {
Set<Branch> branches = new HashSet<Branch>();

BufferedReader rdr = new BufferedReader(new StringReader(fos));
String line;
try {
while ((line = rdr.readLine()) != null) {
// Ignore the 1st
line = line.substring(2);
// Ignore '(no branch)' or anything with " -> ", since I think
// that's just noise
if ((!line.startsWith("("))
&& (line.indexOf(" -> ") == -1)) {
branches.add(new Branch(line, revParse(line)));
// Ignore leading 2 characters (marker for current branch)
// Ignore line if second field is not SHA1 length (40 characters)
// Split fields into branch name, SHA1, and rest of line
// Fields are separated by one or more spaces
String[] branchVerboseOutput = line.substring(2).split(" +", 3);
if (branchVerboseOutput[1].length() == 40) {
branches.add(new Branch(branchVerboseOutput[0], ObjectId.fromString(branchVerboseOutput[1])));
}
}
} catch (IOException e) {
Expand All @@ -1816,7 +1818,7 @@ protected Set<Branch> parseBranches(String fos) throws GitException, Interrupted
* @throws java.lang.InterruptedException if interrupted.
*/
public Set<Branch> getBranches() throws GitException, InterruptedException {
return parseBranches(launchCommand("branch", "-a"));
return parseBranches(launchCommand("branch", "-a", "-v", "--no-abbrev"));
}

/**
Expand Down Expand Up @@ -2554,9 +2556,9 @@ public List<Branch> getBranchesContaining(String revspec, boolean allBranches)
throws GitException, InterruptedException {
final String commandOutput;
if (allBranches) {
commandOutput = launchCommand("branch", "-a", "--contains", revspec);
commandOutput = launchCommand("branch", "-a", "-v", "--no-abbrev", "--contains", revspec);
} else {
commandOutput = launchCommand("branch", "--contains", revspec);
commandOutput = launchCommand("branch", "-v", "--no-abbrev", "--contains", revspec);
}
return new ArrayList<Branch>(parseBranches(commandOutput));
}
Expand Down
28 changes: 28 additions & 0 deletions src/test/java/org/jenkinsci/plugins/gitclient/GitAPITestCase.java
View file
Expand Up @@ -87,6 +87,12 @@ public abstract class GitAPITestCase extends TestCase {
private static final String LOGGING_STARTED = "Logging started";

private static final String SRC_DIR = (new File(".")).getAbsolutePath();
private String revParseBranchName = null;

private void createRevParseBranch() throws GitException, InterruptedException {
revParseBranchName = "rev-parse-branch-" + UUID.randomUUID().toString();
w.git.checkout("origin/master", revParseBranchName);
}

/**
* One local workspace of a Git repository on a temporary directory
Expand Down Expand Up @@ -306,6 +312,7 @@ protected void setExpectedTimeouts(List<Integer> timeouts) {

@Override
protected void setUp() throws Exception {
revParseBranchName = null;
setTimeoutVisibleInCurrentTest(true);
expectedTimeouts = null;
Logger logger = Logger.getLogger(this.getClass().getPackage().getName() + "-" + logCount++);
Expand Down Expand Up @@ -351,6 +358,20 @@ public String localMirror() throws IOException, InterruptedException {
throw new IllegalStateException();
}

/* JENKINS-33258 detected many calls to git rev-parse. This checks
* those calls are not being made. The createRevParseBranch call
* creates a branch whose name is unknown to the tests. This
* checks that the branch name is not mentioned in a call to
* git rev-parse.
*/
private void checkRevParseCalls(String branchName) {
String messages = StringUtils.join(handler.getMessages(), ";");
// Linux uses rev-parse without quotes
assertFalse("git rev-parse called: " + messages, handler.containsMessageSubstring("rev-parse " + branchName));
// Windows quotes the rev-parse argument
assertFalse("git rev-parse called: " + messages, handler.containsMessageSubstring("rev-parse \"" + branchName));
}

private void checkTimeout() {
List<Integer> timeouts = handler.getTimeouts();
if (expectedTimeouts == null) {
Expand Down Expand Up @@ -380,6 +401,9 @@ protected void tearDown() throws Exception {
if (getTimeoutVisibleInCurrentTest()) {
checkTimeout();
}
if (revParseBranchName != null) {
checkRevParseCalls(revParseBranchName);
}
} finally {
handler.close();
}
Expand Down Expand Up @@ -458,6 +482,7 @@ public void test_clone() throws IOException, InterruptedException
{
int newTimeout = 7;
w.git.clone_().timeout(newTimeout).url(localMirror()).repositoryName("origin").execute();
createRevParseBranch(); // Verify JENKINS-32258 is fixed
w.git.checkout("origin/master", "master");
check_remote_url("origin");
assertBranchesExist(w.git.getBranches(), "master");
Expand All @@ -480,6 +505,7 @@ public void test_clone_repositoryName() throws IOException, InterruptedException
public void test_clone_shallow() throws IOException, InterruptedException
{
w.git.clone_().url(localMirror()).repositoryName("origin").shallow().execute();
createRevParseBranch(); // Verify JENKINS-32258 is fixed
w.git.checkout("origin/master", "master");
check_remote_url("origin");
assertBranchesExist(w.git.getBranches(), "master");
Expand All @@ -500,6 +526,7 @@ public void test_clone_shallow_with_depth() throws IOException, InterruptedExcep
public void test_clone_shared() throws IOException, InterruptedException
{
w.git.clone_().url(localMirror()).repositoryName("origin").shared().execute();
createRevParseBranch(); // Verify JENKINS-32258 is fixed
w.git.checkout("origin/master", "master");
check_remote_url("origin");
assertBranchesExist(w.git.getBranches(), "master");
Expand All @@ -510,6 +537,7 @@ public void test_clone_shared() throws IOException, InterruptedException
public void test_clone_reference() throws IOException, InterruptedException
{
w.git.clone_().url(localMirror()).repositoryName("origin").reference(localMirror()).execute();
createRevParseBranch(); // Verify JENKINS-32258 is fixed
w.git.checkout("origin/master", "master");
check_remote_url("origin");
assertBranchesExist(w.git.getBranches(), "master");
Expand Down

0 comments on commit 89ab085

Please sign in to comment.