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

Enhance BuildWrapper Extension to allow decorating build variables in the scm checkout phase

    • Icon: Improvement Improvement
    • Resolution: Fixed
    • Icon: Minor Minor
    • core
    • None

      I brought this up in the hudson-dev mailing list, but wanted to followup and enter a JIRA ticket for more formal tracking.

      I have a build job that takes a parameter - VERSION, which I expect to
      be a 4 digit number, like 1.2.3.4

      What I would like to do is derive a new parameter, BRANCH, which is
      the first 3 digits, like 1.2.3.

      I then hope to use new BRANCH parameter to be available from the view
      configuration for a perforce job.

      For example, my build job would define the Perforce view to be:

      //SOME/ROOT/PATH/${BRANCH}/...

      I thought I knew how to do this, writing a new BuildWrapper extension
      plugin similar to SetEnv. The only difference I added was to supply
      options fields for including support for regular expression pattern
      matching and replacement. It worked really well, and various
      buildsteps of the job were able to get access to my derived env
      variable.

      I discovered however that while perforce gets access to use the
      original parameters as a macro when defining the view, it was failing
      to resolve my derived variable.

      Turns out the BuildWrapper is not hooked into the build until after the scm checkout phase occurs.

      This enhancement request is to provide a hook for BuildWrapper to decorate build variables prior to the scm checkout phase.

      I've played around with checking out the core and altering things to get it to work, but not sure how to formalize getting feedback and actually committing assuming it is approved.

      Here's how I go things to work:

      n the abstract BuildWrapper class, I created a new method –

      public Map<String,String> decorateBuildVariables(AbstractBuild build,
      Map<String,String> buildVariables)

      { return buildVariables; }

      Then, I modified the the AbstractBuild.getBuildVariables() method, as seen here:

      public Map<String,String> getBuildVariables() {
      Map<String,String> r = new HashMap<String, String>();

      ParametersAction parameters = getAction(ParametersAction.class);
      if (parameters!=null) {
      // this is a rather round about way of doing this...
      for (ParameterValue p : parameters)

      { String v = p.createVariableResolver(this).resolve(p.getName()); if (v!=null) r.put(p.getName(),v); }

      }

      // NEW CODE here to allow buildwrapers to alter build variables

      if (project instanceof BuildableItemWithBuildWrappers)

      { BuildableItemWithBuildWrappers biwbw = (BuildableItemWithBuildWrappers) project; for (BuildWrapper bw : biwbw.getBuildWrappersList()) r = bw.decorateBuildVariables(this, r); }

      // END NEW CODE

      return r;
      }

      After making these changes, I implemented my BuildWrapper extension to override the decorateBuildVariables() method and I was able to confirm that the perforce plugin was able to get access to my new build variables my plugin injected into the build.

      As the default behavior of the BuildWapper just returns the supplied map, this results in backwards compatibility for those extensions already deriving from BuildWrapper.

      This was the easiest way to introduce this functionality into hudson core. I don't have the experience within the hudson core codebase to know the implication of these changes.

      One potential problem I see is that AbstractBuild.getBuildVariables() will call BuildWrapper.decorateBuildVariables(), passing an instance of itself in the parameters. This produces the danger that the called BuildWrapper might in its implementation turn around and re-call getBuildParameters(), thus setting up an infinite loop.

      For my specific BuildWrapper extension I'm writing which needs to take advantage of these core changes, I don't need access to the Build instance and don't use it. I had only thrown it there thinking that its reasonable that someone might want to use some additional information from the build to derive new dynamic BuildVariables.

          [JENKINS-6497] Enhance BuildWrapper Extension to allow decorating build variables in the scm checkout phase

          vbuzzsaw added a comment -

          I created a patch off of svn revision 32105 to address this issue. Can someone code review it and apply comments? Thanks.

          vbuzzsaw added a comment - I created a patch off of svn revision 32105 to address this issue. Can someone code review it and apply comments? Thanks.

          Alan Harder added a comment -

          r32257 | kohsuke | 2010-06-23 17:47:03 -0700 (Wed, 23 Jun 2010)
          Changed paths:
          M /trunk/hudson/main/core/src/main/java/hudson/model/AbstractBuild.java
          M /trunk/hudson/main/core/src/main/java/hudson/tasks/BuildWrapper.java
          M /trunk/www/changelog.html

          [FIXED JENKINS-6497] slightly revised the patch, to avoid using a boolean flag for a concurrency cont
          rol. It affects the serialization format, and it's not concurrency safe.

          Given the use case described in the bug, I wonder if it's better to have a different extension point
          (sort of like ComputerListener) that can contribute build variables to any builds going on in the s
          ystem.

          Alan Harder added a comment - r32257 | kohsuke | 2010-06-23 17:47:03 -0700 (Wed, 23 Jun 2010) Changed paths: M /trunk/hudson/main/core/src/main/java/hudson/model/AbstractBuild.java M /trunk/hudson/main/core/src/main/java/hudson/tasks/BuildWrapper.java M /trunk/www/changelog.html [FIXED JENKINS-6497] slightly revised the patch, to avoid using a boolean flag for a concurrency cont rol. It affects the serialization format, and it's not concurrency safe. Given the use case described in the bug, I wonder if it's better to have a different extension point (sort of like ComputerListener) that can contribute build variables to any builds going on in the s ystem.

          rsteele added a comment - - edited

          > Given the use case described in the bug, I wonder if it's better to have a different extension point
          > (sort of like ComputerListener) that can contribute build variables to any builds going on in the
          > system.

          I think a new extension point would be great: we're trying to create a plugin to map (Windows) drive letters before the build, and we really need this to happen before the (ClearCase) SCM updates the view.

          (The reason: our standard company policy is to use the ClearCase config spec "include" facility to load a config spec, in this case from a drive mounted by letter. However, since we launch our slaves as Windows services, the standard drive letters aren't mapped by default. We need to add a step that happens before SCM to do the mapping. We thought BuildWrapper would do it, but we now see that it won't.)

          rsteele added a comment - - edited > Given the use case described in the bug, I wonder if it's better to have a different extension point > (sort of like ComputerListener) that can contribute build variables to any builds going on in the > system. I think a new extension point would be great: we're trying to create a plugin to map (Windows) drive letters before the build, and we really need this to happen before the (ClearCase) SCM updates the view. (The reason: our standard company policy is to use the ClearCase config spec "include" facility to load a config spec, in this case from a drive mounted by letter. However, since we launch our slaves as Windows services, the standard drive letters aren't mapped by default. We need to add a step that happens before SCM to do the mapping. We thought BuildWrapper would do it, but we now see that it won't.)

            kohsuke Kohsuke Kawaguchi
            vbuzzsaw vbuzzsaw
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: