From 5afbe576ac00494fa1df056b08de959543941f59 Mon Sep 17 00:00:00 2001
From: Sergey Orlov <sorlov@redhat.com>
Date: Oct 22 2019 13:03:24 +0000
Subject: Always execute commands and prelude in subshell


Before the fix the prelude was executed in main shell and commands were
executed in nested shell only when they were passed as a string, not as a
list.
This approach produces problem with some  installations
(particularly Cygwin) which have
/etc/bash.bash_logout file  containing invocation of "/usr/bin/clear".
This file is executed when user logs out. When run_command() sends
"exit" command to remote shell, "clear" returns exit code 1 complaining
about TERM variable not being defined, which is normal for non-interactive
sessions. Without "set -e" option this is handled correctly and shell
returns result code of last executed user command. But with "set -e" shell
terminates prematurely and when user command succeeded we receive exit
status 1.

Executing "set -e" and user commands inside subshell solves both problems.

Also adding test for the described scenario.

---

diff --git a/pytest_multihost/host.py b/pytest_multihost/host.py
index 2937296..342c468 100644
--- a/pytest_multihost/host.py
+++ b/pytest_multihost/host.py
@@ -245,25 +245,23 @@ class BaseHost(object):
             quoted = shell_quote(encode(self.env_sh_path))
             command.stdin.write(b'. %s\n' % quoted)
 
-        if self.command_prelude:
-            command.stdin.write(encode(self.command_prelude))
-
         if stdin_text:
             command.stdin.write(b"echo -en ")
             command.stdin.write(_echo_quote(encode(stdin_text)))
             command.stdin.write(b" | ")
 
+        command.stdin.write(b'(')
+        if self.command_prelude:
+            command.stdin.write(encode(self.command_prelude))
         if isinstance(argv, basestring):
             # Run a shell command given as a string
-            command.stdin.write(b'(')
             command.stdin.write(encode(argv))
-            command.stdin.write(b')')
         else:
             # Run a command given as a popen-style list (no shell expansion)
             for arg in argv:
                 command.stdin.write(shell_quote(encode(arg)))
                 command.stdin.write(b' ')
-
+        command.stdin.write(b')')
         command.stdin.write(b'\nexit\n')
         command.stdin.flush()
         command.raiseonerr = raiseonerr
diff --git a/test_pytestmultihost/test_localhost.py b/test_pytestmultihost/test_localhost.py
index 62a3486..01acd92 100644
--- a/test_pytestmultihost/test_localhost.py
+++ b/test_pytestmultihost/test_localhost.py
@@ -8,6 +8,8 @@ from subprocess import CalledProcessError
 import contextlib
 import sys
 import os
+import tempfile
+import shutil
 
 import pytest_multihost
 import pytest_multihost.transport
@@ -117,6 +119,16 @@ def multihost_badpassword(request, transport_class):
     mh.host.transport_class = transport_class
     return mh.install()
 
+@pytest.yield_fixture(scope='class')
+def multihost_with_failing_bash_logout(request, transport_class):
+    mh = multihost(request, transport_class)
+    temp_dir = tempfile.mkdtemp()
+    bash_logout = os.path.join(temp_dir, '.bash_logout')
+    with open(bash_logout, 'w') as f:
+        f.write('false\n')
+    mh.host.command_prelude += b'HOME=%s\n' % temp_dir.encode('utf-8')
+    yield mh
+    shutil.rmtree(temp_dir)
 
 @contextlib.contextmanager
 def _first_command(host):
@@ -330,6 +342,14 @@ class TestLocalhost(object):
             with pytest.raises(CalledProcessError):
                 false.wait()
 
+    @pytest.mark.parametrize('argv_as_list', [True, False])
+    def test_failing_bash_logout(self, multihost_with_failing_bash_logout,
+                                 argv_as_list):
+        argv = 'true'
+        if argv_as_list:
+            argv = [argv]
+        host = multihost_with_failing_bash_logout.host
+        host.run_command(argv)
 
 
 @pytest.mark.needs_ssh