• Icon: Improvement Improvement
    • Resolution: Unresolved
    • Icon: Minor Minor
    • core

      There is a frequent idiom when making plugins Pipeline-compatible that goes something like

      EnvVars env;
      if (run instanceof AbstractBuild) {
          env = run.getEnvironment(listener);
          env.overrideAll(((AbstractBuild<?,?>) run).getBuildVariables());
      } else {
          env = new EnvVars();
      }
      String whatever = env.expand(this.whatever);
      

      For a traditional build, we wish to allow configuration parameters (for example of a Builder) to be expanded using either the build's global environment, or "build variables" which include parameters, build wrappers, and other stuff. For a Pipeline build, we do not wish to do any expansion whatsoever—the Groovy script should have done any desired expansion before the SimpleBuildStep (for example) even receives its arguments.

      To simplify this idiom, we should:

      • define a method getEnvironmentForExpansion(TaskListener) in Run which by default just delegates to getEnvironment(TaskListener)
      • override it in AbstractBuild to include getBuildVariables()
      • define a method of the same name in WorkflowRun which just returns an empty new EnvVars() (later it can be an @Override)
      • optimize Util.replaceMacro(String, Map) to return its argument immediately if the map is empty
      • search existing plugins for the above idiom and add a TODO comment to simplify them once the baseline is sufficiently new

      The revised code should be something as simple as:

      String whatever = run.getEnvironmentForExpansion(listener).expand(this.whatever);
      

          [JENKINS-35671] Define Run.getEnvironmentForExpansion

          Code changed in jenkins
          User: restjohn
          Path:
          src/main/java/org/jenkinsci/plugins/androidsigning/SignApksBuilder.java
          src/main/java/org/jenkinsci/plugins/androidsigning/ZipalignTool.java
          src/test/java/org/jenkinsci/plugins/androidsigning/SignApksBuilderTest.java
          src/test/java/org/jenkinsci/plugins/androidsigning/ZipalignToolTest.java
          http://jenkins-ci.org/commit/android-signing-plugin/2a67d3b2fccd83d73e6a6180caf65a9e11c61ebb
          Log:
          added builder properties to directly override envirnoment variables for ANDROID_HOME and ANDROID_ZIPALIGN so that can be set from a pipeline or dsl script irrespective of JENKINS-35671

          Compare: https://github.com/jenkinsci/android-signing-plugin/compare/484e5fff47ef^...2a67d3b2fccd

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: restjohn Path: src/main/java/org/jenkinsci/plugins/androidsigning/SignApksBuilder.java src/main/java/org/jenkinsci/plugins/androidsigning/ZipalignTool.java src/test/java/org/jenkinsci/plugins/androidsigning/SignApksBuilderTest.java src/test/java/org/jenkinsci/plugins/androidsigning/ZipalignToolTest.java http://jenkins-ci.org/commit/android-signing-plugin/2a67d3b2fccd83d73e6a6180caf65a9e11c61ebb Log: added builder properties to directly override envirnoment variables for ANDROID_HOME and ANDROID_ZIPALIGN so that can be set from a pipeline or dsl script irrespective of JENKINS-35671 Compare: https://github.com/jenkinsci/android-signing-plugin/compare/484e5fff47ef ^...2a67d3b2fccd

          The issue reference in that previous commit message should have been JENKINS-29144 - sorry about that.

          Robert St. John added a comment - The issue reference in that previous commit message should have been JENKINS-29144 - sorry about that.

            Unassigned Unassigned
            jglick Jesse Glick
            Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated: