You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[JENKINS-46175] Ability to mark a SimpleBuildStep as not requiring a workspace context #22243
Currently SimpleBuildStep expects to be passed a valid FilePath workspace, limiting its use in Pipelines to step appearing inside a node block. This is because BuildStep historically ran in an AbstractBuild which always has a workspace. But there are many build steps, especially Publisher but sometimes also Builder, which do not actually use the workspace (or node) at all, and these are needlessly blocked from being used outside a node block.
Would be useful to have a SimpleBuildStepDescriptor defining a method which would allow an implementation to declare that the workspace and launcher parameters to perform may be null. CoreStep would then be able to run in any context, passing those parameters only if the StepContext defined them.
SimpleBuildWrapper could potentially benefit from a similar API. This would have been useful for TimestamperBuildWrapper, avoiding the need for a separate TimestamperStep.
Currently SimpleBuildStep expects to be passed a valid FilePath workspace, limiting its use in Pipelines to step appearing inside a node block. This is because BuildStep historically ran in an AbstractBuild which always has a workspace. But there are many build steps, especially Publisher but sometimes also Builder, which do not actually use the workspace (or node) at all, and these are needlessly blocked from being used outside a node block.
Would be useful to have a SimpleBuildStepDescriptor defining a method which would allow an implementation to declare that the workspace and launcher parameters to perform may be null. CoreStep would then be able to run in any context, passing those parameters only if the StepContext defined them.
SimpleBuildWrapper could potentially benefit from a similar API. This would have been useful for TimestamperBuildWrapper, avoiding the need for a separate TimestamperStep.
So if CoreStep no longer lists FilePath and Launcher as required, then it would happily wrap a SimpleBuildStep that says it needs one or both of those, wouldn't it? With an error only produced at execution time?
So if CoreStep no longer lists FilePath and Launcher as required, then it would happily wrap a SimpleBuildStep that says it needs one or both of those, wouldn't it? With an error only produced at execution time?
Yes the error would only be produced at runtime (according to whatever flag is mechanism is chosen—I suggested a method in a new SimpleBuildStepDescriptor but maybe you could use a new default method), but that is already the case. It just means that rather than this being enforced by getRequiredContext and thus MissingContextVariableException being thrown automatically by checkContextAvailability from DSL right before the step is started, the exception will be thrown from CoreStep.Execution if the step says it needs a workspace yet none was in fact provided. https://github.com/jenkinsci/credentials-binding-plugin/blob/9bed48e5b36820cc1d2f8053965eb37bb18a6f17/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java#L131-L134 is a good example.
Yes the error would only be produced at runtime (according to whatever flag is mechanism is chosen—I suggested a method in a new SimpleBuildStepDescriptor but maybe you could use a new default method), but that is already the case. It just means that rather than this being enforced by getRequiredContext and thus MissingContextVariableException being thrown automatically by checkContextAvailability from DSL right before the step is started, the exception will be thrown from CoreStep.Execution if the step says it needs a workspace yet none was in fact provided. https://github.com/jenkinsci/credentials-binding-plugin/blob/9bed48e5b36820cc1d2f8053965eb37bb18a6f17/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java#L131-L134 is a good example.
OK, then that should not be too hard. I'd be inclined towards extra default interface methods (needsFilePath() and needsLauncher() come to mind). A SimpleBuildStepDescriptor would need to be an interface too, and would be trickier to integrate.
OK, then that should not be too hard. I'd be inclined towards extra default interface methods (needsFilePath() and needsLauncher() come to mind). A SimpleBuildStepDescriptor would need to be an interface too, and would be trickier to integrate.
If that's guaranteed (now and in the future). then I suppose a single method like requiresNode() or isNodeSpecific() might make sense. Would have to see if there's similar methods elsewhere that I can reuse the name of.
If that's guaranteed (now and in the future). then I suppose a single method like requiresNode() or isNodeSpecific() might make sense. Would have to see if there's similar methods elsewhere that I can reuse the name of.
Draft PR for jenkins core added.
I went with 2 separate predicates (requiresWorkspace() and requiresLauncher()) for maximum flexibility.
Changes made to both SimpleBuildStep and SimpleBuildWrapper (plus the latter's nested Disposer).
Will see what that means for CoreStep and the like once I have an incremental to play with.
Draft PR for jenkins core added.
I went with 2 separate predicates (requiresWorkspace() and requiresLauncher()) for maximum flexibility.
Changes made to both SimpleBuildStep and SimpleBuildWrapper (plus the latter's nested Disposer).
Will see what that means for CoreStep and the like once I have an incremental to play with.
Note that for purposes of local development you can simply mvn -DskipTests clean install from core and then set jenkins.version to 2.XXX-SNAPSHOT in your plugin POM. IDEs with proper Maven support will also interpret this to mean that core + plugin are effectively part of a single build tree, so you can refactor across the two, make a core change and run a fast JUnit test from the plugin to verify it, etc.
once I have an incremental to play with
Note that for purposes of local development you can simply mvn -DskipTests clean install from core and then set jenkins.version to 2.XXX-SNAPSHOT in your plugin POM. IDEs with proper Maven support will also interpret this to mean that core + plugin are effectively part of a single build tree, so you can refactor across the two, make a core change and run a fast JUnit test from the plugin to verify it, etc.
Activity
jenkins-infra-bot commentedon Aug 14, 2017
Remote links associated with this issue:
jenkins-infra-bot commentedon Jun 22, 2018
Raw content of original comment:
If SimpleBuildWrapper is updated, then this code should also be expanded to permit such wrappers to be used in Declarative options blocks.
jenkins-infra-bot commentedon Jun 2, 2020
Raw content of original comment:
Will try to give this a look after
JENKINS-29144is done.jenkins-infra-bot commentedon Jun 6, 2020
Raw content of original comment:
So if CoreStep no longer lists FilePath and Launcher as required, then it would happily wrap a SimpleBuildStep that says it needs one or both of those, wouldn't it? With an error only produced at execution time?
jenkins-infra-bot commentedon Jun 8, 2020
Raw content of original comment:
Yes the error would only be produced at runtime (according to whatever flag is mechanism is chosen—I suggested a method in a new SimpleBuildStepDescriptor but maybe you could use a new default method), but that is already the case. It just means that rather than this being enforced by getRequiredContext and thus MissingContextVariableException being thrown automatically by checkContextAvailability from DSL right before the step is started, the exception will be thrown from CoreStep.Execution if the step says it needs a workspace yet none was in fact provided. https://github.com/jenkinsci/credentials-binding-plugin/blob/9bed48e5b36820cc1d2f8053965eb37bb18a6f17/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java#L131-L134 is a good example.
jenkins-infra-bot commentedon Jun 8, 2020
Raw content of original comment:
OK, then that should not be too hard. I'd be inclined towards extra default interface methods (needsFilePath() and needsLauncher() come to mind). A SimpleBuildStepDescriptor would need to be an interface too, and would be trickier to integrate.
jenkins-infra-bot commentedon Jun 8, 2020
Raw content of original comment:
You should not need separate markers for FilePath and Launcher—if you are inside node you will get both, otherwise you will get neither.
jenkins-infra-bot commentedon Jun 8, 2020
Raw content of original comment:
If that's guaranteed (now and in the future). then I suppose a single method like requiresNode() or isNodeSpecific() might make sense. Would have to see if there's similar methods elsewhere that I can reuse the name of.
jenkins-infra-bot commentedon Jun 9, 2020
Raw content of original comment:
or requiresWorkspace
jenkins-infra-bot commentedon Jun 13, 2020
Raw content of original comment:
Draft PR for jenkins core added.
I went with 2 separate predicates (requiresWorkspace() and requiresLauncher()) for maximum flexibility.
Changes made to both SimpleBuildStep and SimpleBuildWrapper (plus the latter's nested Disposer).
Will see what that means for CoreStep and the like once I have an incremental to play with.
jenkins-infra-bot commentedon Jun 15, 2020
Raw content of original comment:
Note that for purposes of local development you can simply mvn -DskipTests clean install from core and then set jenkins.version to 2.XXX-SNAPSHOT in your plugin POM. IDEs with proper Maven support will also interpret this to mean that core + plugin are effectively part of a single build tree, so you can refactor across the two, make a core change and run a fast JUnit test from the plugin to verify it, etc.
jenkins-infra-bot commentedon Oct 1, 2020
Raw content of original comment:
Is this done?
jenkins-infra-bot commentedon Oct 1, 2020
Raw content of original comment:
The API on the core side is now there, but the ticket won't be resolved until workflow-basic-steps uses it.
I'll link to that PR too.
jenkins-infra-bot commentedon Oct 1, 2020
Raw content of original comment:
Both PRs now merged.
Will set as Resolved once workflow-basic-steps 2.22 is released.
jenkins-infra-bot commentedon Oct 17, 2020
Raw content of original comment:
Workflow basic steps plugin 2.22 released Oct 1, 2020