Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-46175

Ability to mark a SimpleBuildStep as not requiring a workspace context

    • jenkins 2.258, workflow-basic-steps 2.22

      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.

          [JENKINS-46175] Ability to mark a SimpleBuildStep as not requiring a workspace context

          Jesse Glick created issue -

          Jesse Glick added a comment -

          If SimpleBuildWrapper is updated, then this code should also be expanded to permit such wrappers to be used in Declarative options blocks.

          Jesse Glick added a comment - If SimpleBuildWrapper is updated, then this code should also be expanded to permit such wrappers to be used in Declarative options blocks.
          Jesse Glick made changes -
          Link New: This issue relates to JENKINS-29144 [ JENKINS-29144 ]
          Tim Van Holder made changes -
          Assignee New: Tim Van Holder [ zastai ]

          Will try to give this a look after JENKINS-29144 is done.

          Tim Van Holder added a comment - Will try to give this a look after JENKINS-29144 is done.

          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?

          Tim Van Holder added a 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?

          Jesse Glick added a 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.

          Jesse Glick added a 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.

          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.

          Tim Van Holder added a 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.

          Jesse Glick added a 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.

          Jesse Glick added a 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.

          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.

          Tim Van Holder added a 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.

            zastai Tim Van Holder
            jglick Jesse Glick
            Votes:
            2 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: