[JENKINS-27352] Fix commit notification for a parametrized branchspec

When the branchspec is parametrized, and a commit notification is received for the tracked repository, the polling is always triggered, even if a sha1 was received. Also, add some FINE logs to the notifyCommit code.
jenkinsci · Jul 11, 2015 · cc2ddb7 · cc2ddb7
1 parent 3a83b2c
commit cc2ddb7
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 6 deletions.
diff --git a/src/main/java/hudson/plugins/git/GitStatus.java b/src/main/java/hudson/plugins/git/GitStatus.java
@@ -13,6 +13,7 @@
 import java.io.PrintWriter;
 import java.net.URISyntaxException;
 import java.util.*;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -202,6 +203,10 @@ public static class JenkinsAbstractProjectListener extends Listener {
          */
         @Override
         public List<ResponseContributor> onNotifyCommit(URIish uri, String sha1, List<ParameterValue> buildParameters, String... branches) {
+            if (LOGGER.isLoggable(Level.FINE)) {
+                LOGGER.fine("Received notification for uri = " + uri + " ; sha1 = " + sha1 + " ; branches = " + Arrays.toString(branches));
+            }
+
             List<ResponseContributor> result = new ArrayList<ResponseContributor>();
             // run in high privilege to see all the projects anonymous users don't see.
             // this is safe because when we actually schedule a build, it's a build that can
@@ -243,15 +248,28 @@ public List<ResponseContributor> onNotifyCommit(URIish uri, String sha1, List<Pa
                                 continue;
                             }
 
-                            Boolean branchFound = false;
+                            boolean branchFound = false,
+                                    parametrizedBranchSpec = false;
                             if (branches.length == 0) {
                                 branchFound = true;
                             } else {
                                 OUT: for (BranchSpec branchSpec : git.getBranches()) {
-                                    for (String branch : branches) {
-                                        if (branchSpec.matches(repository.getName() + "/" + branch)) {
-                                            branchFound = true;
-                                            break OUT;
+                                    if (branchSpec.getName().contains("$")) {
+                                        // If the branchspec is parametrized, always run the polling
+                                        if (LOGGER.isLoggable(Level.FINE)) {
+                                            LOGGER.fine("Branch Spec is parametrized for " + project.getFullDisplayName() + ". ");
+                                        }
+                                        branchFound = true;
+                                        parametrizedBranchSpec = true;
+                                    } else {
+                                        for (String branch : branches) {
+                                            if (branchSpec.matches(repository.getName() + "/" + branch)) {
+                                                if (LOGGER.isLoggable(Level.FINE)) {
+                                                    LOGGER.fine("Branch Spec " + branchSpec + " matches modified branch " + branch + " for " + project.getFullDisplayName() + ". ");
+                                                }
+                                                branchFound = true;
+                                                break OUT;
+                                            }
                                         }
                                     }
                                 }
@@ -260,7 +278,7 @@ public List<ResponseContributor> onNotifyCommit(URIish uri, String sha1, List<Pa
                             urlFound = true;
 
                             if (!(project instanceof AbstractProject && ((AbstractProject) project).isDisabled())) {
-                                if (isNotEmpty(sha1)) {
+                                if (!parametrizedBranchSpec && isNotEmpty(sha1)) {
                                     LOGGER.info("Scheduling " + project.getFullDisplayName() + " to build commit " + sha1);
                                     scmTriggerItem.scheduleBuild2(scmTriggerItem.getQuietPeriod(),
                                             new CauseAction(new CommitHookCause(sha1)),

diff --git a/src/test/java/hudson/plugins/git/GitStatusTest.java b/src/test/java/hudson/plugins/git/GitStatusTest.java
@@ -128,6 +128,18 @@ public void testDoNotifyCommitWithNoMatchingBranches() throws Exception {
         Mockito.verify(bTopicTrigger, Mockito.never()).run();
     }
 
+    @Test
+    public void testDoNotifyCommitWithParametrizedBranch() throws Exception {
+        SCMTrigger aMasterTrigger = setupProject("a", "$BRANCH_TO_BUILD", false);
+        SCMTrigger bMasterTrigger = setupProject("b", "master", false);
+        SCMTrigger bTopicTrigger = setupProject("b", "topic", false);
+
+        this.gitStatus.doNotifyCommit(requestWithNoParameter, "a", "master", null);
+        Mockito.verify(aMasterTrigger).run();
+        Mockito.verify(bMasterTrigger, Mockito.never()).run();
+        Mockito.verify(bTopicTrigger, Mockito.never()).run();
+    }
+
     @Test
     public void testDoNotifyCommitWithIgnoredRepository() throws Exception {
         SCMTrigger aMasterTrigger = setupProject("a", "master", true);