In https://github.com/jenkinsci/workflow-plugin/commit/e24a64a7a6404be97f42340f4c44d3988be1052c you added
createBodyBlockNode = (name==null);
Is this not backwards? The Javadoc says that if name == null, the body is "anonymous", and
anonymous body invocation suppress the generation of BlockStartNode/BlockEndNode
which makes sense. Yet here you are saying that an anonymous body should create a body block node, which looks like a mistake. There appears to be no call to this method passing an actual display name anywhere in the codebase, and no test of how it should behave. I suspect that a call to non-null displayName would actually produce an IllegalStateException in start, since then startNodeActions would have a LabelAction. Note that ParallelStepExecution does call withStartAction, the only one to do so, and effectively sets a display name for the block, yet not using regular LabelAction; it seems the only purpose for the custom subclass is to assert branch names in a test.
Furthermore, displayName is null by default, yet the Javadoc explicitly suggests calling withDisplayName(null), which will have no effect since createBodyBlockNode is also true by default. And many—but not all—steps indeed call withDisplayName(null).
This all seems confused. Either blocks should be anonymous by default, in which case createBodyBlockNode should default to false, and all the calls to withDisplayName(null) should be deleted, and the name parameter should be marked @Nonnull, and the Javadoc should confirm the default behavior, and ParallelStepExecution should be handled somehow; or blocks should be visible by default, in which case displayName should have some default value (perhaps computed according to StepDescriptor.displayName?) and you would need to call withDisplayName(null) to turn it off. The first option seems more appropriate to me.
- is blocking
-
JENKINS-26107 Allow stage to operate as a labelled block
-
- Closed
-
-
JENKINS-30088 Clean up step display
-
- Resolved
-
- links to
[JENKINS-26156] Erroneous handling of BodyInvoker.withDisplayName
Code changed in jenkins
User: Jesse Glick
Path:
src/main/java/org/jenkinsci/plugins/workflow/cps/CpsBodyExecution.java
src/main/java/org/jenkinsci/plugins/workflow/cps/CpsBodyInvoker.java
src/test/java/org/jenkinsci/plugins/workflow/DynamicEnvironmentExpanderTest.java
http://jenkins-ci.org/commit/workflow-cps-plugin/ff4ab3e544a346400aaead547565f29d99c74990
Log:
[FIXED JENKINS-26156] Fixed BodyInvoker.withDisplayName implementation to allow a non-null argument.
Code changed in jenkins
User: Jesse Glick
Path:
src/main/java/org/jenkinsci/plugins/workflow/cps/CpsBodyExecution.java
src/main/java/org/jenkinsci/plugins/workflow/cps/CpsBodyInvoker.java
src/main/java/org/jenkinsci/plugins/workflow/cps/nodes/StepEndNode.java
src/main/java/org/jenkinsci/plugins/workflow/cps/nodes/StepStartNode.java
src/test/java/org/jenkinsci/plugins/workflow/DynamicEnvironmentExpanderTest.java
src/test/java/org/jenkinsci/plugins/workflow/WorkflowJobNonRestartingTest.java
http://jenkins-ci.org/commit/workflow-cps-plugin/5e8cbc2be2b340c2532d4720c7a24ca44d53c4c3
Log:
Merge pull request #3 from jglick/JENKINS-26156-BodyInvoker.withDisplayName
JENKINS-26156 BodyInvoker.displayName was broken
Compare: https://github.com/jenkinsci/workflow-cps-plugin/compare/654e8b4fdbde...5e8cbc2be2b3
Code changed in jenkins
User: Jesse Glick
Path:
src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java
src/main/java/org/jenkinsci/plugins/workflow/steps/PushdStep.java
src/main/java/org/jenkinsci/plugins/workflow/steps/TimeoutStepExecution.java
http://jenkins-ci.org/commit/workflow-basic-steps-plugin/a14ab59ff8445f6341b2fbb7e0e8ccbef04eedec
Log:
JENKINS-26156 Calling BodyInvoker.withDisplayName(null) was always a no-op, and will soon be a FindBugs warning.
(cherry picked from commit 7806a767f7f4253200b588f81858c07139ce6fdb)
Code changed in jenkins
User: Jesse Glick
Path:
src/main/java/org/jenkinsci/plugins/workflow/steps/CatchErrorStep.java
src/main/java/org/jenkinsci/plugins/workflow/steps/PushdStep.java
src/main/java/org/jenkinsci/plugins/workflow/steps/TimeoutStepExecution.java
http://jenkins-ci.org/commit/workflow-basic-steps-plugin/7360c8d48bdb0f4ac2fb4aa4f8ac1e7040faf7cb
Log:
Merge pull request #6 from jglick/cleanup-JENKINS-26156
JENKINS-26156 Remove calls to BodyInvoker.withDisplayName(null)
Compare: https://github.com/jenkinsci/workflow-basic-steps-plugin/compare/3d258572897d...7360c8d48bdb
May make it feasible to make waitUntil be less verbose.