[JENKINS-20560] - Don't try to override empty paths

Caused by the initial fix of https://issues.jenkins-ci.org/browse/JENKINS-20560 Signed-off-by: Oleg Nenashev <nenashev@synopsys.com>
jenkinsci · Nov 20, 2013 · 84cac80 · 84cac80
1 parent 1ed68b4
commit 84cac80
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 9 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
@@ -214,20 +214,25 @@ public Proc launch(ProcStarter starter) throws IOException {
                     vars = new EnvVars();
                 }
 
-                // HACK: Avoids issue with invalid separators in EnvVars::override in case of different master/slave
-                vars.override("PATH+", paths.toListString());
+                // Inject paths
+                final String injectedPaths = paths.toListString();              
+                if (injectedPaths != null) {              
+                    vars.override("PATH+", injectedPaths);
+                }
+
+                // Inject additional variables
                 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);
+                // Override paths to prevent JENKINS-20560              
+                if (vars.containsKey("PATH")) {
+                    final String overallPaths=vars.get("PATH");
+                    vars.remove("PATH");
+                    vars.put("PATH+", overallPaths);
+                }
 
                 return getInner().launch(starter.envs(vars));
             }

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
@@ -86,8 +86,16 @@ public boolean add(PathsList pathsList) {
         return this.paths.addAll(pathsList.paths);
     }
 
+    /**
+     * Gets the list of installed tools
+     * @return A list with valid delimiters or null if paths is empty
+     */
     public String toListString() {
-        StringBuilder builder = new StringBuilder(paths.size()*2);
+        if (paths.isEmpty()) {
+            return null;
+        }
+
+        StringBuilder builder = new StringBuilder();
         for ( String path : paths) {         
             builder.append(path);
             builder.append(pathSeparator);

diff --git a/src/test/java/com/cloudbees/jenkins/plugins/customtools/CustomToolInstallWrapperTest.java b/src/test/java/com/cloudbees/jenkins/plugins/customtools/CustomToolInstallWrapperTest.java
@@ -58,6 +58,8 @@ public void testNestedWrapperReverse() throws Exception {
         nestedWrapperTestImpl(wrappers, true);
     }
 
+
+
     /**
      * Tests custom tools with wrapper, which calls wrapper without
      * specifying of envs.
@@ -71,6 +73,14 @@ public void testNestedLauncherCalls() throws Exception {
         nestedWrapperTestImpl(wrappers, false);
     }
 
+    //@Bug(20560)
+    public void testEmptyToolsList() throws Exception {
+        List<BuildWrapper> wrappers = new ArrayList<BuildWrapper>(0); 
+        wrappers.add(new CommandCallerInstaller());
+        wrappers.add(new CustomToolInstallWrapper(null, MulticonfigWrapperOptions.DEFAULT, false));
+        nestedWrapperTestImpl(wrappers, false);
+    }
+
     /**
      * Implements tests for nested wrappers.
      * The test checks that environment variables have been set correctly.