[FIXED JENKINS-30798] SCMBinder was calling the wrong overload of SCM…

…Source.fetch. There is no need to consider SCMSourceCriteria here, nor to inspect unrelated branches. It was also not dealing gracefully with problems such as deleted branches.
jenkinsci · Nov 2, 2015 · 1d791a9 · 1d791a9
1 parent 00c8c62
commit 1d791a9
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 8 deletions.
diff --git a/cps/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinition.java b/cps/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScmFlowDefinition.java
@@ -24,6 +24,7 @@
 
 package org.jenkinsci.plugins.workflow.cps;
 
+import hudson.AbortException;
 import hudson.Extension;
 import hudson.FilePath;
 import hudson.model.Action;
@@ -40,7 +41,6 @@
 import java.io.IOException;
 import java.util.Collection;
 import java.util.List;
-import java.util.Properties;
 import javax.inject.Inject;
 import jenkins.model.Jenkins;
 import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn;
@@ -111,6 +111,9 @@ public String getScriptPath() {
             if (!scriptFile.absolutize().getRemote().replace('\\', '/').startsWith(dir.absolutize().getRemote().replace('\\', '/') + '/')) { // TODO JENKINS-26838
                 throw new IOException(scriptFile + " is not inside " + dir);
             }
+            if (!scriptFile.exists()) {
+                throw new AbortException(scriptFile + " not found");
+            }
             script = scriptFile.readToString();
         } finally {
             lease.release();

diff --git a/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/SCMBinder.java b/multibranch/src/main/java/org/jenkinsci/plugins/workflow/multibranch/SCMBinder.java
@@ -84,14 +84,15 @@ class SCMBinder extends FlowDefinition {
             throw new IllegalStateException(branch.getSourceId() + " not found");
         }
         SCMHead head = branch.getHead();
-        SCMRevision tip = scmSource.fetch(SCMHeadObserver.select(head), listener).result();
-        if (tip == null) {
-            // TODO observed (but not now reproducible) after trying to rebuild projects without rerunning branch indexing
-            // Perhaps because above we are calling the wrong `fetch` overload? (Simpler to pass SCMHead + TaskListener.)
-            throw new IllegalStateException("could not find branch tip on " + head);
+        SCMRevision tip = scmSource.fetch(head, listener);
+        SCM scm;
+        if (tip != null) {
+            scm = scmSource.build(head, tip);
+        } else {
+            listener.error("Could not determine exact tip revision of " + branch.getName() + "; falling back to nondeterministic checkout");
+            // Build might fail later anyway, but reason should become clear: for example, branch was deleted before indexing could run.
+            scm = branch.getScm();
         }
-        SCM scm = scmSource.build(head, tip);
-        // Fallback if one is needed: scm = branch.getScm()
         CpsFlowExecution execution = new CpsScmFlowDefinition(scm, WorkflowMultiBranchProject.SCRIPT).create(handle, listener, actions);
         scms.put(execution, scm); // stash for later
         return execution;

diff --git a/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/SCMBinderTest.java b/multibranch/src/test/java/org/jenkinsci/plugins/workflow/multibranch/SCMBinderTest.java
@@ -25,6 +25,8 @@
 package org.jenkinsci.plugins.workflow.multibranch;
 
 import hudson.ExtensionList;
+import hudson.Util;
+import hudson.model.Result;
 import hudson.model.RootAction;
 import hudson.plugins.mercurial.MercurialInstallation;
 import hudson.plugins.mercurial.MercurialSCMSource;
@@ -229,4 +231,62 @@ public class SCMBinderTest {
         });
     }
 
+    @Test public void deletedJenkinsfile() throws Exception {
+        story.addStep(new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                sampleGitRepo.init();
+                sampleGitRepo.write("Jenkinsfile", "node { echo 'Hello World' }");
+                sampleGitRepo.git("add", "Jenkinsfile");
+                sampleGitRepo.git("commit", "--all", "--message=flow");
+                WorkflowMultiBranchProject mp = story.j.jenkins.createProject(WorkflowMultiBranchProject.class, "p");
+                mp.getSourcesList().add(new BranchSource(new GitSCMSource(null, sampleGitRepo.toString(), "", "*", "", false), new DefaultBranchPropertyStrategy(new BranchProperty[0])));
+                WorkflowJob p = WorkflowMultiBranchProjectTest.scheduleAndFindBranchProject(mp, "master");
+                assertEquals(1, mp.getItems().size());
+                story.j.waitUntilNoActivity();
+                WorkflowRun b1 = p.getLastBuild();
+                assertEquals(1, b1.getNumber());
+                sampleGitRepo.git("rm", "Jenkinsfile");
+                sampleGitRepo.git("commit", "--all", "--message=remove");
+                WorkflowRun b2 = story.j.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get());
+                story.j.assertLogContains("Jenkinsfile not found", b2);
+            }
+        });
+    }
+
+    @Test public void deletedBranch() throws Exception {
+        story.addStep(new Statement() {
+            @Override
+            public void evaluate() throws Throwable {
+                sampleGitRepo.init();
+                // TODO GitSCMSource offers no way to set a GitSCMExtension such as CleanBeforeCheckout; work around with deleteDir
+                // (without cleaning, b2 will succeed since the workspace will still have a cached origin/feature ref)
+                sampleGitRepo.write("Jenkinsfile", "node {deleteDir(); checkout scm; echo 'Hello World'}");
+                sampleGitRepo.git("add", "Jenkinsfile");
+                sampleGitRepo.git("commit", "--all", "--message=flow");
+                sampleGitRepo.git("checkout", "-b", "feature");
+                sampleGitRepo.write("somefile", "stuff");
+                sampleGitRepo.git("add", "somefile");
+                sampleGitRepo.git("commit", "--all", "--message=tweaked");
+                WorkflowMultiBranchProject mp = story.j.jenkins.createProject(WorkflowMultiBranchProject.class, "p");
+                mp.getSourcesList().add(new BranchSource(new GitSCMSource(null, sampleGitRepo.toString(), "", "*", "", false), new DefaultBranchPropertyStrategy(new BranchProperty[0])));
+                WorkflowJob p = WorkflowMultiBranchProjectTest.scheduleAndFindBranchProject(mp, "feature");
+                assertEquals(2, mp.getItems().size());
+                story.j.waitUntilNoActivity();
+                WorkflowRun b1 = p.getLastBuild();
+                assertEquals(1, b1.getNumber());
+                sampleGitRepo.git("checkout", "master");
+                sampleGitRepo.git("branch", "-D", "feature");
+                { // TODO AbstractGitSCMSource.retrieve is incorrect: after fetching remote refs into the cache, the origin/feature ref remains locally even though it has been deleted upstream:
+                    Util.deleteRecursive(new File(story.j.jenkins.getRootDir(), "caches"));
+                }
+                WorkflowRun b2 = story.j.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get());
+                story.j.assertLogContains("nondeterministic checkout", b2); // SCMBinder
+                story.j.assertLogContains("any revision to build", b2); // checkout scm
+                mp.scheduleBuild2(0).getFuture().get();
+                assertEquals(1, mp.getItems().size());
+            }
+        });
+    }
+
 }