From 1dfd6671e7f6ce22c834fb38f8f110456f684924 Mon Sep 17 00:00:00 2001
From: Darko Poljak <darko.poljak@gmail.com>
Date: Fri, 18 Mar 2016 21:57:50 +0100
Subject: [PATCH] Fix #416: error for non-posix remote shell. Fix remote.py
 test errors.

---
 cdist/exec/remote.py      | 27 ++++++++++++++++++++++-----
 cdist/test/exec/remote.py | 31 ++++++++++++++++++++++++++++---
 2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py
index 9b7d5d1c..c99cc77c 100644
--- a/cdist/exec/remote.py
+++ b/cdist/exec/remote.py
@@ -130,12 +130,29 @@ class Remote(object):
         # FIXME: replace this by -o SendEnv name -o SendEnv name ... to ssh?
         # can't pass environment to remote side, so prepend command with
         # variable declarations
+
+        # cdist command prepended with variable assignments expects
+        # posix shell (bourne, bash) at the remote as user default shell.
+        # If remote user shell isn't poxis shell, but for e.g. csh/tcsh
+        # then these var assignments are not var assignments for this
+        # remote shell, it tries to execute it as a command and fails.
+        # So really do this by default:
+        # /bin/sh -c 'export <var assignments>; command'
+        # so that constructed remote command isn't dependent on remote
+        # shell. Do this only if env is not None. env breaks this.
+        # Explicitly use /bin/sh, because var assignments assume poxis
+        # shell already.
+        # This leaves the posibility to write script that needs to be run
+        # remotely in e.g. csh and setting up CDIST_REMOTE_SHELL to e.g.
+        # /bin/csh will execute this script in the right way.
         if env:
-            remote_env = ["%s=%s" % item for item in env.items()]
-            cmd.extend(remote_env)
-
-        cmd.extend(command)
-
+            cmd.append("/bin/sh")
+            cmd.append("-c")
+            remote_env = [" export %s=%s;" % item for item in env.items()]
+            string_cmd = " ".join(remote_env) + " ".join(command)
+            cmd.append(string_cmd)
+        else:
+            cmd.extend(command)
         return self._run_command(cmd, env=env, return_output=return_output)
 
     def _run_command(self, command, env=None, return_output=False):
diff --git a/cdist/test/exec/remote.py b/cdist/test/exec/remote.py
index 8e7d408a..7d61b85c 100644
--- a/cdist/test/exec/remote.py
+++ b/cdist/test/exec/remote.py
@@ -39,7 +39,7 @@ class RemoteTestCase(test.CdistTestCase):
         user = getpass.getuser()
         remote_exec = "ssh -o User=%s -q" % user
         remote_copy = "scp -o User=%s -q" % user
-        self.remote = remote.Remote(self.target_host, self.base_path, remote_exec, remote_copy)
+        self.remote = remote.Remote(self.target_host, base_path=self.base_path, remote_exec=remote_exec, remote_copy=remote_copy)
 
     def tearDown(self):
         shutil.rmtree(self.temp_dir)
@@ -125,7 +125,7 @@ class RemoteTestCase(test.CdistTestCase):
         os.chmod(remote_exec_path, 0o755)
         remote_exec = remote_exec_path
         remote_copy = "echo"
-        r = remote.Remote(self.target_host, self.base_path, remote_exec, remote_copy)
+        r = remote.Remote(self.target_host, base_path=self.base_path, remote_exec=remote_exec, remote_copy=remote_copy)
         self.assertEqual(r.run('/bin/true', return_output=True), "%s\n" % self.target_host)
 
     def test_run_script_target_host_in_env(self):
@@ -135,8 +135,33 @@ class RemoteTestCase(test.CdistTestCase):
         os.chmod(remote_exec_path, 0o755)
         remote_exec = remote_exec_path
         remote_copy = "echo"
-        r = remote.Remote(self.target_host, self.base_path, remote_exec, remote_copy)
+        r = remote.Remote(self.target_host, base_path=self.base_path, remote_exec=remote_exec, remote_copy=remote_copy)
         handle, script = self.mkstemp(dir=self.temp_dir)
         with os.fdopen(handle, "w") as fd:
             fd.writelines(["#!/bin/sh\n", "/bin/true"])
         self.assertEqual(r.run_script(script, return_output=True), "%s\n" % self.target_host)
+
+    def test_run_script_with_env_target_host_in_env(self):
+        handle, script = self.mkstemp(dir=self.temp_dir)
+        with os.fdopen(handle, "w") as fd:
+            fd.writelines(["#!/bin/sh\n", 'if [ "$__object" ]; then echo $__object; else echo no_env; fi\n'])
+        os.chmod(script, 0o755)
+        handle, remote_exec_path = self.mkstemp(dir=self.temp_dir)
+        with os.fdopen(handle, 'w') as fd:
+            fd.writelines(["#!/bin/sh\n", 'shift; cmd=$1; shift; $cmd "$@"\n'])
+        os.chmod(remote_exec_path, 0o755)
+        remote_exec = remote_exec_path
+        remote_copy = "echo"
+        r = remote.Remote(self.target_host, base_path=self.base_path, remote_exec=remote_exec, remote_copy=remote_copy)
+        output = r.run_script(script, return_output=True)
+        self.assertEqual(output, "no_env\n")
+        env = {
+            '__object': 'test_object',
+        }
+        output = r.run_script(script, env=env, return_output=True)
+        self.assertEqual(output, "test_object\n")
+
+if __name__ == '__main__':
+    import unittest
+
+    unittest.main()