From f47fa58b62e6702f840625bff9fc8a07cef3408f Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Mon, 30 May 2011 23:26:51 +0200 Subject: [PATCH] better UI when the user does not have the perms to remove the project --- Lib/packaging/install.py | 27 ++++++++++++++++++++++++-- Lib/packaging/tests/test_uninstall.py | 28 ++++++++++++++++++++++----- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/Lib/packaging/install.py b/Lib/packaging/install.py index cd2bbb6b6b4..9f945329dc7 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 45ce4ca5f4d..4b372860592 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 @@ def install_dist(self, name='Foo', dirname=None, **kw): 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 @@ def test_uninstall(self): 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)