From e6fdc19cf9174390386a98935ff1d7cf81dbe82f Mon Sep 17 00:00:00 2001
From: Stephen Connolly <stephen.alan.connolly@gmail.com>
Date: Mon, 16 Mar 2015 12:36:04 +0000
Subject: [PATCH] [JENKINS-27058] Clarify that remoteFS can be relative if you
 know what you are doing

- Get the absolute path on connect and use that while the connection is on-line (which is ok as when the connection is not online a null was returned from getRootPath() anyway)
---
 core/src/main/java/hudson/model/Slave.java    | 50 ++++++++++------
 .../java/hudson/slaves/CommandLauncher.java   | 23 ++++----
 .../java/hudson/slaves/SlaveComputer.java     | 59 ++++++++++++++-----
 .../hudson/model/Messages.properties          |  4 +-
 4 files changed, 90 insertions(+), 46 deletions(-)

diff --git a/core/src/main/java/hudson/model/Slave.java b/core/src/main/java/hudson/model/Slave.java
index 92ac16518d..6d32b29e94 100644
--- a/core/src/main/java/hudson/model/Slave.java
+++ b/core/src/main/java/hudson/model/Slave.java
@@ -1,19 +1,19 @@
 /*
  * The MIT License
- * 
+ *
  * Copyright (c) 2004-2011, Sun Microsystems, Inc., Kohsuke Kawaguchi,
  * Erik Ramfelt, Martin Eigenbrodt, Stephen Connolly, Tom Huybrechts
- * 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
  * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
  * copies of the Software, and to permit persons to whom the Software is
  * furnished to do so, subject to the following conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -63,6 +63,7 @@ import jenkins.security.MasterToSlaveCallable;
 import jenkins.slaves.WorkspaceLocator;
 
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.StaplerRequest;
@@ -92,8 +93,12 @@ public abstract class Slave extends Node implements Serializable {
     private final String description;
 
     /**
-     * Absolute path to the root of the workspace
-     * from the view point of this node, such as "/hudson"
+     * Path to the root of the workspace from the view point of this node, such as "/hudson", this need not
+     * be absolute provided that the launcher establishes a consistent working directory, such as "./.jenkins-slave"
+     * when used with an SSH launcher.
+     *
+     * NOTE: if the administrator is using a relative path they are responsible for ensuring that the launcher used
+     * provides a consistent working directory
      */
     protected final String remoteFS;
 
@@ -121,14 +126,14 @@ public abstract class Slave extends Node implements Serializable {
      * Whitespace-separated labels.
      */
     private String label="";
-    
+
     private /*almost final*/ DescribableList<NodeProperty<?>,NodePropertyDescriptor> nodeProperties = new DescribableList<NodeProperty<?>,NodePropertyDescriptor>(Jenkins.getInstance());
 
     /**
      * Lazily computed set of labels from {@link #label}.
      */
     private transient volatile Set<Label> labels;
-    
+
     /**
      * Id of user which creates this slave {@link User}.
      */
@@ -147,7 +152,7 @@ public abstract class Slave extends Node implements Serializable {
             Mode mode, String labelString, ComputerLauncher launcher, RetentionStrategy retentionStrategy) throws FormException, IOException {
     	this(name, nodeDescription, remoteFS, numExecutors, mode, labelString, launcher, retentionStrategy, new ArrayList());
     }
-    
+
     public Slave(@Nonnull String name, String nodeDescription, String remoteFS, int numExecutors,
                  Mode mode, String labelString, ComputerLauncher launcher, RetentionStrategy retentionStrategy, List<? extends NodeProperty<?>> nodeProperties) throws FormException, IOException {
         this.name = name;
@@ -159,7 +164,7 @@ public abstract class Slave extends Node implements Serializable {
         this.launcher = launcher;
         this.retentionStrategy = retentionStrategy;
         getAssignedLabels();    // compute labels now
-        
+
         this.nodeProperties.replaceBy(nodeProperties);
          Slave node = (Slave) Jenkins.getInstance().getNode(name);
 
@@ -168,7 +173,7 @@ public abstract class Slave extends Node implements Serializable {
         }
        else{
             User user = User.current();
-            userId = user!=null ? user.getId() : "anonymous";     
+            userId = user!=null ? user.getId() : "anonymous";
         }
         if (name.equals(""))
             throw new FormException(Messages.Slave_InvalidConfig_NoName(), null);
@@ -179,10 +184,10 @@ public abstract class Slave extends Node implements Serializable {
         if (this.numExecutors<=0)
             throw new FormException(Messages.Slave_InvalidConfig_Executors(name), null);
     }
-    
+
     /**
      * Return id of user which created this slave
-     * 
+     *
      * @return id of user
      */
     public String getUserId() {
@@ -192,7 +197,7 @@ public abstract class Slave extends Node implements Serializable {
     public void setUserId(String userId){
         this.userId = userId;
     }
-    
+
     public ComputerLauncher getLauncher() {
         return launcher == null ? new JNLPLauncher() : launcher;
     }
@@ -214,7 +219,7 @@ public abstract class Slave extends Node implements Serializable {
     }
 
     public void setNodeName(String name) {
-        this.name = name; 
+        this.name = name;
     }
 
     public String getNodeDescription() {
@@ -237,7 +242,7 @@ public abstract class Slave extends Node implements Serializable {
         assert nodeProperties != null;
     	return nodeProperties;
     }
-    
+
     public RetentionStrategy getRetentionStrategy() {
         return retentionStrategy == null ? RetentionStrategy.Always.INSTANCE : retentionStrategy;
     }
@@ -273,14 +278,21 @@ public abstract class Slave extends Node implements Serializable {
                 return workspace;
             }
         }
-        
+
         FilePath r = getWorkspaceRoot();
         if(r==null)     return null;    // offline
         return r.child(item.getFullName());
     }
 
+    @CheckForNull
     public FilePath getRootPath() {
-        return createPath(remoteFS);
+        final SlaveComputer computer = getComputer();
+        if (computer == null) {
+            // if computer is null then channel is null and thus we were going to return null anyway
+            return null;
+        } else {
+            return createPath(StringUtils.defaultString(computer.getAbsoluteRemoteFs(), remoteFS));
+        }
     }
 
     /**
@@ -425,7 +437,7 @@ public abstract class Slave extends Node implements Serializable {
             if (!value.contains("\\") && !value.startsWith("/")) {
                 // Unix-looking path that doesn't start with '/'
                 // TODO: detect Windows-looking relative path
-                return FormValidation.error(Messages.Slave_the_remote_root_must_be_an_absolute_path());
+                return FormValidation.error(Messages.Slave_Remote_Relative_Path_Warning());
             }
 
             return FormValidation.ok();
diff --git a/core/src/main/java/hudson/slaves/CommandLauncher.java b/core/src/main/java/hudson/slaves/CommandLauncher.java
index 8d08f9b39c..eceb307f0b 100644
--- a/core/src/main/java/hudson/slaves/CommandLauncher.java
+++ b/core/src/main/java/hudson/slaves/CommandLauncher.java
@@ -1,18 +1,18 @@
 /*
  * The MIT License
- * 
+ *
  * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Stephen Connolly
- * 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
  * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
  * copies of the Software, and to permit persons to whom the Software is
  * furnished to do so, subject to the following conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -41,6 +41,7 @@ import java.util.Date;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.DataBoundConstructor;
 import org.kohsuke.stapler.QueryParameter;
 
@@ -57,7 +58,7 @@ public class CommandLauncher extends ComputerLauncher {
      * "ssh myslave java -jar /path/to/hudson-remoting.jar"
      */
     private final String agentCommand;
-    
+
     /**
      * Optional environment variables to add to the current environment. Can be null.
      */
@@ -67,7 +68,7 @@ public class CommandLauncher extends ComputerLauncher {
     public CommandLauncher(String command) {
         this(command, null);
     }
-    
+
     public CommandLauncher(String command, EnvVars env) {
     	this.agentCommand = command;
     	this.env = env;
@@ -90,10 +91,10 @@ public class CommandLauncher extends ComputerLauncher {
         Process _proc = null;
         try {
             Slave node = computer.getNode();
-            if (node == null) { 
+            if (node == null) {
                 throw new AbortException("Cannot launch commands on deleted nodes");
             }
-            
+
             listener.getLogger().println(hudson.model.Messages.Slave_Launching(getTimestamp()));
             if(getCommand().trim().length()==0) {
                 listener.getLogger().println(Messages.CommandLauncher_NoLaunchCommand());
@@ -103,8 +104,8 @@ public class CommandLauncher extends ComputerLauncher {
 
             ProcessBuilder pb = new ProcessBuilder(Util.tokenize(getCommand()));
             final EnvVars cookie = _cookie = EnvVars.createCookie();
-            pb.environment().putAll(cookie);      
-            pb.environment().put("WORKSPACE", node.getRemoteFS()); //path for local slave log
+            pb.environment().putAll(cookie);
+            pb.environment().put("WORKSPACE", StringUtils.defaultString(computer.getAbsoluteRemoteFs(), node.getRemoteFS())); //path for local slave log
 
             {// system defined variables
                 String rootUrl = Jenkins.getInstance().getRootUrl();
@@ -118,7 +119,7 @@ public class CommandLauncher extends ComputerLauncher {
             if (env != null) {
             	pb.environment().putAll(env);
             }
-            
+
             final Process proc = _proc = pb.start();
 
             // capture error information from stderr. this will terminate itself
diff --git a/core/src/main/java/hudson/slaves/SlaveComputer.java b/core/src/main/java/hudson/slaves/SlaveComputer.java
index 0a28b2064d..02ed1b94d2 100644
--- a/core/src/main/java/hudson/slaves/SlaveComputer.java
+++ b/core/src/main/java/hudson/slaves/SlaveComputer.java
@@ -1,18 +1,18 @@
 /*
  * The MIT License
- * 
+ *
  * Copyright (c) 2004-2009, Sun Microsystems, Inc., Kohsuke Kawaguchi, Stephen Connolly
- * 
+ *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
  * in the Software without restriction, including without limitation the rights
  * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
  * copies of the Software, and to permit persons to whom the Software is
  * furnished to do so, subject to the following conditions:
- * 
+ *
  * The above copyright notice and this permission notice shall be included in
  * all copies or substantial portions of the Software.
- * 
+ *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
  * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
@@ -102,7 +102,7 @@ public class SlaveComputer extends Computer {
      *
      * <p>
      * This is normally the same as {@link Slave#getLauncher()} but
-     * can be different. See {@link #grabLauncher(Node)}. 
+     * can be different. See {@link #grabLauncher(Node)}.
      */
     private ComputerLauncher launcher;
 
@@ -132,6 +132,8 @@ public class SlaveComputer extends Computer {
 
     private Object constructed = new Object();
 
+    private transient volatile String absoluteRemoteFs;
+
     public SlaveComputer(Slave slave) {
         super(slave);
         this.log = new ReopenableRotatingFileOutputStream(getLogFile(),10);
@@ -274,7 +276,7 @@ public class SlaveComputer extends Computer {
         if (launcher instanceof ExecutorListener) {
             ((ExecutorListener)launcher).taskAccepted(executor, task);
         }
-        
+
         //getNode() can return null at indeterminate times when nodes go offline
         Slave node = getNode();
         if (node != null && node.getRetentionStrategy() instanceof ExecutorListener) {
@@ -410,6 +412,11 @@ public class SlaveComputer extends Computer {
         return channel.call(new LoadingTime(true));
     }
 
+    @CheckForNull
+    public String getAbsoluteRemoteFs() {
+        return channel == null ? null : absoluteRemoteFs;
+    }
+
     static class LoadingCount extends MasterToSlaveCallable<Integer,RuntimeException> {
         private final boolean resource;
         LoadingCount(boolean resource) {
@@ -481,11 +488,16 @@ public class SlaveComputer extends Computer {
         if (node == null) { // Node has been disabled/removed during the connection
             throw new IOException("Node "+nodeName+" has been deleted during the channel setup");
         }
-        
-        String remoteFs = node.getRemoteFS();
-        if(_isUnix && !remoteFs.contains("/") && remoteFs.contains("\\"))
-            log.println("WARNING: "+remoteFs+" looks suspiciously like Windows path. Maybe you meant "+remoteFs.replace('\\','/')+"?");
-        FilePath root = new FilePath(channel,remoteFs);
+
+        String absoluteRemoteFs = node.getRemoteFS();
+        if (!absoluteRemoteFs.startsWith("\\") && !absoluteRemoteFs.startsWith("/")) {
+            absoluteRemoteFs = channel.call(new AbsolutePath(absoluteRemoteFs));
+            log.println("NOTE: Relative remote path resolved to: "+absoluteRemoteFs);
+        }
+        if(_isUnix && !absoluteRemoteFs.contains("/") && absoluteRemoteFs.contains("\\"))
+            log.println("WARNING: "+absoluteRemoteFs
+                    +" looks suspiciously like Windows path. Maybe you meant "+absoluteRemoteFs.replace('\\','/')+"?");
+        FilePath root = new FilePath(channel,absoluteRemoteFs);
 
         // reference counting problem is known to happen, such as JENKINS-9017, and so as a preventive measure
         // we pin the base classloader so that it'll never get GCed. When this classloader gets released,
@@ -519,6 +531,7 @@ public class SlaveComputer extends Computer {
             isUnix = _isUnix;
             numRetryAttempt = 0;
             this.channel = channel;
+            this.absoluteRemoteFs = absoluteRemoteFs;
             defaultCharset = Charset.forName(defaultCharsetName);
 
             synchronized (statusChangeLock) {
@@ -634,9 +647,13 @@ public class SlaveComputer extends Computer {
      */
     private void closeChannel() {
         // TODO: race condition between this and the setChannel method.
-        Channel c = channel;
-        channel = null;
-        isUnix = null;
+        Channel c;
+        synchronized (channelLock) {
+            c = channel;
+            channel = null;
+            absoluteRemoteFs = null;
+            isUnix = null;
+        }
         if (c != null) {
             try {
                 c.close();
@@ -708,6 +725,18 @@ public class SlaveComputer extends Computer {
         }
     }
 
+    private static final class AbsolutePath extends MasterToSlaveCallable<String,IOException> {
+        private final String relativePath;
+
+        private AbsolutePath(String relativePath) {
+            this.relativePath = relativePath;
+        }
+
+        public String call() throws IOException {
+            return new File(relativePath).getAbsolutePath();
+        }
+    }
+
     private static final class DetectDefaultCharset extends MasterToSlaveCallable<String,IOException> {
         public String call() throws IOException {
             return Charset.defaultCharset().name();
@@ -744,7 +773,7 @@ public class SlaveComputer extends Computer {
             }
 
             Channel.current().setProperty("slave",Boolean.TRUE); // indicate that this side of the channel is the slave side.
-            
+
             return null;
         }
         private static final long serialVersionUID = 1L;
diff --git a/core/src/main/resources/hudson/model/Messages.properties b/core/src/main/resources/hudson/model/Messages.properties
index 1629372412..54261e2110 100644
--- a/core/src/main/resources/hudson/model/Messages.properties
+++ b/core/src/main/resources/hudson/model/Messages.properties
@@ -244,7 +244,9 @@ Slave.Launching={0} Launching slave agent
 Slave.Network.Mounted.File.System.Warning=Are you sure you want to use network mounted file system for FS root? Note that this directory does not need to be visible to the master.
 Slave.Remote.Director.Mandatory=Remote directory is mandatory
 Slave.Terminated={0} slave agent was terminated
-Slave.the_remote_root_must_be_an_absolute_path=The remote root must be an absolute path.
+Slave.Remote.Relative.Path.Warning=Are you sure you want to use a relative path for the FS root? Note that relative \
+  paths require that you can assure that the selected launcher provides a consistent current working directory. Using \
+  an absolute path is strongly encouraged and highly recommended.
 Slave.UnableToLaunch=Unable to launch the slave agent for {0}{1}
 Slave.UnixSlave=This is a Unix slave
 Slave.WindowsSlave=This is a Windows slave
-- 
GitLab