[JENKINS-38837] Adjust multibranch workspace calculation on master to…

… match customized workspaceDir.
jenkinsci · Jun 12, 2017 · 4bf7c68 · 4bf7c68
1 parent 137ce4a
commit 4bf7c68
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 2 deletions.
diff --git a/src/main/java/jenkins/branch/WorkspaceLocatorImpl.java b/src/main/java/jenkins/branch/WorkspaceLocatorImpl.java
@@ -26,15 +26,18 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Charsets;
+import com.google.common.collect.ImmutableMap;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.Extension;
 import hudson.FilePath;
+import hudson.Util;
 import hudson.model.Computer;
 import hudson.model.Item;
 import hudson.model.Node;
 import hudson.model.Slave;
 import hudson.model.TopLevelItem;
 import hudson.model.listeners.ItemListener;
+import java.io.File;
 import java.io.IOException;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
@@ -74,7 +77,11 @@ public FilePath locate(TopLevelItem item, Node node) {
         }
         String minimized = minimize(item.getFullName());
         if (node instanceof Jenkins) {
-            return ((Jenkins) node).getRootPath().child("workspace/" + minimized);
+            String workspaceDir = ((Jenkins) node).getRawWorkspaceDir();
+            if (!workspaceDir.contains("ITEM_FULL")) {
+                LOGGER.log(Level.WARNING, "JENKINS-34564 path sanitization ineffective when using legacy Workspace Root Directory ‘{0}’; switch to $'{'JENKINS_HOME'}'/workspace/$'{'ITEM_FULLNAME'}' as in JENKINS-8446 / JENKINS-21942", workspaceDir);
+            }
+            return new FilePath(new File(expandVariablesForDirectory(workspaceDir, minimized, item.getRootDir().getPath())));
         } else if (node instanceof Slave) {
             FilePath root = ((Slave) node).getWorkspaceRoot();
             return root != null ? root.child(minimized) : null;
@@ -83,6 +90,16 @@ public FilePath locate(TopLevelItem item, Node node) {
         }
     }
 
+    /** copied from {@link Jenkins} */
+    static String expandVariablesForDirectory(String base, String itemFullName, String itemRootDir) {
+        return Util.replaceMacro(base, ImmutableMap.of(
+                "JENKINS_HOME", Jenkins.getInstance().getRootDir().getPath(),
+                "ITEM_ROOTDIR", itemRootDir,
+                "ITEM_FULLNAME", itemFullName,   // legacy, deprecated
+                "ITEM_FULL_NAME", itemFullName.replace(':','$'))); // safe, see JENKINS-12251
+
+    }
+
     private static String uniqueSuffix(String name) {
         // TODO still in beta: byte[] sha256 = Hashing.sha256().hashString(name).asBytes();
         byte[] sha256;

diff --git a/src/test/java/jenkins/branch/WorkspaceLocatorImplTest.java b/src/test/java/jenkins/branch/WorkspaceLocatorImplTest.java
@@ -24,14 +24,17 @@
 
 package jenkins.branch;
 
+import hudson.FilePath;
 import hudson.model.FreeStyleProject;
 import hudson.scm.NullSCM;
 import hudson.slaves.DumbSlave;
 import hudson.slaves.WorkspaceList;
 import java.io.File;
+import java.lang.reflect.Field;
 import java.util.Collections;
 import static jenkins.branch.NoTriggerBranchPropertyTest.showComputation;
 import jenkins.branch.harness.MultiBranchImpl;
+import jenkins.model.Jenkins;
 import jenkins.scm.impl.SingleSCMSource;
 import org.junit.Test;
 import static org.junit.Assert.*;
@@ -118,9 +121,10 @@ public void minimize() {
         assertEquals("U-OSF24EPB4C", WorkspaceLocatorImpl.minimize("blahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahblahXYZWVU"));
     }
 
-    @Issue("JENKINS-34564")
+    @Issue({"JENKINS-34564", "JENKINS-38837"})
     @Test
     public void locate() throws Exception {
+        assertEquals("${JENKINS_HOME}/workspace/${ITEM_FULLNAME}", r.jenkins.getRawWorkspaceDir());
         MultiBranchImpl stuff = r.createProject(MultiBranchImpl.class, "stuff");
         stuff.getSourcesList().add(new BranchSource(new SingleSCMSource(null, "dev/flow", new NullSCM())));
         stuff.scheduleBuild2(0).getFuture().get();
@@ -133,6 +137,15 @@ public void locate() throws Exception {
         assertEquals(slave.getWorkspaceRoot().child("stuff_dev_flow-L5GKER67QGVMJ2UD3JCSGKEV2ACON2O4VO4RNUZ27HGUY32SYVXQ"), slave.getWorkspaceFor(master));
         FreeStyleProject unrelated = r.createFreeStyleProject("100% crazy");
         assertEquals(r.jenkins.getRootPath().child("workspace/100% crazy"), r.jenkins.getWorkspaceFor(unrelated));
+        // Checking other values of workspaceDir.
+        Field workspaceDir = Jenkins.class.getDeclaredField("workspaceDir"); // currently settable only by Jenkins.doConfigSubmit
+        workspaceDir.setAccessible(true);
+        // Poor historical default, and as per JENKINS-21942 even possible after some startup scenarios:
+        workspaceDir.set(r.jenkins, "${ITEM_ROOTDIR}/workspace");
+        assertEquals("JENKINS-34564 inactive in this case", new FilePath(master.getRootDir()).child("workspace"), r.jenkins.getWorkspaceFor(master));
+        // JENKINS-38837: customized root.
+        workspaceDir.set(r.jenkins, "${JENKINS_HOME}/ws/${ITEM_FULLNAME}");
+        assertEquals("ITEM_FULLNAME interpreted a little differently", r.jenkins.getRootPath().child("ws/stuff_dev_flow-L5GKER67QGVMJ2UD3JCSGKEV2ACON2O4VO4RNUZ27HGUY32SYVXQ"), r.jenkins.getWorkspaceFor(master));
     }
 
     @Issue({"JENKINS-2111", "JENKINS-41068"})