Merge pull request #114 from jitakirin/improve-resolve-error-handling

Improve error handling for local installs

pipsi/__init__.py

@@ -3,9 +3,28 @@
 import pkgutil
 import sys
 import shutil
+import subprocess
 import glob
+from collections import namedtuple
 from os.path import join, realpath, dirname, normpath, normcase
 from operator import methodcaller
+try:
+    subprocess.run
+
+    def run(*args, **kw):
+        kw.update(stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+        r = subprocess.run(*args, **kw)
+        r.stdout, r.stderr = map(proc_output, (r.stdout, r.stderr))
+        return r
+except AttributeError:  # no `subprocess.run`, py < 3.5
+    CompletedProcess = namedtuple('CompletedProcess',
+                                  ('args', 'returncode', 'stdout', 'stderr'))
+
+    def run(argv, **kw):
+        p = subprocess.Popen(
+            argv, stdout=subprocess.PIPE, stderr=subprocess.PIPE, **kw)
+        out, err = map(proc_output, p.communicate())
+        return CompletedProcess(argv, p.returncode, out, err)
 try:
     from urlparse import urlparse
 except ImportError:
@@ -32,6 +51,13 @@
 )
 
 
+def proc_output(s):
+    s = s.strip()
+    if not isinstance(s, str):
+        s = s.decode('utf-8', 'replace')
+    return s
+
+
 def normalize_package(value):
     # Strips the version and normalizes name
     requirement = Requirement.parse(value)
@@ -50,16 +76,6 @@ def real_readlink(filename):
     return normpath(realpath(join(dirname(filename), target)))
 
 
-def statusoutput(argv, **kw):
-    from subprocess import Popen, PIPE
-    p = Popen(
-        argv, stdout=PIPE, stderr=PIPE, **kw)
-    output = p.communicate()[0].strip()
-    if not isinstance(output, str):
-        output = output.decode('utf-8', 'replace')
-    return p.returncode, output
-
-
 def publish_script(src, dst):
     if IS_WIN:
         # always copy new exe on windows
@@ -86,19 +102,19 @@ def publish_script(src, dst):
 def extract_package_version(virtualenv, package):
     prefix = normalize(join(virtualenv, BIN_DIR, ''))
 
-    return statusoutput([
+    return run([
         join(prefix, 'python'), '-c', GET_VERSION_SCRIPT,
         package,
-    ])[1].strip()
+    ]).stdout.strip()
 
 
 def find_scripts(virtualenv, package):
     prefix = normalize(join(virtualenv, BIN_DIR, ''))
 
-    files = statusoutput([
+    files = run([
         join(prefix, 'python'), '-c', FIND_SCRIPTS_SCRIPT,
         package, prefix
-    ])[1].splitlines()
+    ]).stdout.splitlines()
 
     files = map(normalize, files)
     files = filter(
@@ -159,13 +175,20 @@ def resolve_package(self, spec, python=None):
         else:
             return spec, [spec]
 
-        error, name = statusoutput(
-            [python or sys.executable, 'setup.py', '--name'],
-            cwd=location)
-        if error:
+        if not os.path.exists(join(location, 'setup.py')):
             raise click.UsageError('%s does not appear to be a local '
                                    'Python package.' % spec)
 
+        res = run(
+            [python or sys.executable, 'setup.py', '--name'],
+            cwd=location)
+        if res.returncode:
+            raise click.UsageError(
+                '%s does not appear to be a valid '
+                'package. Error from setup.py: %s' % (spec, res.stderr)
+            )
+        name = res.stdout
+
         return name, [location]
 
     def get_package_path(self, package):

testing/test_repo.py

@@ -1,6 +1,7 @@
 import os
 import sys
 import pytest
+import click
 from pipsi import Repo, find_scripts
 
 
@@ -9,6 +10,42 @@ def repo(home, bin):
     return Repo(str(home), str(bin))
 
 
+@pytest.mark.resolve
+def test_resolve_local_package(repo, tmpdir):
+    pkgdir = tmpdir.ensure('foopkg', dir=True)
+    pkgdir.join('setup.py').write_text(
+        u'\n'.join([
+            u'from setuptools import setup',
+            u'setup(name="foopkg", version="0.0.1", py_modules=["foo"])'
+        ]),
+        'utf-8'
+    )
+    pkgdir.join('foo.py').write_text(u'print("hello world")\n', 'utf-8')
+
+    assert repo.resolve_package(str(pkgdir)) == ('foopkg', [str(pkgdir)])
+
+
+@pytest.mark.resolve
+def test_resolve_local_fails_when_invalid_package(repo, tmpdir):
+    pkgdir = tmpdir.ensure('foopkg', dir=True)
+    pkgdir.join('setup.py').write_text(u'raise Exception("EXCMSG")', 'utf-8')
+    pkgdir.join('foo.py').ensure()
+
+    with pytest.raises(click.UsageError) as excinfo:
+        repo.resolve_package(str(pkgdir))
+    assert 'does not appear to be a valid package' in str(excinfo.value)
+    assert 'EXCMSG' in str(excinfo.value)
+
+
+@pytest.mark.resolve
+def test_resolve_local_fails_when_no_package(repo, tmpdir):
+    pkgdir = tmpdir.ensure('foopkg', dir=True)
+
+    with pytest.raises(click.UsageError) as excinfo:
+        repo.resolve_package(str(pkgdir))
+    assert 'does not appear to be a local Python package' in str(excinfo.value)
+
+
 @pytest.mark.parametrize('package, glob', [
     ('grin', 'grin*'),
     pytest.param('pipsi', 'pipsi*',