fixed #452 "Windows slave won't execute dos commands".

Env variable overrides like PATH needs to be done carefully, so that override happens on the slave, not on the master.


git-svn-id: https://hudson.dev.java.net/svn/hudson/trunk/hudson/main@3201 71c3de6d-444a-0410-be80-ed276b4c234a

core/src/main/java/hudson/EnvVars.java

 package hudson;
 
 import java.util.Map;
+import java.util.HashMap;
+import java.util.Map.Entry;
+import java.io.File;
 
 /**
  * Environment variables.
+ *
+ * <p>
+ * In Hudson, often we need to build up "environment variable overrides"
+ * on master, then to execute the process on slaves. This causes a problem
+ * when working with variables like <tt>PATH</tt>. So to make this work,
+ * we introduce a special convention <tt>PATH+FOO</tt> &mdash; all entries
+ * that starts with <tt>PATH+</tt> are merged and prepended to the inherited
+ * <tt>PATH</tt> variable, on the process where a new process is executed. 
  * 
  * @author Kohsuke Kawaguchi
  */
-public class EnvVars {
+public class EnvVars extends HashMap<String,String> {
+
+    public EnvVars() {
+    }
+
+    public EnvVars(Map<String,String> m) {
+        super(m);
+    }
+
+    /**
+     * Overrides the current entry by the given entry.
+     *
+     * <p>
+     * Handles <tt>PATH+XYZ</tt> notation.
+     */
+    public void override(String key, String value) {
+        if(value==null || value.length()==0) {
+            remove(key);
+            return;
+        }
+
+        int idx = key.indexOf('+');
+        if(idx>0) {
+            String realKey = key.substring(0,idx);
+            String v = get(realKey);
+            if(v==null) v=value;
+            else        v=value+File.pathSeparatorChar+v;
+            put(realKey,value);
+            return;
+        }
+
+        put(key,value);
+    }
+
+    public void overrideAll(Map<String,String> all) {
+        for (Map.Entry<String, String> e : all.entrySet()) {
+            override(e.getKey(),e.getValue());
+        }
+    }
+
     /**
      * Environmental variables that we've inherited.
      */

core/src/main/java/hudson/Launcher.java

@@ -18,7 +18,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.BufferedOutputStream;
-import java.util.HashMap;
 import java.util.Map;
 
 /**
@@ -192,23 +191,6 @@ public abstract class Launcher {
                 }
             };
         }
-
-        /**
-         * Expands the list of environment variables by inheriting current env variables.
-         */
-        private Map<String,String> inherit(String[] env) {
-            Map<String,String> m = new HashMap<String,String>(EnvVars.masterEnvVars);
-            for (String e : env) {
-                int index = e.indexOf('=');
-                String key = e.substring(0,index);
-                String value = e.substring(index+1);
-                if(value.length()==0)
-                    m.remove(key);
-                else
-                    m.put(key,value);
-            }
-            return m;
-        }
     }
 
     /**
@@ -307,12 +289,24 @@ public abstract class Launcher {
         private static final long serialVersionUID = 1L;
     }
 
+    /**
+     * Expands the list of environment variables by inheriting current env variables.
+     */
+    private static Map<String,String> inherit(String[] env) {
+        EnvVars m = new EnvVars(EnvVars.masterEnvVars);
+        for (String e : env) {
+            int index = e.indexOf('=');
+            m.override(e.substring(0,index), e.substring(index+1));
+        }
+        return m;
+    }
+
     /**
      * Expands the list of environment variables by inheriting current env variables.
      */
     private static Map<String,String> inherit(Map<String,String> overrides) {
-        Map<String,String> m = new HashMap<String,String>(EnvVars.masterEnvVars);
-        m.putAll(overrides);
+        EnvVars m = new EnvVars(EnvVars.masterEnvVars);
+        m.overrideAll(overrides);
         return m;
     }
 

core/src/main/java/hudson/model/JDK.java

 package hudson.model;
 
-import hudson.EnvVars;
-
 import java.io.File;
 import java.util.Map;
 
@@ -63,15 +61,9 @@ public final class JDK {
      * Sets PATH and JAVA_HOME from this JDK.
      */
     public void buildEnvVars(Map<String,String> env) {
-        String path = env.get("PATH");
-        if(path==null)
-            path = EnvVars.masterEnvVars.get("PATH");
-        
-        if(path==null)
-            path = getBinDir().getPath();
-        else
-            path = getBinDir().getPath()+File.pathSeparator+path;
-        env.put("PATH",path);
+        // see EnvVars javadoc for why this adss PATH.
+        env.put("PATH+JDK",getBinDir().getPath());
+
         env.put("JAVA_HOME",javaHome);
         if(!env.containsKey("HUDSON_HOME"))
             env.put("HUDSON_HOME", Hudson.getInstance().getRootDir().getPath() );