Skip to content
Permalink
Browse files

[Fix JENKINS-23299] tag based builds run at every poll

When a build is defined to use a tag, the "check for changes" uses
ls-remote to compare the SHA1 of the remote tag to the SHA1 of the HEAD
of the working directory.  The tag SHA1 is not always the same as the
commit SHA1 (annotated tags), so when that happens, the ls-remote based
"check for changes" can never be satisfied.

This change adapts the tag specific check for changes to use a syntax
which returns the SHA1 of the commit referenced by the tag rather than
the SHA1 of the tag.

JGit always returns the SHA1 of the commit referenced by a tag (a
"peeled" object), while CliGit returns a "peeled" object for tags only
if the refs/tags/tag_name syntax is used.

That difference is maintained for behavioral compatibility for users
of CliGit. Cases where returning the SHA1 of the tag would be better
than returning the SHA1 of the commit referenced by the tag seem rare,
but better to retain CliGit compatibility than risk surprising users in
some as yet undetected use case.
  • Loading branch information
MarkEWaite committed Sep 12, 2014
1 parent 96a49dd commit 4362cf66b19519a558984d33f7e4272131087b6a
@@ -2051,7 +2051,11 @@ public ObjectId getHeadRev(String url, String branchSpec) throws GitException, I
if (cred == null) cred = defaultCredentials;

args.add(url);
args.add(branchName);
if (branchName.startsWith("refs/tags/")) {
args.add(branchName+"^{}"); // JENKINS-23299 - tag SHA1 needs to be converted to commit SHA1
} else {
args.add(branchName);
}
String result = launchCommandWithCredentials(args, null, cred, url);
return result.length()>=40 ? ObjectId.fromString(result.substring(0, 40)) : null;
}
@@ -24,6 +24,7 @@
import hudson.remoting.VirtualChannel;
import hudson.util.IOUtils;
import junit.framework.TestCase;
import static org.junit.Assert.assertNotEquals;

import org.apache.commons.io.FileUtils;
import org.eclipse.jgit.internal.storage.file.FileRepository;
@@ -1187,13 +1188,43 @@ public void test_delete_branch() throws Exception {
}
}

@Bug(23299)
public void test_create_tag() throws Exception {
w.init();
String gitDir = w.repoPath() + File.separator + ".git";
w.commitEmpty("init");
ObjectId init = w.git.revParse("HEAD"); // Remember SHA1 of init commit
w.git.tag("test", "this is a tag");

/* JGit seems to have the better behavior in this case, always
* returning the SHA1 of the commit. Most users are using
* command line git, so the difference is retained in command
* line git for compatibility with any legacy command line git
* use cases which depend on returning the SHA-1 of the
* annotated tag rather than the SHA-1 of the commit to which
* the annotated tag points.
*/
ObjectId testTag = w.git.getHeadRev(gitDir, "test"); // Remember SHA1 of annotated test tag
if (w.git instanceof JGitAPIImpl) {
assertEquals("Annotated tag does not match SHA1", init, testTag);
} else {
assertNotEquals("Annotated tag unexpectedly equals SHA1", init, testTag);
}

/* Because refs/tags/test syntax is more specific than "test",
* and because the more specific syntax was only introduced in
* more recent git client plugin versions (like 1.10.0 and
* later), the CliGit and JGit behavior are kept the same here
* in order to fix JENKINS-23299.
*/
ObjectId testTagCommit = w.git.getHeadRev(gitDir, "refs/tags/test"); // SHA1 of commit identified by test tag
assertEquals("Annotated tag doesn't match queried commit SHA1", init, testTagCommit);
assertEquals(init, w.git.revParse("test")); // SHA1 of commit identified by test tag
assertEquals(init, w.git.revParse("refs/tags/test")); // SHA1 of commit identified by test tag
assertTrue("test tag not created", w.cmd("git tag").contains("test"));
String message = w.cmd("git tag -l -n1");
assertTrue("unexpected test tag message : " + message, message.contains("this is a tag"));
assertNull(w.git.getHeadRev(gitDir, "not-a-valid-tag")); // Confirm invalid tag returns null
}

public void test_delete_tag() throws Exception {
@@ -2088,10 +2119,16 @@ public void test_changelog_abort() throws InterruptedException, IOException
assertTrue("No SHA1 in " + writer.toString(), writer.toString().contains(sha1));
}

@Bug(23299)
public void test_getHeadRev() throws Exception {
Map<String, ObjectId> heads = w.git.getHeadRev(remoteMirrorURL);
ObjectId master = w.git.getHeadRev(remoteMirrorURL, "refs/heads/master");
assertEquals("URL is " + remoteMirrorURL + ", heads is " + heads, master, heads.get("refs/heads/master"));

/* Test with a specific tag reference - JENKINS-23299 */
ObjectId knownTag = w.git.getHeadRev(remoteMirrorURL, "refs/tags/git-client-1.10.0");
ObjectId expectedTag = ObjectId.fromString("1fb23708d6b639c22383c8073d6e75051b2a63aa"); // commit SHA1
assertEquals("Wrong SHA1 for git-client-1.10.0 tag", expectedTag, knownTag);
}

/**

0 comments on commit 4362cf6

Please sign in to comment.
You can’t perform that action at this time.