Use subprocess.run() to properly capture return codes. (#1075)

* Use subprocess.run() to properly capture return codes.
Fixes #1074

* replaced capture_output flag - only available on 3.7

* Ensure the backported CalledProcessError is imported.

* Fix env tests on windows.

* Format and comment fixes

poetry.lock

@@ -731,6 +731,14 @@ python-versions = ">=2.6, !=3.0.*, !=3.1.*"
 version = "1.12.0"
 
 [[package]]
+category = "main"
+description = "A backport of the subprocess module from Python 3 for use on 2.x."
+name = "subprocess32"
+optional = false
+python-versions = ">=2.6, !=3.0.*, !=3.1.*, !=3.2.*, <4"
+version = "3.5.3"
+
+[[package]]
 category = "dev"
 description = "ANSII Color formatting for output in terminal."
 name = "termcolor"
@@ -917,6 +925,7 @@ requests-toolbelt = ["42c9c170abc2cacb78b8ab23ac957945c7716249206f90874651971a4a
 scandir = ["2586c94e907d99617887daed6c1d102b5ca28f1085f90446554abf1faf73123e", "2ae41f43797ca0c11591c0c35f2f5875fa99f8797cb1a1fd440497ec0ae4b022", "2b8e3888b11abb2217a32af0766bc06b65cc4a928d8727828ee68af5a967fa6f", "2c712840c2e2ee8dfaf36034080108d30060d759c7b73a01a52251cc8989f11f", "4d4631f6062e658e9007ab3149a9b914f3548cb38bfb021c64f39a025ce578ae", "67f15b6f83e6507fdc6fca22fedf6ef8b334b399ca27c6b568cbfaa82a364173", "7d2d7a06a252764061a020407b997dd036f7bd6a175a5ba2b345f0a357f0b3f4", "8c5922863e44ffc00c5c693190648daa6d15e7c1207ed02d6f46a8dcc2869d32", "92c85ac42f41ffdc35b6da57ed991575bdbe69db895507af88b9f499b701c188", "b24086f2375c4a094a6b51e78b4cf7ca16c721dcee2eddd7aa6494b42d6d519d", "cb925555f43060a1745d0a321cca94bcea927c50114b623d73179189a4e100ac"]
 shellingham = ["77d37a4fd287c1e663006f7ecf1b9deca9ad492d0082587bd813c44eb49e4e62", "985b23bbd1feae47ca6a6365eacd314d93d95a8a16f8f346945074c28fe6f3e0"]
 six = ["3350809f0555b11f552448330d0b52d5f24c91a322ea4a15ef22629740f3761c", "d16a0141ec1a18405cd4ce8b4613101da75da0e9a7aec5bdd4fa804d0e0eba73"]
+subprocess32 = ["24a7f627ef7a5695138601b665057ad131fa26e80d49d5ffa6b4fdb2357a80d3", "6bc82992316eef3ccff319b5033809801c0c3372709c5f6985299c88ac7225c3"]
 termcolor = ["1d6d69ce66211143803fbc56652b41d73b4a400a2891d7bf7a1cdf4c02de613b"]
 toml = ["229f81c57791a41d65e399fc06bf0848bab550a9dfd5ed66df18ce5f05e73d5c", "235682dd292d5899d361a811df37e04a8828a5b1da3115886b73cf81ebc9100e", "f1db651f9657708513243e61e6cc67d101a39bad662eaa9b5546f789338e07a3"]
 tomlkit = ["d6506342615d051bc961f70bfcfa3d29b6616cc08a3ddfd4bc24196f16fd4ec2", "f077456d35303e7908cc233b340f71e0bec96f63429997f38ca9272b7d64029e"]

poetry/utils/_compat.py

@@ -46,6 +46,13 @@ if PY35:
 else:
     from pathlib2 import Path
 
+if PY35:
+    import subprocess as subprocess
+    from subprocess import CalledProcessError
+
+else:
+    import subprocess32 as subprocess
+    from subprocess32 import CalledProcessError
 
 if not PY36:
     from collections import OrderedDict

poetry/utils/env.py

@@ -8,7 +8,6 @@ import sysconfig
 import warnings
 
 from contextlib import contextmanager
-from subprocess import CalledProcessError
 from typing import Any
 from typing import Dict
 from typing import Optional
@@ -21,6 +20,8 @@ from poetry.utils._compat import Path
 from poetry.utils._compat import decode
 from poetry.utils._compat import encode
 from poetry.utils._compat import list_to_shell_command
+from poetry.utils._compat import subprocess
+from poetry.utils._compat import CalledProcessError
 from poetry.version.markers import BaseMarker
 
 
@@ -91,11 +92,14 @@ class EnvError(Exception):
 
 
 class EnvCommandError(EnvError):
-    def __init__(self, e):  # type: (CalledProcessError) -> None
-        message = "Command {} errored with the following output: \n{}".format(
-            e.cmd, decode(e.output)
-        )
+    def __init__(self, e, input=None):  # type: (CalledProcessError) -> None
+        self.e = e
 
+        message = "Command {} errored with the following return code {}, and output: \n{}".format(
+            e.cmd, e.returncode, decode(e.output)
+        )
+        if input:
+            message += "input was : {}".format(input)
         super(EnvCommandError, self).__init__(message)
 
 
@@ -361,20 +365,19 @@ class Env(object):
 
         if shell:
             cmd = list_to_shell_command(cmd)
-
         try:
             if self._is_windows:
                 kwargs["shell"] = True
 
             if input_:
-                p = subprocess.Popen(
+                output = subprocess.run(
                     cmd,
-                    stdin=subprocess.PIPE,
                     stdout=subprocess.PIPE,
                     stderr=subprocess.STDOUT,
+                    input=encode(input_),
+                    check=True,
                     **kwargs
-                )
-                output = p.communicate(encode(input_))[0]
+                ).stdout
             elif call:
                 return subprocess.call(cmd, stderr=subprocess.STDOUT, **kwargs)
             else:
@@ -382,7 +385,7 @@ class Env(object):
                     cmd, stderr=subprocess.STDOUT, **kwargs
                 )
         except CalledProcessError as e:
-            raise EnvCommandError(e)
+            raise EnvCommandError(e, input=input_)
 
         return decode(output)
 

pyproject.toml

@@ -47,6 +47,8 @@ glob2 = { version = "^0.6", python = "~2.7 || ~3.4" }
 virtualenv = { version = "^16.0", python = "~2.7" }
 # functools32 is needed for Python 2.7
 functools32 = { version = "^3.2.3", python = "~2.7" }
+# Use subprocess32 for Python 2.7 and 3.4
+subprocess32 = { version = "^3.5", python = "~2.7 || ~3.4" }
 
 [tool.poetry.dev-dependencies]
 pytest = "^4.1"

tests/utils/test_env.py

 import os
+import pytest
+import shutil
 
 from poetry.utils._compat import Path
 from poetry.utils.env import Env
 from poetry.utils.env import VirtualEnv
+from poetry.utils.env import EnvCommandError
+
+MINIMAL_SCRIPT = """\
+
+print("Minimal Output"),
+"""
+
+# Script expected to fail.
+ERRORING_SCRIPT = """\
+import nullpackage
+
+print("nullpackage loaded"),
+"""
 
 
 def test_virtualenvs_with_spaces_in_their_path_work_as_expected(tmp_dir):
@@ -25,14 +40,22 @@ def test_env_get_in_project_venv(tmp_dir, environ):
 
     assert venv.path == Path(tmp_dir) / ".venv"
 
+    shutil.rmtree(str(venv.path))
 
-def test_env_has_symlinks_on_nix(tmp_dir):
-    venv_path = Path(tmp_dir) / "Virtual Env"
+
+@pytest.fixture
+def tmp_venv(tmp_dir, request):
+    venv_path = Path(tmp_dir) / "venv"
 
     Env.build_venv(str(venv_path))
 
     venv = VirtualEnv(venv_path)
+    yield venv
+
+    shutil.rmtree(str(venv.path))
+
 
+def test_env_has_symlinks_on_nix(tmp_dir, tmp_venv):
     venv_available = False
     try:
         from venv import EnvBuilder
@@ -42,4 +65,20 @@ def test_env_has_symlinks_on_nix(tmp_dir):
         pass
 
     if os.name != "nt" and venv_available:
-        assert os.path.islink(venv.python)
+        assert os.path.islink(tmp_venv.python)
+
+
+def test_run_with_input(tmp_dir, tmp_venv):
+    result = tmp_venv.run("python", "-", input_=MINIMAL_SCRIPT)
+
+    assert result == "Minimal Output" + os.linesep
+
+
+def test_run_with_input_non_zero_return(tmp_dir, tmp_venv):
+
+    with pytest.raises(EnvCommandError) as processError:
+
+        # Test command that will return non-zero returncode.
+        result = tmp_venv.run("python", "-", input_=ERRORING_SCRIPT)
+
+    assert processError.value.e.returncode == 1