Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-46597] Further handling of serializability issues
Specifically: the checkout step returns a TreeMap, so let's make sure
we never have a variable assigned directly to that return
value. Probably want to change checkout to return a HashMap in the
future, but we'll cross that bridge later.

Also updated workflow-cps dependency so that we can be sure
BooleanClosureWrapper serialization issues won't crop up, letting us
write more things as .every or .any.
  • Loading branch information
abayer committed Nov 13, 2017
1 parent b139483 commit 107bb5c
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 28 deletions.
Expand Up @@ -139,8 +139,8 @@ public class ModelInterpreter implements Serializable {
def getParallelStages(Root root, Agent parentAgent, Stage thisStage, Throwable firstError, Stage parentStage,
boolean skippedForFailure, boolean skippedForUnstable, boolean skippedForWhen) {
def parallelStages = [:]
for (int i = 0; i < thisStage.parallel.stages.size(); i++) {
Stage parallelStage = thisStage.parallel.getStages().get(i)

thisStage.parallel.stages.each { parallelStage ->
if (skippedForFailure) {
parallelStages.put(parallelStage.name, {
script.stage(parallelStage.name) {
Expand Down Expand Up @@ -520,7 +520,7 @@ public class ModelInterpreter implements Serializable {
// To allow for referencing environment variables that have not yet been declared pre-parse time, we need
// to actually instantiate the conditional now, via a closure.
return instancesFromClosure(when.rawClosure, DeclarativeStageConditional.class).every {
it.getScript(script).evaluate()
it?.getScript(script)?.evaluate()
}
}
}
Expand Down
Expand Up @@ -25,7 +25,6 @@

package org.jenkinsci.plugins.pipeline.modeldefinition.when.impl

import org.jenkinsci.plugins.pipeline.modeldefinition.when.DeclarativeStageConditional
import org.jenkinsci.plugins.pipeline.modeldefinition.when.DeclarativeStageConditionalScript
import org.jenkinsci.plugins.workflow.cps.CpsScript

Expand All @@ -37,15 +36,8 @@ class AllOfConditionalScript extends DeclarativeStageConditionalScript<AllOfCond

@Override
public boolean evaluate() {
List<DeclarativeStageConditional<? extends DeclarativeStageConditional>> children = describable.children
for (int i = 0; i < children.size(); i++) {
DeclarativeStageConditional n = children.get(i)
DeclarativeStageConditionalScript s = (DeclarativeStageConditionalScript)n?.getScript(script)
if (s == null || !s.evaluate()) {
return false
}
return describable.children.every {
it?.getScript(script)?.evaluate()
}

return true
}
}
Expand Up @@ -25,7 +25,6 @@

package org.jenkinsci.plugins.pipeline.modeldefinition.when.impl

import org.jenkinsci.plugins.pipeline.modeldefinition.when.DeclarativeStageConditional
import org.jenkinsci.plugins.pipeline.modeldefinition.when.DeclarativeStageConditionalScript
import org.jenkinsci.plugins.workflow.cps.CpsScript

Expand All @@ -37,15 +36,8 @@ class AnyOfConditionalScript extends DeclarativeStageConditionalScript<AnyOfCond

@Override
public boolean evaluate() {
List<DeclarativeStageConditional<? extends DeclarativeStageConditional>> children = describable.children
for (int i = 0; i < children.size(); i++) {
DeclarativeStageConditional n = children.get(i)
DeclarativeStageConditionalScript s = (DeclarativeStageConditionalScript)n?.getScript(script)
if (s != null && s.evaluate()) {
return true
}
return describable.children.any {
it?.getScript(script)?.evaluate()
}

return false
}
}
Expand Up @@ -90,6 +90,7 @@ public class BasicModelDefTest extends AbstractModelDefTest {
@BeforeClass
public static void setUpAgent() throws Exception {
s = j.createOnlineSlave();
s.setNumExecutors(10);
s.setLabelString("some-label docker");
}

Expand Down Expand Up @@ -1235,4 +1236,12 @@ public void parallelStagesHaveStatusWhenSkipped() throws Exception {
parentTags.getTags().get(Utils.getStageStatusMetadata().getTagName()));
}

@Issue("JENKINS-46597")
@Test
public void parallelStagesShoudntTriggerNSE() throws Exception {
expect("parallelStagesShouldntTriggerNSE")
.logContains("ninth branch")
.go();
}

}
@@ -0,0 +1,158 @@
/*
* The MIT License
*
* Copyright (c) 2017, CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

pipeline {
agent none
parameters {
booleanParam(name: 'shouldRun', defaultValue: false, description: "a param")
}
stages {
stage("bar") {
steps {
echo "We're ok"
sleep 2
}
}
stage("foo") {
parallel {
stage("first") {
agent any
when {
expression {
sleep 1
return params.shouldRun == true
}
}
steps {
echo "First branch"
}
}
stage("second") {
agent any
when {
expression {
return params.shouldRun == true
}
}
steps {
echo "Second branch"
}
}
stage("third") {
agent any
when {
expression {
return params.shouldRun == true
}
}
steps {
echo "third branch"
}
}
stage("fourth") {
agent any
when {
expression {
return params.shouldRun == true
}
}
steps {
echo "fourth branch"
}
}
stage("fifth") {
agent any
when {
expression {
return params.shouldRun == true
}
}
steps {
echo "Fifth branch"
}
}
stage("sixth") {
agent any
when {
expression {
return params.shouldRun == true
}
}
steps {
echo "sixth branch"
}
}
stage("seventh") {
agent any
when {
expression {
return params.shouldRun == true
}
}
steps {
echo "seventh branch"
}
}
stage("eighth") {
agent any
when {
expression {
return params.shouldRun == true
}
}
steps {
echo "eighth branch"
}
}
stage("ninth") {
agent any
when {
expression {
return params.shouldRun == false
}
}
steps {
echo "ninth branch"
}
}
}
post {
always {
sleep 2
echo "HUH"
}
}
}
}
post {
always {
sleep 1
echo "ok"
}
}
}




Expand Up @@ -44,15 +44,15 @@ public class CheckoutScript implements Serializable {

private static Closure checkoutAndRun(CpsScript script, DeclarativeAgent agent, Closure body) {
return {
def checkoutMap
def checkoutMap = [:]
if (agent.isDoCheckout() && agent.hasScmContext(script)) {
if (!agent.inStage) {
script.stage(SyntheticStageNames.checkout()) {
checkoutMap = script.checkout script.scm
checkoutMap.putAll(script.checkout(script.scm) ?: [:])
}
} else {
// No stage when we're in a nested stage already
checkoutMap = script.checkout script.scm
checkoutMap.putAll(script.checkout(script.scm) ?: [:])
}
}
if (checkoutMap) {
Expand Down
4 changes: 2 additions & 2 deletions pom.xml
Expand Up @@ -75,7 +75,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-cps</artifactId>
<version>2.36.1</version>
<version>2.41</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
Expand Down Expand Up @@ -332,7 +332,7 @@
</build>

<properties>
<jenkins.version>2.7.1</jenkins.version>
<jenkins.version>2.7.3</jenkins.version>
<java.level>7</java.level>
<groovy.version>2.4.7</groovy.version>
<concurrency>0.5C</concurrency>
Expand Down

0 comments on commit 107bb5c

Please sign in to comment.