diff --git a/pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/RuntimeASTTransformer.groovy b/pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/RuntimeASTTransformer.groovy index a14065bfb..b6af7de70 100644 --- a/pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/RuntimeASTTransformer.groovy +++ b/pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/RuntimeASTTransformer.groovy @@ -70,8 +70,8 @@ import static org.objectweb.asm.Opcodes.ACC_PUBLIC class RuntimeASTTransformer { /** - * Enables or disables the script splitting behavior in {@Wrapper} which - * mitigates "Method code too large" and "Class too large" errors. + * Enables or disables the script splitting behavior in {@Wrapper} which + * mitigates "Method code too large" and "Class too large" errors. */ @SuppressFBWarnings(value="MS_SHOULD_BE_FINAL", justification="For access from script console") public static boolean SCRIPT_SPLITTING_TRANSFORMATION = SystemProperties.getBoolean( @@ -1256,48 +1256,48 @@ class RuntimeASTTransformer { @NonNull private List prepareClosureScopedHandles(@NonNull BlockStatement pipelineBlock) { ArrayList result = new ArrayList() - if (SCRIPT_SPLITTING_TRANSFORMATION) { - ArrayList declarations = new ArrayList() - moduleNode.statementBlock.statements.each { item -> - if (item instanceof ExpressionStatement) { - ExpressionStatement es = (ExpressionStatement) item - if (es.expression instanceof DeclarationExpression) { - declarations.add((DeclarationExpression) es.expression) - } + ArrayList declarations = new ArrayList() + moduleNode.statementBlock.statements.each { item -> + if (item instanceof ExpressionStatement) { + ExpressionStatement es = (ExpressionStatement) item + if (es.expression instanceof DeclarationExpression) { + declarations.add((DeclarationExpression) es.expression) } } + } - if (!declarations.isEmpty()) { - if (SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES) { - result.addAll(pipelineElementHandles) - pipelineElementHandles.clear() - } else { - def declarationNames = [] - declarations.each { item -> - def left = item.getLeftExpression() - if (left instanceof VariableExpression) { - declarationNames.add(((VariableExpression)left).getName()) - } else if (left instanceof ArgumentListExpression) { - left.each { arg -> - if (arg instanceof VariableExpression) { - declarationNames.add(((VariableExpression) arg).getName()) - } else { - declarationNames.add("Unrecognized expression: " + arg.toString()) - } + // if we're not doing script splitting, keep the old behavior that preserves declared variable functionality in matrix + if (!declarations.isEmpty()) { + result.addAll(pipelineElementHandles) + pipelineElementHandles.clear() + + if (SCRIPT_SPLITTING_TRANSFORMATION && !SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES) { + def declarationNames = [] + declarations.each { item -> + def left = item.getLeftExpression() + if (left instanceof VariableExpression) { + declarationNames.add(((VariableExpression) left).getName()) + } else if (left instanceof ArgumentListExpression) { + left.each { arg -> + if (arg instanceof VariableExpression) { + declarationNames.add(((VariableExpression) arg).getName()) + } else { + declarationNames.add("Unrecognized expression: " + arg.toString()) } - } else { - declarationNames.add("Unrecognized declaration structure: " + left.toString()) } + } else { + declarationNames.add("Unrecognized declaration structure: " + left.toString()) } - throw new IllegalStateException("[JENKINS-34987] SCRIPT_SPLITTING_TRANSFORMATION is an experimental feature of Declarative Pipeline and is incompatible with local variable declarations inside a Jenkinsfile. " + - "As a temporary workaround, you can add the '@Field' annotation to these local variable declarations. " + - "However, use of Groovy variables in Declarative pipeline, with or without the '@Field' annotation, is not recommended or supported. " + - "To use less effective script splitting which allows local variable declarations without changing your pipeline code, set SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES=true . " + - "Local variable declarations found: " + declarationNames.sort().join(", ") + ". ") } + throw new IllegalStateException("[JENKINS-34987] SCRIPT_SPLITTING_TRANSFORMATION is an experimental feature of Declarative Pipeline and is incompatible with local variable declarations inside a Jenkinsfile. " + + "As a temporary workaround, you can add the '@Field' annotation to these local variable declarations. " + + "However, use of Groovy variables in Declarative pipeline, with or without the '@Field' annotation, is not recommended or supported. " + + "To use less effective script splitting which allows local variable declarations without changing your pipeline code, set SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES=true . " + + "Local variable declarations found: " + declarationNames.sort().join(", ") + ". ") } } + // In a future version, it may be possible to detect closures that reference script-local variables // and then only use closure scoped handles for those closures. return result diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java index 622a61bdd..27a9910f7 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java @@ -113,11 +113,6 @@ private static String symbolFromDescriptor(Descriptor d) { public void setUpFeatureFlags() { defaultScriptSplitting = RuntimeASTTransformer.SCRIPT_SPLITTING_TRANSFORMATION; defaultScriptSplittingAllowLocalVariables = RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES; - - // For testing we want to default to exercising splitting - // and not allowing local variables - RuntimeASTTransformer.SCRIPT_SPLITTING_TRANSFORMATION = true; - RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; } @After diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java index 8aafd8306..ef2a6a789 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java @@ -48,6 +48,7 @@ import org.jenkinsci.plugins.workflow.pipelinegraphanalysis.GenericStatus; import org.jenkinsci.plugins.workflow.pipelinegraphanalysis.StatusAndTiming; import org.jenkinsci.plugins.workflow.steps.ErrorStep; +import org.junit.Assume; import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; @@ -56,6 +57,7 @@ import java.util.Collection; import java.util.List; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.*; /** @@ -76,6 +78,7 @@ public static void setUpAgent() throws Exception { // Give this a longer timeout @Test(timeout=5 * 60 * 1000) public void stages300() throws Exception { + RuntimeASTTransformer.SCRIPT_SPLITTING_TRANSFORMATION = true; expect("basic/stages300") .logContains("letters1 = 'a', letters10 = 'a', letters100 = 'a'", "letters1 = 'j', letters10 = 'j', letters100 = 'c'") @@ -96,6 +99,20 @@ public void stages300NoSplit() throws Exception { @Issue("JENKINS-37984") @Test public void stages100WithOutsideVarAndFunc() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + expect("basic/stages100WithOutsideVarAndFunc") + .logContains("letters1 = 'a', letters10 = 'a', letters100 = 'a'", + "letters1 = 'j', letters10 = 'j', letters100 = 'a'", + "Hi there - This comes from a function") + .logNotContains("Method code too large!") + .go(); + } + + @Issue("JENKINS-37984") + @Test + public void stages100WithOutsideVarAndFuncNoSplitting() throws Exception { + RuntimeASTTransformer.SCRIPT_SPLITTING_TRANSFORMATION = false; expect("basic/stages100WithOutsideVarAndFunc") .logContains("letters1 = 'a', letters10 = 'a', letters100 = 'a'", "letters1 = 'j', letters10 = 'j', letters100 = 'a'", @@ -107,6 +124,9 @@ public void stages100WithOutsideVarAndFunc() throws Exception { @Issue("JENKINS-37984") @Test public void stages100WithOutsideVarAndFuncNotAllowed() throws Exception { + // this test only works if script splitting is enabled + Assume.assumeThat(RuntimeASTTransformer.SCRIPT_SPLITTING_TRANSFORMATION, is(true)); + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = false; expect(Result.FAILURE,"basic/stages100WithOutsideVarAndFunc") .logContains("add the '@Field' annotation to these local variable declarations") diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/EnvironmentTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/EnvironmentTest.java index 42b38deab..00cbb9597 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/EnvironmentTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/EnvironmentTest.java @@ -26,6 +26,7 @@ import hudson.model.Result; import hudson.model.Slave; import hudson.slaves.EnvironmentVariablesNodeProperty; +import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer; import org.junit.BeforeClass; import org.junit.Test; import org.jvnet.hudson.test.Issue; @@ -194,6 +195,9 @@ public void environmentWithWorkspace() throws Exception { @Issue("JENKINS-42753") @Test public void stmtExprInEnvironment() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + expect("environment/stmtExprInEnvironment") .logContains("FOO is BAR", "LIST_EXP is [a, BAR, c]", @@ -211,6 +215,9 @@ public void stmtExprInEnvironment() throws Exception { @Test public void nonLiteralEnvironment() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + initGlobalLibrary(); expect("environment/nonLiteralEnvironment") diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/LibrariesTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/LibrariesTest.java index 5017233a7..531a0780b 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/LibrariesTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/LibrariesTest.java @@ -29,6 +29,7 @@ import jenkins.plugins.git.GitSCMSource; import org.jenkinsci.plugins.pipeline.modeldefinition.actions.ExecutionModelAction; import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTStages; +import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.libs.FolderLibraries; import org.jenkinsci.plugins.workflow.libs.GlobalLibraries; @@ -130,6 +131,9 @@ public void librariesDirective() throws Exception { @Issue("JENKINS-38110") @Test public void librariesDirectiveWithOutsideVarAndFunc() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + otherRepo.init(); otherRepo.write("vars/myecho.groovy", "def call() {echo 'something special'}"); otherRepo.write("vars/myecho.txt", "Says something very special!"); diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/MatrixTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/MatrixTest.java index 2b232a1eb..eeefe8abd 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/MatrixTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/MatrixTest.java @@ -334,7 +334,6 @@ public void matrixStageDirectives() throws Exception { .go(); } - // Behavior should be identical to previous, just expressed differently @Issue("JENKINS-41334") @Test @@ -371,6 +370,88 @@ public void matrixStageDirectivesChildStage() throws Exception { .go(); } + @Issue("JENKINS-64846") + @Test + public void matrixStageDirectivesOutsideVarAndFunc() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + + expect("matrix/matrixStageDirectivesOutsideVarAndFunc") + .logContains("[Pipeline] { (foo)", + "{ (Branch: Matrix - OS_VALUE = 'linux')", + "{ (Branch: Matrix - OS_VALUE = 'windows')", + "{ (Branch: Matrix - OS_VALUE = 'mac')", + "First stage, mac agent", + "First stage, do not override", + "First stage, overrode once and done", + "First stage, overrode twice, in first mac-os branch", + "First stage, overrode per nested, in first mac-os branch", + "First stage, declared per nested, in first mac-os branch", + "First stage, Hi there - This comes from a function", + "First stage, windows agent", + "First stage, do not override", + "First stage, overrode once and done", + "First stage, overrode twice, in first windows-os branch", + "First stage, overrode per nested, in first windows-os branch", + "First stage, declared per nested, in first windows-os branch", + "First stage, Hi there - This comes from a function", + "First stage, linux agent", + "First stage, do not override", + "First stage, overrode once and done", + "First stage, overrode twice, in first linux-os branch", + "First stage, overrode per nested, in first linux-os branch", + "First stage, declared per nested, in first linux-os branch", + "First stage, Hi there - This comes from a function", + "Apache Maven 3.0.1", + "Apache Maven 3.0.1", + "Apache Maven 3.0.1") + .logNotContains("WE SHOULD NEVER GET HERE", + "java.lang.IllegalArgumentException", + "override in matrix axis") + .go(); + } + + @Issue("JENKINS-64846") + @Test + public void matrixStageDirectivesOutsideVarAndFuncNoSplitting() throws Exception { + // ensure vars in matrix still works with splitting transform turned off + RuntimeASTTransformer.SCRIPT_SPLITTING_TRANSFORMATION = false; + + expect("matrix/matrixStageDirectivesOutsideVarAndFunc") + .logContains("[Pipeline] { (foo)", + "{ (Branch: Matrix - OS_VALUE = 'linux')", + "{ (Branch: Matrix - OS_VALUE = 'windows')", + "{ (Branch: Matrix - OS_VALUE = 'mac')", + "First stage, mac agent", + "First stage, do not override", + "First stage, overrode once and done", + "First stage, overrode twice, in first mac-os branch", + "First stage, overrode per nested, in first mac-os branch", + "First stage, declared per nested, in first mac-os branch", + "First stage, Hi there - This comes from a function", + "First stage, windows agent", + "First stage, do not override", + "First stage, overrode once and done", + "First stage, overrode twice, in first windows-os branch", + "First stage, overrode per nested, in first windows-os branch", + "First stage, declared per nested, in first windows-os branch", + "First stage, Hi there - This comes from a function", + "First stage, linux agent", + "First stage, do not override", + "First stage, overrode once and done", + "First stage, overrode twice, in first linux-os branch", + "First stage, overrode per nested, in first linux-os branch", + "First stage, declared per nested, in first linux-os branch", + "First stage, Hi there - This comes from a function", + "Apache Maven 3.0.1", + "Apache Maven 3.0.1", + "Apache Maven 3.0.1") + .logNotContains("WE SHOULD NEVER GET HERE", + "java.lang.IllegalArgumentException", + "override in matrix axis") + .go(); + } + @Issue("JENKINS-41334") @Test public void matrixStageDirectivesWhenBeforeAgent() throws Exception { diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/OptionsTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/OptionsTest.java index 7d78e9423..2e89f75cb 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/OptionsTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/OptionsTest.java @@ -40,6 +40,7 @@ import jenkins.model.BuildDiscarder; import jenkins.model.BuildDiscarderProperty; import org.jenkinsci.plugins.pipeline.modeldefinition.actions.DeclarativeJobPropertyTrackerAction; +import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.cps.nodes.StepStartNode; import org.jenkinsci.plugins.workflow.flow.FlowExecution; @@ -179,6 +180,9 @@ public void checkoutToSubdirectory() throws Exception { @Issue("JENKINS-44277") @Test public void checkoutToSubdirectoryWithOutsideVarAndFunc() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + expect("options/checkoutToSubdirectoryWithOutsideVarAndFunc") .logContains("[Pipeline] { (foo)", "hello") @@ -484,6 +488,9 @@ public void rateLimitBuilds() throws Exception { @Issue("JENKINS-46354") @Test public void topLevelRetryExecutesAllStages() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + expect("options/topLevelRetryExecutesAllStages") .logContains("Actually executing stage Bar") .go(); @@ -492,6 +499,9 @@ public void topLevelRetryExecutesAllStages() throws Exception { @Issue("JENKINS-46354") @Test public void parentStageRetryExecutesAllChildStages() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + expect("options/parentStageRetryExecutesAllChildStages") .logContains("Actually executing stage Bar", "Actually executing stage Baz") .go(); diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ParametersTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ParametersTest.java index 1940843ff..a9deec6b7 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ParametersTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ParametersTest.java @@ -29,6 +29,7 @@ import hudson.model.ParametersDefinitionProperty; import hudson.model.PasswordParameterDefinition; import hudson.model.StringParameterDefinition; +import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; @@ -70,6 +71,9 @@ public void simpleParameters() throws Exception { @Test public void simpleParametersWithOutsideVarAndFunc() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + WorkflowRun b = expect("simpleParametersWithOutsideVarAndFunc") .logContains("[Pipeline] { (foo)", "hello true: Hi there - This comes from a function") .logNotContains("[Pipeline] { (" + SyntheticStageNames.postBuild() + ")") diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/PostStageTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/PostStageTest.java index c0baa9cc8..5eaefb119 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/PostStageTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/PostStageTest.java @@ -25,6 +25,7 @@ import hudson.model.Result; import hudson.model.Slave; +import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; @@ -187,6 +188,9 @@ public void parallelParentPostFailure() throws Exception { @Test public void postWithOutsideVarAndFunc() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + expect("postWithOutsideVarAndFunc") .logContains("Hi there - This comes from a function") .logNotContains("I FAILED") diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/StageInputTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/StageInputTest.java index cc0f67d1d..3c12ce400 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/StageInputTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/StageInputTest.java @@ -26,6 +26,7 @@ import com.gargoylesoftware.htmlunit.html.*; import hudson.model.queue.QueueTaskFuture; +import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution; import org.jenkinsci.plugins.workflow.job.WorkflowJob; @@ -77,6 +78,9 @@ public void simpleInput() throws Exception { @Test public void simpleInputWithOutsideVarAndFunc() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "simpleInputWithOutsideVarAndFunc"); p.setDefinition(new CpsFlowDefinition(pipelineSourceFromResources("simpleInputWithOutsideVarAndFunc"), true)); // get the build going, and wait until workflow pauses diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ToolsTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ToolsTest.java index e43e7792e..3feb93b9d 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ToolsTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ToolsTest.java @@ -26,6 +26,7 @@ import hudson.model.JDK; import hudson.model.Slave; import hudson.tasks.Maven; +import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer; import org.junit.BeforeClass; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -121,6 +122,9 @@ public void buildPluginParentPOM() throws Exception { @Issue("JENKINS-46809") @Test public void toolsWithOutsideVarAndFunc() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + Maven.MavenInstallation maven301 = ToolInstallations.configureMaven3(); j.jenkins.getDescriptorByType(Maven.DescriptorImpl.class).setInstallations(maven301); diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/TriggersTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/TriggersTest.java index 0a6e4b557..41f7b8c3b 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/TriggersTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/TriggersTest.java @@ -32,6 +32,7 @@ import hudson.triggers.TriggerDescriptor; import jenkins.model.Jenkins; import org.jenkinsci.Symbol; +import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; @@ -78,6 +79,9 @@ public void simpleTriggers() throws Exception { @Test public void simpleTriggersWithOutsideVarAndFunc() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + WorkflowRun b = expect("simpleTriggersWithOutsideVarAndFunc") .logContains("[Pipeline] { (foo)", "hello") .logNotContains("[Pipeline] { (Post Actions)") diff --git a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/WhenStageTest.java b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/WhenStageTest.java index a0b051653..93e37354b 100644 --- a/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/WhenStageTest.java +++ b/pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/WhenStageTest.java @@ -44,6 +44,7 @@ import jenkins.scm.impl.mock.MockSCMSource; import net.sf.json.JSONObject; import org.jenkinsci.plugins.pipeline.modeldefinition.endpoints.ModelConverterAction; +import org.jenkinsci.plugins.pipeline.modeldefinition.parser.RuntimeASTTransformer; import org.jenkinsci.plugins.pipeline.modeldefinition.when.ChangeLogStrategy; import org.jenkinsci.plugins.pipeline.modeldefinition.when.DeclarativeStageConditional; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; @@ -640,6 +641,9 @@ public void BuildStatusWhenWithUserIdCause() throws Exception { @Test public void whenExprUsingOutsideVarAndFunc() throws Exception { + // this should have same behavior whether script splitting is enable or not + RuntimeASTTransformer.SCRIPT_SPLITTING_ALLOW_LOCAL_VARIABLES = true; + expect("when/whenExprUsingOutsideVarAndFunc") .logContains("[Pipeline] { (One)", "[Pipeline] { (Two)", "World") .go(); diff --git a/pipeline-model-definition/src/test/resources/matrix/matrixStageDirectivesOutsideVarAndFunc.groovy b/pipeline-model-definition/src/test/resources/matrix/matrixStageDirectivesOutsideVarAndFunc.groovy new file mode 100644 index 000000000..4b5af5e67 --- /dev/null +++ b/pipeline-model-definition/src/test/resources/matrix/matrixStageDirectivesOutsideVarAndFunc.groovy @@ -0,0 +1,108 @@ +/* + * 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. + */ + +def someVar = "Hi there" +def someFunc() { + return "This comes from a function" +} + +pipeline { + agent none + environment { + OS_VALUE = "override in matrix axis" + OVERRIDE_TWICE = "override twice" + DO_NOT_OVERRIDE = "do not override" + OVERRIDE_ONCE = "override once" + } + stages { + stage("foo") { + environment { + OVERRIDE_TWICE = "overrode once, one to go" + OVERRIDE_ONCE = "overrode once and done" + OVERRIDE_PER_NESTED = "override in each branch" + } + matrix { + axes { + axis { + name 'OS_VALUE' + values "linux", "windows", "mac" + } + } + agent { + label "${OS_VALUE}-agent" + } + tools { + maven "apache-maven-${MAVEN_VERSION}" + } + when { + environment name: "WHICH_AGENT", value: "${OS_VALUE} agent" + } + environment { + OS_VALUE = "${OS_VALUE}-os" + OVERRIDE_TWICE = "overrode twice, in first ${OS_VALUE} branch" + OVERRIDE_PER_NESTED = "overrode per nested, in first ${OS_VALUE} branch" + DECLARED_PER_NESTED = "declared per nested, in first ${OS_VALUE} branch" + MAVEN_VERSION = "3.0.1" + } + stages { + stage("first") { + steps { + echo "First stage, ${WHICH_AGENT}" + echo "First stage, ${DO_NOT_OVERRIDE}" + echo "First stage, ${OVERRIDE_ONCE}" + echo "First stage, ${OVERRIDE_TWICE}" + echo "First stage, ${OVERRIDE_PER_NESTED}" + echo "First stage, ${DECLARED_PER_NESTED}" + echo "First stage, ${someVar} - ${someFunc()}" + dir("subdir") { + script { + if (isUnix()) { + sh 'mvn --version' + } else { + bat 'mvn --version' + } + if (!fileExists("Jenkinsfile")) { + echo "Jenkinsfile does not exist" + } + } + } + } + } + stage("second") { + when { + environment name: "OS_VALUE", value: "not-an-os" + } + steps { + echo "WE SHOULD NEVER GET HERE" + } + } + } + } + } + } +} + + + + diff --git a/pom.xml b/pom.xml index a7d330fb8..0cf24d917 100644 --- a/pom.xml +++ b/pom.xml @@ -192,6 +192,64 @@ 2.16 + + + no-script-splitting + + [,1.9) + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + false + + + + + + + + script-splitting + + [1.9,) + + + + + org.apache.maven.plugins + maven-surefire-plugin + + + true + false + + + + + + + + weak-script-splitting + + + + org.apache.maven.plugins + maven-surefire-plugin + + + true + true + + + + + + + repo.jenkins-ci.org