make sure we check for write access before starting the install, and add correct exit code

4 files changed, 91 insertions, 16 deletions

diff --git a/Lib/packaging/install.py b/Lib/packaging/install.py
index 9f94532..4b82088 100644
--- a/Lib/packaging/install.py
+++ b/Lib/packaging/install.py
@@ -6,7 +6,6 @@ obtained from an index (e.g. PyPI), with dependencies.
 This is a higher-level module built on packaging.database and
 packaging.pypi.
 """
-
 import os
 import sys
 import stat
@@ -14,7 +13,7 @@ import errno
 import shutil
 import logging
 import tempfile
-from sysconfig import get_config_var
+from sysconfig import get_config_var, get_path
 
 from packaging import logger
 from packaging.dist import Distribution
@@ -28,6 +27,8 @@ from packaging.depgraph import generate_graph
 from packaging.errors import (PackagingError, InstallationException,
                               InstallationConflict, CCompilerError)
 from packaging.pypi.errors import ProjectNotFound, ReleaseNotFound
+from packaging import database
+
 
 __all__ = ['install_dists', 'install_from_infos', 'get_infos', 'remove',
            'install', 'install_local_project']
@@ -75,6 +76,7 @@ def _run_distutils_install(path):
 def _run_setuptools_install(path):
     cmd = '%s setup.py install --record=%s --single-version-externally-managed'
     record_file = os.path.join(path, 'RECORD')
+
     os.system(cmd % (sys.executable, record_file))
     if not os.path.exists(record_file):
         raise ValueError('failed to install')
@@ -88,8 +90,10 @@ def _run_packaging_install(path):
     dist.parse_config_files()
     try:
         dist.run_command('install_dist')
+        name = dist.metadata['name']
+        return database.get_distribution(name) is not None
     except (IOError, os.error, PackagingError, CCompilerError) as msg:
-        raise SystemExit("error: " + str(msg))
+        raise ValueError("Failed to install, " + str(msg))
 
 
 def _install_dist(dist, path):
@@ -115,18 +119,20 @@ def install_local_project(path):
     If the source directory contains a setup.py install using distutils1.
     If a setup.cfg is found, install using the install_dist command.
 
+    Returns True on success, False on Failure.
     """
     path = os.path.abspath(path)
     if os.path.isdir(path):
         logger.info('Installing from source directory: %s', path)
-        _run_install_from_dir(path)
+        return _run_install_from_dir(path)
     elif _is_archive_file(path):
         logger.info('Installing from archive: %s', path)
         _unpacked_dir = tempfile.mkdtemp()
         shutil.unpack_archive(path, _unpacked_dir)
-        _run_install_from_archive(_unpacked_dir)
+        return _run_install_from_archive(_unpacked_dir)
     else:
         logger.warning('No projects to install.')
+        return False
 
 
 def _run_install_from_archive(source_dir):
@@ -152,7 +158,13 @@ def _run_install_from_dir(source_dir):
     func = install_methods[install_method]
     try:
         func = install_methods[install_method]
-        return func(source_dir)
+        try:
+            func(source_dir)
+            return True
+        except ValueError as err:
+            # failed to install
+            logger.info(str(err))
+            return False
     finally:
         os.chdir(old_dir)
 
@@ -472,16 +484,34 @@ def remove(project_name, paths=sys.path, auto_confirm=True):
 
 
 def install(project):
+    """Installs a project.
+
+    Returns True on success, False on failure
+    """
+    logger.info('Checking the installation location...')
+    purelib_path = get_path('purelib')
+    # trying to write a file there
+    try:
+        with tempfile.NamedTemporaryFile(suffix=project,
+                                         dir=purelib_path) as testfile:
+            testfile.write(b'test')
+    except OSError:
+        # was unable to write a file
+        logger.info('Unable to write in "%s". Do you have the permissions ?'
+                    % purelib_path)
+        return False
+
+
     logger.info('Getting information about %r...', project)
     try:
         info = get_infos(project)
     except InstallationException:
         logger.info('Cound not find %r', project)
-        return
+        return False
 
     if info['install'] == []:
         logger.info('Nothing to install')
-        return
+        return False
 
     install_path = get_config_var('base')
     try:
@@ -493,6 +523,8 @@ def install(project):
             projects = ['%s %s' % (p.name, p.version) for p in e.args[0]]
             logger.info('%r conflicts with %s', project, ','.join(projects))
 
+    return True
+
 
 def _main(**attrs):
     if 'script_args' not in attrs:
diff --git a/Lib/packaging/run.py b/Lib/packaging/run.py
index 6e4816d..1a246c5 100644
--- a/Lib/packaging/run.py
+++ b/Lib/packaging/run.py
@@ -225,16 +225,22 @@ def _install(dispatcher, args, **kw):
         if 'setup.py' in listing or 'setup.cfg' in listing:
             args.insert(1, os.getcwd())
         else:
-            logger.warning('no project to install')
-            return
+            logger.warning('No project to install.')
+            return 1
 
     target = args[1]
     # installing from a source dir or archive file?
     if os.path.isdir(target) or _is_archive_file(target):
-        install_local_project(target)
+        if install_local_project(target):
+            return 0
+        else:
+            return 1
     else:
         # download from PyPI
-        install(target)
+        if install(target):
+            return 0
+        else:
+            return 1
 
 
 @action_help(metadata_usage)
diff --git a/Lib/packaging/tests/test_install.py b/Lib/packaging/tests/test_install.py
index c0924bf..c50a45e 100644
--- a/Lib/packaging/tests/test_install.py
+++ b/Lib/packaging/tests/test_install.py
@@ -1,11 +1,10 @@
 """Tests for the packaging.install module."""
-
 import os
 from tempfile import mkstemp
+
 from packaging import install
 from packaging.pypi.xmlrpc import Client
 from packaging.metadata import Metadata
-
 from packaging.tests.support import (LoggingCatcher, TempdirManager, unittest,
                                      fake_dec)
 try:
@@ -340,7 +339,7 @@ class TestInstall(LoggingCatcher, TempdirManager, unittest.TestCase):
                     self.assertTrue(os.path.exists(f))
                 dist._unlink_installed_files()
         finally:
-            install.install_dist = old_install_dist
+            install._install_dist = old_install_dist
             install.uninstall = old_uninstall
 
     def test_install_from_infos_install_succes(self):
@@ -357,6 +356,21 @@ class TestInstall(LoggingCatcher, TempdirManager, unittest.TestCase):
         finally:
             install._install_dist = old_install_dist
 
+    def test_install_permission_denied(self):
+        # if we don't have the access to the installation
+        # path, we should abort immediatly
+        project = os.path.join(os.path.dirname(__file__), 'package.tgz')
+        install_path = self.mkdtemp()
+        old_get_path = install.get_path
+        install.get_path = lambda path: install_path
+        old_mod = os.stat(install_path).st_mode
+        os.chmod(install_path, 0)
+        try:
+            self.assertFalse(install.install(project))
+        finally:
+            os.chmod(install_path, old_mod)
+            install.get_path = old_get_path
+
 
 def test_suite():
     suite = unittest.TestSuite()
diff --git a/Lib/packaging/tests/test_run.py b/Lib/packaging/tests/test_run.py
index 01fa5aa..cb576b7 100644
--- a/Lib/packaging/tests/test_run.py
+++ b/Lib/packaging/tests/test_run.py
@@ -3,8 +3,12 @@
 import os
 import sys
 import shutil
+from tempfile import mkstemp
+from io import StringIO
 
+from packaging import install
 from packaging.tests import unittest, support, TESTFN
+from packaging.run import main
 
 # setup script that uses __file__
 setup_using___file__ = """\
@@ -25,7 +29,8 @@ setup()
 """
 
 
-class CoreTestCase(unittest.TestCase):
+class CoreTestCase(support.TempdirManager, support.LoggingCatcher,
+                   unittest.TestCase):
 
     def setUp(self):
         super(CoreTestCase, self).setUp()
@@ -54,6 +59,24 @@ class CoreTestCase(unittest.TestCase):
 
     # TODO restore the tests removed six months ago and port them to pysetup
 
+    def test_install(self):
+        # making sure install returns 0 or 1 exit codes
+        project = os.path.join(os.path.dirname(__file__), 'package.tgz')
+        install_path = self.mkdtemp()
+        old_get_path = install.get_path
+        install.get_path = lambda path: install_path
+        old_mod = os.stat(install_path).st_mode
+        os.chmod(install_path, 0)
+        old_stderr = sys.stderr
+        sys.stderr = StringIO()
+        try:
+            self.assertFalse(install.install(project))
+            self.assertEqual(main(['install', 'blabla']), 1)
+        finally:
+            sys.stderr = old_stderr
+            os.chmod(install_path, old_mod)
+            install.get_path = old_get_path
+
 
 def test_suite():
     return unittest.makeSuite(CoreTestCase)