better UI when the user does not have the perms to remove the project

2 files changed, 48 insertions, 7 deletions

diff --git a/Lib/packaging/install.py b/Lib/packaging/install.py
index cd2bbb6..9f94532 100644
--- a/Lib/packaging/install.py
+++ b/Lib/packaging/install.py
@@ -376,7 +376,10 @@ def _remove_dist(dist, paths=sys.path):
 
 
 def remove(project_name, paths=sys.path, auto_confirm=True):
-    """Removes a single project from the installation"""
+    """Removes a single project from the installation.
+
+    Returns True on success
+    """
     dist = get_distribution(project_name, use_egg_info=True, paths=paths)
     if dist is None:
         raise PackagingError('Distribution "%s" not found' % project_name)
@@ -384,13 +387,26 @@ def remove(project_name, paths=sys.path, auto_confirm=True):
     rmdirs = []
     rmfiles = []
     tmp = tempfile.mkdtemp(prefix=project_name + '-uninstall')
+
+    def _move_file(source, target):
+        try:
+            os.rename(source, target)
+        except OSError as err:
+            return err
+        return None
+
+    success = True
+    error = None
     try:
         for file_, md5, size in files:
             if os.path.isfile(file_):
                 dirname, filename = os.path.split(file_)
                 tmpfile = os.path.join(tmp, filename)
                 try:
-                    os.rename(file_, tmpfile)
+                    error = _move_file(file_, tmpfile)
+                    if error is not None:
+                        success = False
+                        break
                 finally:
                     if not os.path.isfile(file_):
                         os.rename(tmpfile, file_)
@@ -401,6 +417,11 @@ def remove(project_name, paths=sys.path, auto_confirm=True):
     finally:
         shutil.rmtree(tmp)
 
+    if not success:
+        logger.info('%r cannot be removed.', project_name)
+        logger.info('Error: %s' % str(error))
+        return False
+
     logger.info('Removing %r: ', project_name)
 
     for file_ in rmfiles:
@@ -447,6 +468,8 @@ def remove(project_name, paths=sys.path, auto_confirm=True):
         logger.info('Success: removed %d files and %d dirs',
                     file_count, dir_count)
 
+    return True
+
 
 def install(project):
     logger.info('Getting information about %r...', project)
diff --git a/Lib/packaging/tests/test_uninstall.py b/Lib/packaging/tests/test_uninstall.py
index 45ce4ca..4b37286 100644
--- a/Lib/packaging/tests/test_uninstall.py
+++ b/Lib/packaging/tests/test_uninstall.py
@@ -2,6 +2,7 @@
 import os
 import sys
 from io import StringIO
+import stat
 
 from packaging.database import disable_cache, enable_cache
 from packaging.run import main
@@ -82,10 +83,7 @@ class UninstallTestCase(support.TempdirManager,
         os.chdir(dirname)
         old_out = sys.stderr
         sys.stderr = StringIO()
-        try:
-            dist = self.run_setup('install_dist', '--prefix=' + self.root_dir)
-        finally:
-            sys.stderr = old_out
+        dist = self.run_setup('install_dist', '--prefix=' + self.root_dir)
         install_lib = self.get_path(dist, 'purelib')
         return dist, install_lib
 
@@ -99,10 +97,30 @@ class UninstallTestCase(support.TempdirManager,
         self.assertIsFile(install_lib, 'foo', '__init__.py')
         self.assertIsFile(install_lib, 'foo', 'sub', '__init__.py')
         self.assertIsFile(install_lib, 'Foo-0.1.dist-info', 'RECORD')
-        remove('Foo', paths=[install_lib])
+        self.assertTrue(remove('Foo', paths=[install_lib]))
         self.assertIsNotFile(install_lib, 'foo', 'sub', '__init__.py')
         self.assertIsNotFile(install_lib, 'Foo-0.1.dist-info', 'RECORD')
 
+    @unittest.skipIf(sys.platform == 'win32', 'deactivated for now')
+    def test_remove_issue(self):
+        # makes sure if there are OSErrors (like permission denied)
+        # remove() stops and display a clean error
+        dist, install_lib = self.install_dist('Meh')
+
+        # breaking os.rename
+        old = os.rename
+
+        def _rename(source, target):
+            raise OSError()
+
+        os.rename = _rename
+        try:
+            self.assertFalse(remove('Meh', paths=[install_lib]))
+        finally:
+            os.rename = old
+
+        self.assertTrue(remove('Meh', paths=[install_lib]))
+
 
 def test_suite():
     return unittest.makeSuite(UninstallTestCase)