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

Promote delegates of metasteps to top-level functions, deprecate $class

      Currently you must write e.g.

      step([$class: 'JUnitResultArchiver', testResults: 'target/surefire-reports/*.xml'])
      

      which is awkward. Also in Snippet Generator you need to look under General Build Step for what to a user is logically a distinct step.

      Metasteps (step, wrap, checkout) should have an API by which they can declare that their delegate (scm in the last case) ought to be treated as a top-level step as far as DSL and Snippetizer are concerned, via some kind of syntactic sugar. In the absence of any Jenkins core API which would allow a Descriptor to specify a short name (yaml-project tries to define one of its own), this would have to be constructed somehow from the $class, perhaps simply:

      JUnitResultArchiver testResults: 'target/surefire-reports/*.xml'
      

      or with just one mandatory parameter even

      JUnitResultArchiver 'target/surefire-reports/*.xml'
      

      The follow-up question is what to do with nested Describable objects used in the configuration. So

      GitSCM ..., extensions: [[$class: 'PruneStaleBranch']]
      

      still looks unnatural. The Groovy builder idiom might suggest

      GitSCM ..., extensions: [PruneStaleBranch {}]
      

      though closure handling in JENKINS-26135 would need to be addressed first. Requires study to make a PoC.

          [JENKINS-29922] Promote delegates of metasteps to top-level functions, deprecate $class

          Jesse Glick created issue -
          Jesse Glick made changes -
          Link New: This issue is related to JENKINS-30088 [ JENKINS-30088 ]
          Jesse Glick made changes -
          Link New: This issue depends on JENKINS-30222 [ JENKINS-30222 ]

          Jesse Glick added a comment -

          kohsuke proposes introducing a plugin (or other library?) with an annotation that could specify a script-friendly name (restricted to be a Java identifier, I suppose) for a Describable; the fallback value if not annotated could be the Class.simpleName. Using an annotation rather than a method on Descriptor ensures that mechanical indexers can run without executing code. Perhaps StepDescriptor.getFunctionName should also be deprecated in favor of the new annotation.

          Thus we could have something like

          junit testResults: '…'
          

          This could also be used in nested describables, using a builder pattern. Assuming annotations on GitSCM¹ & UserIdentity:

          gitSCM …, extensions: [identity {name = 'J. Q. Hacker', email = 'jqhacker@jenkins-ci.org'}]
          

          (JENKINS-30222 might need to be solved before adding support for nested describables, since we must ensure that these dynamically-generated functions are accessible not just in the sandbox but also in secondary CpsScript instances. Of course the existing technique in DSL may suffice without much work.)

          The builder pattern would allow us to replace $class on polymorphic (singleton or collection) properties. An open question is what, if anything, to do with monomorphic properties (usually collections). A quick recap:

          1. Prior to 1.0, all Describable values in properties needed to be instantiated directly as Java objects (constructor + setters). This is still allowed, but deprecated, since it forces you to import FQNs from plugins, and does not work in the sandbox.
          2. As of 1.0, a Describable should instead be specified as a Map<String,Object>, typically using the Groovy syntax for map literals.
            1. The implementation’s Class.name must in general be specified as a special key $class.
            2. $class may instead be a Class.simpleName when that is unambiguous.
            3. If a singleton property, or the element type of a collection-typed property, is monomorphic, $class may be omitted.
          3. The proposal here is to add a new and preferred option for specifying a nested Describable based on the builder pattern.

          So consider a monomorphic property such as GitSCM.userRemoteConfigs. We could either force it into the builder pattern as well:

          gitSCM userRemoteConfigs: [config {url = 'git@…', credentialsId = 'git-login'}, config {…}]
          

          or just leave it to use the map syntax as today (which we must continue to support for compatibility anyway):

          gitSCM userRemoteConfigs: [[url: 'git@…', credentialsId: 'git-login'}, […]]
          

          ¹Note that the identifier git is already taken by a custom step, so either we do not annotate GitSCM, or we use a different name, or the custom step wins and flows must use the Class.simpleName.

          ² Using the builder pattern also at top level, for steps, would be more consistent, but also more verbose, and perhaps incompatible:

          echo {message = 'hello world'}

          does not flow.

          Jesse Glick added a comment - kohsuke proposes introducing a plugin (or other library?) with an annotation that could specify a script-friendly name (restricted to be a Java identifier, I suppose) for a Describable ; the fallback value if not annotated could be the Class.simpleName . Using an annotation rather than a method on Descriptor ensures that mechanical indexers can run without executing code. Perhaps StepDescriptor.getFunctionName should also be deprecated in favor of the new annotation. Thus we could have something like junit testResults: '…' This could also be used in nested describables, using a builder pattern. Assuming annotations on GitSCM ¹ & UserIdentity : gitSCM …, extensions: [identity {name = 'J. Q. Hacker' , email = 'jqhacker@jenkins-ci.org' }] ( JENKINS-30222 might need to be solved before adding support for nested describables, since we must ensure that these dynamically-generated functions are accessible not just in the sandbox but also in secondary CpsScript instances. Of course the existing technique in DSL may suffice without much work.) The builder pattern would allow us to replace $class on polymorphic (singleton or collection) properties. An open question is what, if anything, to do with monomorphic properties (usually collections). A quick recap: Prior to 1.0, all Describable values in properties needed to be instantiated directly as Java objects (constructor + setters). This is still allowed, but deprecated, since it forces you to import FQNs from plugins, and does not work in the sandbox. As of 1.0, a Describable should instead be specified as a Map<String,Object> , typically using the Groovy syntax for map literals. The implementation’s Class.name must in general be specified as a special key $class . $class may instead be a Class.simpleName when that is unambiguous. If a singleton property, or the element type of a collection-typed property, is monomorphic, $class may be omitted. The proposal here is to add a new and preferred option for specifying a nested Describable based on the builder pattern. So consider a monomorphic property such as GitSCM.userRemoteConfigs . We could either force it into the builder pattern as well: gitSCM userRemoteConfigs: [config {url = 'git@…' , credentialsId = 'git-login' }, config {…}] or just leave it to use the map syntax as today (which we must continue to support for compatibility anyway): gitSCM userRemoteConfigs: [[url: 'git@…' , credentialsId: 'git-login' }, […]] ¹Note that the identifier git is already taken by a custom step, so either we do not annotate GitSCM , or we use a different name, or the custom step wins and flows must use the Class.simpleName . ² Using the builder pattern also at top level, for steps, would be more consistent, but also more verbose, and perhaps incompatible: echo {message = 'hello world' } does not flow.
          Jesse Glick made changes -
          Assignee Original: Jesse Glick [ jglick ] New: Kohsuke Kawaguchi [ kohsuke ]
          Summary Original: Promote delegates of metasteps to top-level functions New: Promote delegates of metasteps to top-level functions, deprecate $class
          Jesse Glick made changes -
          Link New: This issue is blocking JENKINS-30519 [ JENKINS-30519 ]
          Jesse Glick made changes -
          Link New: This issue is blocking JENKINS-31247 [ JENKINS-31247 ]
          Jesse Glick made changes -
          Link Original: This issue depends on JENKINS-30222 [ JENKINS-30222 ]

          Jesse Glick added a comment -

          Hoping to solve JENKINS-30222 a different way.

          Jesse Glick added a comment - Hoping to solve JENKINS-30222 a different way.
          Jesse Glick made changes -
          Assignee Original: Kohsuke Kawaguchi [ kohsuke ]

            kohsuke Kohsuke Kawaguchi
            jglick Jesse Glick
            Votes:
            7 Vote for this issue
            Watchers:
            19 Start watching this issue

              Created:
              Updated:
              Resolved: