Avoid need to quote arguments for commands running without a shell (c…

…onan-io#5578) (conan-io#5583)

* Support running commands specified as a sequences (conan-io#5578).

* Restore previous behaviour of modifying process environment.

* Retain DYLD_LIBRARY_PATH value across invocation of /bin/sh on macOS.

* Fix tests on Python 2.7.

* add test to prove Macos behavior

Co-authored-by: jgsogo <[email protected]>

conans/client/runner.py

@@ -51,7 +51,7 @@ def __call__(self, command, output=True, log_filepath=None, cwd=None, subprocess
             log_filepath = None
 
         # Log the command call in output and logger
-        call_message = "\n----Running------\n> %s\n-----------------\n" % command
+        call_message = "\n----Running------\n> %s\n-----------------\n" % (command,)
         if self._print_commands_to_output and stream_output and self._log_run_to_output:
             stream_output.write(call_message)
 
@@ -62,7 +62,7 @@ def __call__(self, command, output=True, log_filepath=None, cwd=None, subprocess
                 return self._simple_os_call(command, cwd)
             elif log_filepath:
                 if stream_output:
-                    stream_output.write("Logging command output to file '%s'\n" % log_filepath)
+                    stream_output.write("Logging command output to file '%s'\n" % (log_filepath,))
                 with open(log_filepath, "a+") as log_handler:
                     if self._print_commands_to_output:
                         log_handler.write(call_message)
@@ -76,7 +76,7 @@ def _pipe_os_call(self, command, stream_output, log_handler, cwd):
             # piping both stdout, stderr and then later only reading one will hang the process
             # if the other fills the pip. So piping stdout, and redirecting stderr to stdout,
             # so both are merged and use just a single get_stream_lines() call
-            proc = Popen(command, shell=True, stdout=PIPE, stderr=STDOUT, cwd=cwd)
+            proc = Popen(command, shell=isinstance(command, six.string_types), stdout=PIPE, stderr=STDOUT, cwd=cwd)
         except Exception as e:
             raise ConanException("Error while executing '%s'\n\t%s" % (command, str(e)))
 
@@ -108,12 +108,12 @@ def get_stream_lines(the_stream):
 
     def _simple_os_call(self, command, cwd):
         if not cwd:
-            return os.system(command)
+            return subprocess.call(command, shell=isinstance(command, six.string_types))
         else:
             try:
                 old_dir = get_cwd()
                 os.chdir(cwd)
-                result = os.system(command)
+                result = subprocess.call(command, shell=isinstance(command, six.string_types))
             except Exception as e:
                 raise ConanException("Error while executing"
                                      " '%s'\n\t%s" % (command, str(e)))

conans/model/conan_file.py

@@ -1,5 +1,6 @@
 import os
 from contextlib import contextmanager
+from six import string_types
 
 import six
 
@@ -271,7 +272,9 @@ def _run():
             # When using_build_profile the required environment is already applied through 'conanfile.env'
             # in the contextmanager 'get_env_context_manager'
             with tools.run_environment(self) if not self._conan_using_build_profile else no_op():
-                if OSInfo().is_macos:
+                if OSInfo().is_macos and isinstance(command, string_types):
+                    # Security policy on macOS clears this variable when executing /bin/sh. To
+                    # keep its value, set it again inside the shell when running the command.
                     command = 'DYLD_LIBRARY_PATH="%s" DYLD_FRAMEWORK_PATH="%s" %s' % \
                               (os.environ.get('DYLD_LIBRARY_PATH', ''),
                                os.environ.get("DYLD_FRAMEWORK_PATH", ''),

conans/test/functional/environment/run_environment_test.py

@@ -31,7 +31,7 @@ class HelloConan(ConanFile):
                 version = "0.1"
                 build_policy = "missing"
                 requires = "Hello0/0.1@lasote/stable"
-            
+
                 def build(self):
                     run_env = RunEnvironment(self)
                     with tools.environment_append(run_env.vars):
@@ -65,7 +65,7 @@ def setUp(self):
             #else
               #define HELLO_EXPORT
             #endif
-            
+
             HELLO_EXPORT void hello();
         """)
 
@@ -92,7 +92,7 @@ def build(self):
                     cmake = CMake(self)
                     cmake.configure()
                     cmake.build()
-            
+
                 def package(self):
                     self.copy("*say_hello.exe", dst="bin", keep_path=False)
                     self.copy("*say_hello", dst="bin", keep_path=False)
@@ -129,6 +129,39 @@ def build(self):
         client.run("build .")
         self.assertIn("Hello Tool!", client.out)
 
+    @unittest.skipUnless(platform.system() == "Darwin", "Check SIP protection shell=False")
+    def test_command_as_list(self):
+        client = TestClient(servers=self.servers, users={"default": [("lasote", "mypass")]})
+
+        reuse = textwrap.dedent("""
+            from conans import ConanFile
+            class HelloConan(ConanFile):
+                requires = "Pkg/0.1@lasote/testing"
+                options = {"cmd_list": [True, False]}
+                default_options = {"cmd_list": False}
+
+                def build(self):
+                    if self.options.cmd_list:
+                        self.run(["say_hello",], run_environment=True)
+                    else:
+                        self.run("say_hello", run_environment=True)
+        """)
+
+        client.save({"conanfile.py": reuse}, clean_first=True)
+        client.run("config set log.print_run_commands=1")
+
+        # Using a string, a new shell is expanded, DYLD_... paths has to be informed
+        client.run("install . -o cmd_list=False")
+        client.run("build .")
+        self.assertIn("> DYLD_LIBRARY_PATH=", client.out)
+        self.assertIn("Hello Tool!", client.out)
+
+        # Using a list, no new shell, DYLD_... are already in the environment
+        client.run("install . -o cmd_list=True")
+        client.run("build .")
+        self.assertNotIn("DYLD_LIBRARY_PATH", client.out)
+        self.assertIn("Hello Tool!", client.out)
+
     @unittest.skipIf(platform.system() == "Darwin", "SIP protection (read comment)")
     def test_with_tools_run_environment(self):
         # This test is excluded from OSX, because of the SIP protection. CMake helper will