[FIXED JENKINS-20560] - Custom-tools should not override global paths…

… inside nested launchers I've also refactored retrieval of paths list. Related to: https://issues.jenkins-ci.org/browse/JENKINS-20560 Signed-off-by: Oleg Nenashev <nenashev@synopsys.com>
jenkinsci · Nov 20, 2013 · 1ed68b4 · 1ed68b4
1 parent 5d9a150
commit 1ed68b4
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 12 deletions.
diff --git a/src/main/java/com/cloudbees/jenkins/plugins/customtools/CustomToolInstallWrapper.java b/src/main/java/com/cloudbees/jenkins/plugins/customtools/CustomToolInstallWrapper.java
@@ -215,18 +215,21 @@ public Proc launch(ProcStarter starter) throws IOException {
                 }
 
                 // HACK: Avoids issue with invalid separators in EnvVars::override in case of different master/slave
-                String overridenPaths = vars.get("PATH");
-                overridenPaths += paths.toListString();
-                vars.override("PATH", overridenPaths);
+                vars.override("PATH+", paths.toListString());
                 vars.putAll(homes);
                 vars.putAll(versions);
 
                 // Inject additional variables
                 for (EnvVariablesInjector injector : additionalVarInjectors) {
                     injector.Inject(vars);
                 }
+
+                // Override vars
+                String overallPaths=vars.get("PATH");
+                vars.remove("PATH");
+                vars.put("PATH+", overallPaths);
 
-                return getInner().launch(starter.envs(Util.mapToEnv(vars)));
+                return getInner().launch(starter.envs(vars));
             }
 
             private EnvVars toEnvVars(String[] envs) {

diff --git a/src/main/java/com/synopsys/arc/jenkinsci/plugins/customtools/PathsList.java b/src/main/java/com/synopsys/arc/jenkinsci/plugins/customtools/PathsList.java
@@ -68,15 +68,17 @@ public boolean add(String path) {
     /**
      * Adds PathsList and overrides null variables.
      * @param pathsList PathsList to be added
-     * @return True if 
+     * @return True if the paths list has been modified after the tool installation
      */
+    //TODO: Is it a bug?
     public boolean add(PathsList pathsList) {
         if (pathSeparator == null) {
             pathSeparator = pathsList.pathSeparator;
         }
         if (separator == null) {
             separator = pathsList.separator;
         }
+        // Add homeDir as well (legacy behavior)
         if (pathsList.homeDir != null) {
             this.paths.add(pathsList.homeDir);
         }
@@ -86,14 +88,10 @@ public boolean add(PathsList pathsList) {
 
     public String toListString() {
         StringBuilder builder = new StringBuilder(paths.size()*2);
-        for ( String path : paths) {
-            builder.append(pathSeparator);
+        for ( String path : paths) {         
             builder.append(path);
-        } 
-
-        // Add homeDir as well (legacy behavior)
-        builder.append(pathSeparator);
-
+            builder.append(pathSeparator);
+        }            
         return builder.toString();
     }
 }