Merge pull request pypa#6154 from cjerdonek/show-stdout-default

Change call_subprocess()'s show_stdout default from True to False

Author: pradyunsg

src/pip/_internal/build_env.py

@@ -192,7 +192,7 @@ def install_requirements(
         args.append('--')
         args.extend(requirements)
         with open_spinner(message) as spinner:
-            call_subprocess(args, show_stdout=False, spinner=spinner)
+            call_subprocess(args, spinner=spinner)
 
 
 class NoOpBuildEnvironment(BuildEnvironment):

src/pip/_internal/download.py

@@ -795,7 +795,7 @@ def _copy_dist_from_dir(link_path, location):
     logger.info('Running setup.py sdist for %s', link_path)
 
     with indent_log():
-        call_subprocess(sdist_args, cwd=link_path, show_stdout=False)
+        call_subprocess(sdist_args, cwd=link_path)
 
     # unpack sdist into `location`
     sdist = os.path.join(location, os.listdir(location)[0])

src/pip/_internal/req/req_install.py

@@ -507,7 +507,6 @@ def runner(cmd, cwd=None, extra_environ=None):
                         cmd,
                         cwd=cwd,
                         extra_environ=extra_environ,
-                        show_stdout=False,
                         spinner=spinner
                     )
                 self.spin_message = ""
@@ -605,7 +604,6 @@ def run_egg_info(self):
             call_subprocess(
                 egg_info_cmd + egg_base_option,
                 cwd=self.setup_py_dir,
-                show_stdout=False,
                 command_desc='python setup.py egg_info')
 
     @property
@@ -757,7 +755,6 @@ def install_editable(
                     list(install_options),
 
                     cwd=self.setup_py_dir,
-                    show_stdout=False,
                 )
 
         self.install_succeeded = True
@@ -941,7 +938,6 @@ def install(
                         call_subprocess(
                             install_args + install_options,
                             cwd=self.setup_py_dir,
-                            show_stdout=False,
                             spinner=spinner,
                         )
 

src/pip/_internal/utils/misc.py

@@ -650,7 +650,7 @@ def unpack_file(
 
 def call_subprocess(
     cmd,  # type: List[str]
-    show_stdout=True,  # type: bool
+    show_stdout=False,  # type: bool
     cwd=None,  # type: Optional[str]
     on_returncode='raise',  # type: str
     extra_ok_returncodes=None,  # type: Optional[Iterable[int]]
@@ -677,13 +677,13 @@ def call_subprocess(
     #
     # The obvious thing that affects output is the show_stdout=
     # kwarg. show_stdout=True means, let the subprocess write directly to our
-    # stdout. Even though it is nominally the default, it is almost never used
+    # stdout. It is almost never used
     # inside pip (and should not be used in new code without a very good
     # reason); as of 2016-02-22 it is only used in a few places inside the VCS
     # wrapper code. Ideally we should get rid of it entirely, because it
     # creates a lot of complexity here for a rarely used feature.
     #
-    # Most places in pip set show_stdout=False. What this means is:
+    # Most places in pip use show_stdout=False. What this means is:
     # - We connect the child stdout to a pipe, which we read.
     # - By default, we hide the output but show a spinner -- unless the
     #   subprocess exits with an error, in which case we show the output.

src/pip/_internal/wheel.py

@@ -953,7 +953,7 @@ def _build_one_legacy(self, req, tempd, python_tag=None):
 
             try:
                 output = call_subprocess(wheel_args, cwd=req.setup_py_dir,
-                                         show_stdout=False, spinner=spinner)
+                                         spinner=spinner)
             except Exception:
                 spinner.finish("error")
                 logger.error('Failed building wheel for %s', req.name)
@@ -974,7 +974,7 @@ def _clean_one(self, req):
         logger.info('Running setup.py clean for %s', req.name)
         clean_args = base_args + ['clean', '--all']
         try:
-            call_subprocess(clean_args, cwd=req.source_dir, show_stdout=False)
+            call_subprocess(clean_args, cwd=req.source_dir)
             return True
         except Exception:
             logger.error('Failed cleaning build dir for %s', req.name)

tests/unit/test_utils.py

@@ -724,16 +724,27 @@ def test_get_prog(self, monkeypatch, argv, executable, expected):
         assert get_prog() == expected
 
 
-def test_call_subprocess_works_okay_when_just_given_nothing():
-    try:
-        call_subprocess([sys.executable, '-c', 'print("Hello")'])
-    except Exception:
-        assert False, "Expected subprocess call to succeed"
+def test_call_subprocess_works__no_keyword_arguments():
+    result = call_subprocess(
+        [sys.executable, '-c', 'print("Hello")'],
+    )
+    assert result.rstrip() == 'Hello'
+
+
+def test_call_subprocess_works__show_stdout_true():
+    result = call_subprocess(
+        [sys.executable, '-c', 'print("Hello")'],
+        show_stdout=True,
+    )
+    assert result is None
 
 
 def test_call_subprocess_closes_stdin():
     with pytest.raises(InstallationError):
-        call_subprocess([sys.executable, '-c', 'input()'])
+        call_subprocess(
+            [sys.executable, '-c', 'input()'],
+            show_stdout=True,
+        )
 
 
 @pytest.mark.parametrize('args, expected', [