[FIXED JENKINS-15348] applying patch provided in ticket

jenkinsci · Sep 28, 2012 · a807f5e · a807f5e
1 parent 0e5b7c4
commit a807f5e
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 38 deletions.
diff --git a/src/main/java/hudson/plugins/perforce/PerforceSCM.java b/src/main/java/hudson/plugins/perforce/PerforceSCM.java
@@ -268,6 +268,7 @@ public class PerforceSCM extends SCM {
     private String p4Charset = null;
     private String p4CommandCharset = null;
 
+    // Plugin constructor, (only?) used when a job configuration is saved
     @DataBoundConstructor
     public PerforceSCM(
             String p4User,
@@ -334,43 +335,11 @@ public PerforceSCM(
 
         this.clientOwner = Util.fixEmptyAndTrim(clientOwner);
 
-        if (p4SysRoot != null && p4SysRoot.length() != 0)
-            this.p4SysRoot = Util.fixEmptyAndTrim(p4SysRoot);
-
-        if (p4SysDrive != null && p4SysDrive.length() != 0)
-            this.p4SysDrive = Util.fixEmptyAndTrim(p4SysDrive);
-
-        // Get systemDrive,systemRoot computer environment variables from
-        // the current machine.
-        String systemDrive = null;
-        String systemRoot = null;
-        if (Hudson.isWindows()) {
-            try {
-                Computer currentComputer = Computer.currentComputer();
-                // A master with no executors seems to throw an NPE here, so
-                // we need to check for null.
-                if (currentComputer != null) {
-                    EnvVars envVars = currentComputer.getEnvironment();
-                    systemDrive = envVars.get("SystemDrive");
-                    systemRoot = envVars.get("SystemRoot");
-                }
-            } catch (Exception ex) {
-                LOGGER.log(Level.WARNING, ex.getMessage(), ex);
-            }
+        if (p4SysRoot != null) {
+            this.p4SysRoot = p4SysRoot.trim();
         }
-        if (p4SysRoot != null && p4SysRoot.length() != 0) {
-            this.p4SysRoot = Util.fixEmptyAndTrim(p4SysRoot);
-        } else {
-            if (systemRoot != null && !systemRoot.trim().equals("")) {
-                this.p4SysRoot = Util.fixEmptyAndTrim(systemRoot);
-            }
-        }
-        if (p4SysDrive != null && p4SysDrive.length() != 0) {
-            this.p4SysDrive = Util.fixEmptyAndTrim(p4SysDrive);
-        } else {
-            if (systemDrive != null && !systemDrive.trim().equals("")) {
-                this.p4SysDrive = Util.fixEmptyAndTrim(systemDrive);
-            }
+        if (p4SysDrive != null) {
+            this.p4SysDrive = p4SysDrive.trim();
         }
 
         this.lineEndValue = lineEndValue;
@@ -433,8 +402,31 @@ protected Depot getDepot(Launcher launcher, FilePath workspace, AbstractProject
             depot.setExecutable(getP4Executable(p4Tool));
         else
             depot.setExecutable(getP4Executable(p4Tool,node,TaskListener.NULL));
-        depot.setSystemDrive(p4SysDrive);
-        depot.setSystemRoot(p4SysRoot);
+
+        // Get systemDrive,systemRoot computer environment variables from
+        // the current machine.
+        // The current machine is the machine about to do something (run a
+        // build, poll the server) according to whatever called getDepot
+        String systemDrive = Util.fixEmptyAndTrim(p4SysDrive);
+        String systemRoot = Util.fixEmptyAndTrim(p4SysRoot);
+        try {
+            Computer currentComputer = Computer.currentComputer();
+            // A master with no executors seems to throw an NPE here, so
+            // we need to check for null.
+            if (currentComputer != null) {
+                EnvVars envVars = currentComputer.getEnvironment();
+                if (systemDrive == null && envVars.containsKey("SystemDrive")) {
+                    systemDrive = envVars.get("SystemDrive");
+                }
+                if (systemRoot == null && envVars.containsKey("SystemRoot")) {
+                    systemRoot = envVars.get("SystemRoot");
+                }
+            }
+        } catch (Exception ex) {
+            LOGGER.log(Level.WARNING, ex.getMessage(), ex);
+        }
+        depot.setSystemDrive(systemDrive);
+        depot.setSystemRoot(systemRoot);
 
         depot.setCharset(p4Charset);
         depot.setCommandCharset(p4CommandCharset);

diff --git a/src/test/java/hudson/plugins/perforce/PerforceSCMTest.java b/src/test/java/hudson/plugins/perforce/PerforceSCMTest.java
@@ -44,6 +44,26 @@ public void testConfigRoundtrip() throws Exception {
         //assertEqualBeans(scm.getBrowser(),p.getScm().getBrowser(),"URL");
     }
 
+    public void testConfigRoundtripWithNoSystemRoot() throws Exception {
+	FreeStyleProject project = createFreeStyleProject();
+        P4Web browser = new P4Web(new URL("http://localhost/"));
+        PerforceSCM scm = new PerforceSCM(
+                        "user", "pass", "client", "port", "", "exe", "",
+                        "", "label", "counter", "upstreamProject", "shared", "charset", "charset2", "user", false, true, true, true, true, true, false,
+                        false, true, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true);
+        assertEquals("", scm.getP4SysDrive());
+        assertEquals("", scm.getP4SysRoot());
+        scm.setProjectPath("path");
+        project.setScm(scm);
+
+        // config roundtrip
+        submit(new WebClient().getPage(project,"configure").getFormByName("config"));
+
+        // verify that the data is intact
+        assertEqualBeans(scm, project.getScm(),
+                "p4SysRoot,p4SysDrive");
+    }
+
     public void testConfigRoundtripWithStream() throws Exception {
         FreeStyleProject project = createFreeStyleProject();
         P4Web browser = new P4Web(new URL("http://localhost/"));