Skip to content
Permalink
Browse files

Merge pull request #324 from ndeloof/JENKINS-28506

[FIXED JENKINS-28506] - Git fails to trigger from polling for refs/heads/ style branch name
  • Loading branch information
ndeloof committed Jun 4, 2015
2 parents 06415b9 + f53bbf2 commit c9a8e11fd7f63f6bda64c0cefd430e1a9ae32db5
@@ -60,8 +60,21 @@ public boolean matches(String item) {
return matches(item, env);
}

public boolean matches(String item, EnvVars env) {
return getPattern(env).matcher(item).matches();
/**
* Compare a git branch reference to configured pattern.
* <p>
* reference uses normalized format `ref/(heads|tags)/xx`
* pattern do support
* <ul>
* <li>ref/heads/branch</li>
* <li>(remote-name)?/branch</li>
* <li>ref/remotes/branch</li>
* <li>tag</li>
* <li>(commit sha1)</li>
* </ul>
*/
public boolean matches(String ref, EnvVars env) {
return getPattern(env).matcher(ref).matches();
}

/**
@@ -110,26 +123,28 @@ private Pattern getPattern(EnvVars env) {
String regexSubstring = expandedName.substring(1, expandedName.length());
return Pattern.compile(regexSubstring);
}

// if an unqualified branch was given add a "*/" so it will match branches
// from remote repositories as the user probably intended
String qualifiedName;
if (!expandedName.contains("**") && !expandedName.contains("/"))
qualifiedName = "*/" + expandedName;
else
qualifiedName = expandedName;


// build a pattern into this builder
StringBuilder builder = new StringBuilder();

// for legacy reasons (sic) we do support various branch spec format to declare remotes / branches
builder.append("(refs/heads/|refs/remotes/|remotes/)?");
builder.append("(refs/heads/");


// if an unqualified branch was given, consider all remotes (with various possible syntaxes)
// so it will match branches from any remote repositories as the user probably intended
if (!expandedName.contains("**") && !expandedName.contains("/")) {
builder.append("|refs/remotes/[^/]+/|remotes/[^/]+/|[^/]+/");
} else {
builder.append("|refs/remotes/|remotes/");
}
builder.append(")?");

// was the last token a wildcard?
boolean foundWildcard = false;

// split the string at the wildcards
StringTokenizer tokenizer = new StringTokenizer(qualifiedName, "*", true);
StringTokenizer tokenizer = new StringTokenizer(expandedName, "*", true);
while (tokenizer.hasMoreTokens()) {
String token = tokenizer.nextToken();

@@ -545,6 +545,8 @@ private static Node workspaceToNode(FilePath workspace) { // TODO https://trello
return j;
}

public static final Pattern GIT_REF = Pattern.compile("(refs/[^/]+)/.*");

private PollingResult compareRemoteRevisionWithImpl(Job<?, ?> project, Launcher launcher, FilePath workspace, final TaskListener listener) throws IOException, InterruptedException {
// Poll for changes. Are there any unbuilt revisions that Hudson ought to build ?

@@ -583,22 +585,28 @@ private PollingResult compareRemoteRevisionWithImpl(Job<?, ?> project, Launcher
for (BranchSpec branchSpec : getBranches()) {
for (Entry<String, ObjectId> entry : heads.entrySet()) {
final String head = entry.getKey();
String name;
// head is "refs/(heads|tags)/branchName
if (head.startsWith("refs/heads/")) name = remote + "/" + head.substring(11);
else if (head.startsWith("refs/tags/")) name = remote + "/" + head.substring(10);
else name = remote + "/" + head;

if (!branchSpec.matches(name, environment)) continue;
// first, check the a canonical git reference is configured
if (!branchSpec.matches(head, environment)) {

// convert head `refs/(heads|tags|whatever)/branch` into shortcut notation `remote/branch`
String name = head;
Matcher matcher = GIT_REF.matcher(head);
if (matcher.matches()) name = remote + head.substring(matcher.group(1).length());
else name = remote + "/" + head;

if (!branchSpec.matches(name, environment)) continue;
}

final ObjectId sha1 = entry.getValue();
Build built = buildData.getLastBuild(sha1);
if (built != null) {
listener.getLogger().println("[poll] Latest remote head revision on " + name + " is: " + sha1.getName() + " - already built by " + built.getBuildNumber());
listener.getLogger().println("[poll] Latest remote head revision on " + head + " is: " + sha1.getName() + " - already built by " + built.getBuildNumber());
continue;
}

listener.getLogger().println("[poll] Latest remote head revision on " + name + " is: " + sha1.getName());
listener.getLogger().println("[poll] Latest remote head revision on " + head + " is: " + sha1.getName());
return BUILD_NOW;
}
}
@@ -3,16 +3,14 @@
import hudson.plugins.git.extensions.GitClientType;
import hudson.plugins.git.extensions.impl.EnforceGitClient;

public class CliGitSCMTriggerRemotePollTest extends SCMTriggerTest
{
/**
* Remote polling and local polling behave differently due to bugs in productive
* code which probably cannot be fixed without serious compatibility problems.
* The isChangeExpected() method adjusts the tests to the difference between
* local and remote polling.
*/
public class CliGitSCMTriggerRemotePollTest extends SCMTriggerTest {

/**
* Currently some tests still fail due to bugs in productive code.
* TODO: Fix bugs and enable tests.
*/
private boolean SKIP_FAILING_TESTS = true;


@Override
protected EnforceGitClient getGitClient()
{
@@ -24,52 +22,5 @@ protected boolean isDisableRemotePoll()
{
return false;
}

@Override
public void testNamespaces_with_master() throws Exception {
if(SKIP_FAILING_TESTS) return; //TODO Fix productive code
super.testNamespaces_with_master();
}

@Override
public void testNamespaces_with_namespace1Master() throws Exception {
//This one works by accident! ls-remote lists this as first entry
super.testNamespaces_with_namespace1Master();
}

@Override
public void testNamespaces_with_namespace2Master() throws Exception {
if(SKIP_FAILING_TESTS) return; //TODO Fix productive code
super.testNamespaces_with_namespace2Master();
}

@Override
public void testCommitAsBranchSpec() throws Exception {
if(SKIP_FAILING_TESTS) return; //TODO Fix productive code
super.testCommitAsBranchSpec();
}

@Override
public void testTags_with_TagA() throws Exception {
if(SKIP_FAILING_TESTS) return; //TODO Fix productive code
super.testTags_with_TagA();
}

@Override
public void testTags_with_TagBAnnotated() throws Exception {
if(SKIP_FAILING_TESTS) return; //TODO Fix productive code
super.testTags_with_TagBAnnotated();
}

/* Skip this test because git client 1.10.2 and later include a fix for
* JENKINS-23299. The fix resolves refs/tags/tag_name as the commit to
* which tag_name points. Prior to that change, the ref pointed to the
* SHA-1 of the tag, instead of the SHA-1 of the commit to which the tag
* points. Because of that bug fix, the git plugin correctly detects
* refs/tags/TagA as needing to build.
*/
@Override
public void testTags_with_refsTagsTagA() throws Exception {
return;
}
}
@@ -3,16 +3,14 @@
import hudson.plugins.git.extensions.GitClientType;
import hudson.plugins.git.extensions.impl.EnforceGitClient;

/**
* Remote polling and local polling behave differently due to bugs in productive
* code which probably cannot be fixed without serious compatibility problems.
* The isChangeExpected() method adjusts the tests to the difference between
* local and remote polling.
*/
public class JGitSCMTriggerRemotePollTest extends SCMTriggerTest
{

/**
* Currently some tests still fail due to bugs in productive code.
* TODO: Fix bugs and enable tests.
*/
private boolean SKIP_FAILING_TESTS = true;


@Override
protected EnforceGitClient getGitClient()
{
@@ -25,22 +23,4 @@ protected boolean isDisableRemotePoll()
return false;
}

@Override
public void testNamespaces_with_master() throws Exception {
if(SKIP_FAILING_TESTS) return; //TODO Fix productive code
super.testNamespaces_with_master();
}

@Override
public void testNamespaces_with_namespace2Master() throws Exception {
if(SKIP_FAILING_TESTS) return; //TODO Fix productive code
super.testNamespaces_with_namespace2Master();
}

@Override
public void testCommitAsBranchSpec() throws Exception {
if(SKIP_FAILING_TESTS) return; //TODO Fix productive code
super.testCommitAsBranchSpec();
}

}
}
@@ -22,8 +22,6 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.FutureTask;
import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.zip.ZipEntry;
@@ -39,6 +37,7 @@
private ZipFile namespaceRepoZip;
private Properties namespaceRepoCommits;
private ExecutorService singleThreadExecutor;
protected boolean expectChanges = false;

@Override
protected void tearDown() throws Exception
@@ -59,6 +58,7 @@ protected void tearDown() throws Exception
@Override
public void setUp() throws Exception {
super.setUp();
expectChanges = false;
namespaceRepoZip = new ZipFile("src/test/resources/namespaceBranchRepo.zip");
namespaceRepoCommits = parseLsRemote(new File("src/test/resources/namespaceBranchRepo.ls-remote"));
tempAllocator = new TemporaryDirectoryAllocator();
@@ -68,7 +68,7 @@ public void setUp() throws Exception {
protected abstract EnforceGitClient getGitClient();

protected abstract boolean isDisableRemotePoll();

public void testNamespaces_with_refsHeadsMaster() throws Exception {
check(namespaceRepoZip, namespaceRepoCommits,
"refs/heads/master",
@@ -124,7 +124,35 @@ public void testNamespaces_with_refsHeadsNamespace2Master() throws Exception {
namespaceRepoCommits.getProperty("refs/heads/a_tests/b_namespace2/master"),
"origin/a_tests/b_namespace2/master");
}


public void testNamespaces_with_namespace3_feature3_sha1() throws Exception {
check(namespaceRepoZip, namespaceRepoCommits,
namespaceRepoCommits.getProperty("refs/heads/a_tests/b_namespace3/feature3"),
namespaceRepoCommits.getProperty("refs/heads/a_tests/b_namespace3/feature3"),
"detached");
}

public void testNamespaces_with_namespace3_feature3_branchName() throws Exception {
check(namespaceRepoZip, namespaceRepoCommits,
"a_tests/b_namespace3/feature3",
namespaceRepoCommits.getProperty("refs/heads/a_tests/b_namespace3/feature3"),
"origin/a_tests/b_namespace3/feature3");
}

public void testNamespaces_with_refsHeadsNamespace3_feature3_sha1() throws Exception {
check(namespaceRepoZip, namespaceRepoCommits,
namespaceRepoCommits.getProperty("refs/heads/a_tests/b_namespace3/feature3"),
namespaceRepoCommits.getProperty("refs/heads/a_tests/b_namespace3/feature3"),
"detached");
}

public void testNamespaces_with_refsHeadsNamespace3_feature3_branchName() throws Exception {
check(namespaceRepoZip, namespaceRepoCommits,
"refs/heads/a_tests/b_namespace3/feature3",
namespaceRepoCommits.getProperty("refs/heads/a_tests/b_namespace3/feature3"),
"origin/a_tests/b_namespace3/feature3");
}

public void testTags_with_TagA() throws Exception {
check(namespaceRepoZip, namespaceRepoCommits,
"TagA",
@@ -153,6 +181,20 @@ public void testTags_with_refsTagsTagBAnnotated() throws Exception {
"refs/tags/TagBAnnotated");
}

public void testCommitAsBranchSpec_feature4_sha1() throws Exception {
check(namespaceRepoZip, namespaceRepoCommits,
namespaceRepoCommits.getProperty("refs/heads/b_namespace3/feature4"),
namespaceRepoCommits.getProperty("refs/heads/b_namespace3/feature4"),
"detached");
}

public void testCommitAsBranchSpec_feature4_branchName() throws Exception {
check(namespaceRepoZip, namespaceRepoCommits,
"refs/heads/b_namespace3/feature4",
namespaceRepoCommits.getProperty("refs/heads/b_namespace3/feature4"),
"origin/b_namespace3/feature4");
}

public void testCommitAsBranchSpec() throws Exception {
check(namespaceRepoZip, namespaceRepoCommits,
namespaceRepoCommits.getProperty("refs/heads/b_namespace3/master"),
@@ -179,7 +221,7 @@ public void check(ZipFile repoZip, Properties commits, String branchSpec,
assertNotNull("Job has not been triggered", build1);

PollingResult poll = project.poll(listener);
assertFalse("Polling found new changes although nothing new", poll.hasChanges());
assertEquals("Expected and actual polling results disagree", false, poll.hasChanges());

//Speedup test - avoid waiting 1 minute
triggerSCMTrigger(project.getTrigger(SCMTrigger.class)).get(20, SECONDS);
@@ -12,7 +12,7 @@ public void testMatch() {
BranchSpec l = new BranchSpec("master");
Assert.assertTrue(l.matches("origin/master"));
Assert.assertFalse(l.matches("origin/something/master"));
Assert.assertFalse(l.matches("master"));
Assert.assertTrue(l.matches("master"));
Assert.assertFalse(l.matches("dev"));


@@ -67,7 +67,7 @@ public void testMatchEnv() {
BranchSpec l = new BranchSpec("${master}");
Assert.assertTrue(l.matches("origin/master", env));
Assert.assertFalse(l.matches("origin/something/master", env));
Assert.assertFalse(l.matches("master", env));
Assert.assertTrue(l.matches("master", env));
Assert.assertFalse(l.matches("dev", env));


@@ -1,7 +1,9 @@
86e6eeccc0b3a5e1c8034ff51718b8843a755789 HEAD
1aeaf8635d2e419a6e9587fd1ed1ddcd445845d9 refs/heads/a_tests/b_namespace1/master
3e73b26f220d8ad3e517858ffbe83b837d7f04c5 refs/heads/a_tests/b_namespace2/master
73d4779eae7b6aed8191635f5debc2b37ad083d0 refs/heads/a_tests/b_namespace3/feature3
d940b841b7a5488ab42772f263b107a58eba2621 refs/heads/a_tests/b_namespace3/master
74ae8c24eb2783794d969e7f1df7260a8aa06d6b refs/heads/b_namespace3/feature4
0b97e49ddbf699cf7b6deb31982d9d568d5e30f4 refs/heads/b_namespace3/master
b00d0c59cf79da494640788cb23453a0315b9c41 refs/heads/branchForTagA
dde47cb9b577cb6b50597931cdead4255669f322 refs/heads/branchForTagBAnnotated
Binary file not shown.

0 comments on commit c9a8e11

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