diff --git a/pip/commands/install.py b/pip/commands/install.py index d1792bd0a2d..3f53720c885 100644 --- a/pip/commands/install.py +++ b/pip/commands/install.py @@ -144,6 +144,12 @@ def __init__(self, *args, **kw): default=False, help="Include pre-release and development versions. By default, pip only finds stable versions.") + cmd_opts.add_option( + '--no-clean', + action='store_true', + default=False, + help="Don't delete build directories after installs or errors.") + index_opts = cmdoptions.make_option_group(cmdoptions.index_group, self.parser) self.parser.insert_option_group(0, index_opts) @@ -230,28 +236,31 @@ def run(self, options, args): raise InstallationError('--user --editable not supported with setuptools, use distribute') - if not options.no_download: - requirement_set.prepare_files(finder, force_root_egg_info=self.bundle, bundle=self.bundle) - else: - requirement_set.locate_files() - - if not options.no_install and not self.bundle: - requirement_set.install(install_options, global_options, root=options.root_path) - installed = ' '.join([req.name for req in - requirement_set.successfully_installed]) - if installed: - logger.notify('Successfully installed %s' % installed) - elif not self.bundle: - downloaded = ' '.join([req.name for req in - requirement_set.successfully_downloaded]) - if downloaded: - logger.notify('Successfully downloaded %s' % downloaded) - elif self.bundle: - requirement_set.create_bundle(self.bundle_filename) - logger.notify('Created bundle in %s' % self.bundle_filename) - # Clean up - if not options.no_install or options.download_dir: - requirement_set.cleanup_files(bundle=self.bundle) + try: + if not options.no_download: + requirement_set.prepare_files(finder, force_root_egg_info=self.bundle, bundle=self.bundle) + else: + requirement_set.locate_files() + + if not options.no_install and not self.bundle: + requirement_set.install(install_options, global_options, root=options.root_path) + installed = ' '.join([req.name for req in + requirement_set.successfully_installed]) + if installed: + logger.notify('Successfully installed %s' % installed) + elif not self.bundle: + downloaded = ' '.join([req.name for req in + requirement_set.successfully_downloaded]) + if downloaded: + logger.notify('Successfully downloaded %s' % downloaded) + elif self.bundle: + requirement_set.create_bundle(self.bundle_filename) + logger.notify('Created bundle in %s' % self.bundle_filename) + finally: + # Clean up + if (not options.no_clean) and ((not options.no_install) or options.download_dir): + requirement_set.cleanup_files(bundle=self.bundle) + if options.target_dir: if not os.path.exists(options.target_dir): os.makedirs(options.target_dir) diff --git a/pip/exceptions.py b/pip/exceptions.py index 02032550aa1..4c1ac09851f 100644 --- a/pip/exceptions.py +++ b/pip/exceptions.py @@ -29,3 +29,8 @@ class BadCommand(PipError): class CommandError(PipError): """Raised when there is an error in command-line arguments""" + + +class PreviousBuildDirError(PipError): + """Raised when there's a previous conflicting build directory""" + diff --git a/pip/req.py b/pip/req.py index e23744b236f..31a33319ba1 100644 --- a/pip/req.py +++ b/pip/req.py @@ -6,13 +6,14 @@ import sys import shutil import tempfile +import textwrap import zipfile from distutils.util import change_root from pip.locations import bin_py, running_under_virtualenv from pip.exceptions import (InstallationError, UninstallationError, BestVersionAlreadyInstalled, - DistributionNotFound, CommandError) + DistributionNotFound, PreviousBuildDirError) from pip.vcs import vcs from pip.log import logger from pip.util import (display_path, rmtree, ask, ask_path_exists, backup_dir, @@ -1045,11 +1046,23 @@ def prepare_files(self, finder, force_root_egg_info=False, bundle=False): # NB: This call can result in the creation of a temporary build directory location = req_to_install.build_location(self.build_dir, not self.is_download) - - ## FIXME: is the existance of the checkout good enough to use it? I don't think so. unpack = True url = None - if not os.path.exists(os.path.join(location, 'setup.py')): + + # If a checkout exists, it's unwise to keep going. + # Version inconsistencies are logged later, but do not fail the installation. + if os.path.exists(os.path.join(location, 'setup.py')): + msg = textwrap.dedent(""" + pip can't install requirement '%s' due to a pre-existing build directory. + location: %s + This is likely due to a previous installation that failed. + pip is being responsible and not assuming it can delete this. + Please delete it and try again. + """ % (req_to_install, location)) + e = PreviousBuildDirError(msg) + logger.fatal(e) + raise e + else: ## FIXME: this won't upgrade when there's an existing package unpacked in `location` if req_to_install.url is None: if not_found: diff --git a/tests/packages/README.txt b/tests/packages/README.txt index ec3a83c4185..69af18ef310 100644 --- a/tests/packages/README.txt +++ b/tests/packages/README.txt @@ -13,6 +13,10 @@ install). If any earlier step would fail (i.e. egg-info-generation), the already-installed version would never be uninstalled, so uninstall-rollback would not come into play. +brokenegginfo-0.1.tar.gz +------------------------ +crafted to fail on egg_info + BrokenEmitsUTF8 --------------- for generating unicode error in py3.x diff --git a/tests/packages/brokenegginfo-0.1.tar.gz b/tests/packages/brokenegginfo-0.1.tar.gz new file mode 100644 index 00000000000..e0486065d9c Binary files /dev/null and b/tests/packages/brokenegginfo-0.1.tar.gz differ diff --git a/tests/test_cleanup.py b/tests/test_cleanup.py index 5ec52b8f4dd..21965c1306b 100644 --- a/tests/test_cleanup.py +++ b/tests/test_cleanup.py @@ -1,24 +1,33 @@ import os import textwrap from os.path import abspath, exists, join -from tests.test_pip import (here, reset_env, run_pip, write_file, mkdir) +from tests.test_pip import (here, reset_env, run_pip, + write_file, mkdir, path_to_url) from tests.local_repos import local_checkout from tests.path import Path +find_links = path_to_url(os.path.join(here, 'packages')) -def test_cleanup_after_install_from_pypi(): +def test_cleanup_after_install(): """ - Test clean up after installing a package from PyPI. - + Test clean up after installing a package. """ env = reset_env() - run_pip('install', 'INITools==0.2', expect_error=True) - build = env.scratch_path/"build" - src = env.scratch_path/"src" + run_pip('install', '--no-index', '--find-links=%s' % find_links, 'simple') + build = env.venv_path/"build" + src = env.venv_path/"src" assert not exists(build), "build/ dir still exists: %s" % build assert not exists(src), "unexpected src/ dir exists: %s" % src env.assert_no_temp() +def test_no_clean_option_blocks_cleaning(): + """ + Test --no-clean option blocks cleaning. + """ + env = reset_env() + result = run_pip('install', '--no-clean', '--no-index', '--find-links=%s' % find_links, 'simple') + build = env.venv_path/'build'/'simple' + assert exists(build), "build/simple should still exist %s" % str(result) def test_cleanup_after_install_editable_from_hg(): """ @@ -135,3 +144,25 @@ def test_download_should_not_delete_existing_build_dir(): assert os.path.exists(env.venv_path/'build'), "build/ should be left if it exists before pip run" assert content == 'I am not empty!', "it should not affect build/ and its content" assert ['somefile.txt'] == os.listdir(env.venv_path/'build') + +def test_cleanup_after_install_exception(): + """ + Test clean up after a 'setup.py install' exception. + """ + env = reset_env() + #broken==0.2broken fails during install; see packages readme file + result = run_pip('install', '-f', find_links, '--no-index', 'broken==0.2broken', expect_error=True) + build = env.venv_path/'build' + assert not exists(build), "build/ dir still exists: %s" % result.stdout + env.assert_no_temp() + +def test_cleanup_after_egg_info_exception(): + """ + Test clean up after a 'setup.py egg_info' exception. + """ + env = reset_env() + #brokenegginfo fails during egg_info; see packages readme file + result = run_pip('install', '-f', find_links, '--no-index', 'brokenegginfo==0.1', expect_error=True) + build = env.venv_path/'build' + assert not exists(build), "build/ dir still exists: %s" % result.stdout + env.assert_no_temp() diff --git a/tests/test_req.py b/tests/test_req.py new file mode 100644 index 00000000000..357fa15a8ae --- /dev/null +++ b/tests/test_req.py @@ -0,0 +1,45 @@ +import os +import tempfile +import shutil + +from pip.exceptions import PreviousBuildDirError +from pip.index import PackageFinder +from pip.req import InstallRequirement, RequirementSet +from tests.test_pip import here, path_to_url, assert_raises_regexp + +find_links = path_to_url(os.path.join(here, 'packages')) + +class TestRequirementSet(object): + """RequirementSet tests""" + + def setup(self): + self.tempdir = tempfile.mkdtemp() + + def teardown(self): + shutil.rmtree(self.tempdir, ignore_errors=True) + + def basic_reqset(self): + return RequirementSet( + build_dir=os.path.join(self.tempdir, 'build'), + src_dir=os.path.join(self.tempdir, 'src'), + download_dir=None, + download_cache=os.path.join(self.tempdir, 'download_cache'), + ) + + def test_no_reuse_existing_build_dir(self): + """Test prepare_files raise exception with previous build dir""" + + build_dir = os.path.join(self.tempdir, 'build', 'simple') + os.makedirs(build_dir) + open(os.path.join(build_dir, "setup.py"), 'w') + reqset = self.basic_reqset() + req = InstallRequirement.from_line('simple') + reqset.add_requirement(req) + finder = PackageFinder([find_links], []) + assert_raises_regexp( + PreviousBuildDirError, + "pip can't install[\s\S]*%s[\s\S]*%s" % (req, build_dir), + reqset.prepare_files, + finder + ) + diff --git a/tests/test_user_site.py b/tests/test_user_site.py index bdaf4c5325a..0cdf26a6e0f 100644 --- a/tests/test_user_site.py +++ b/tests/test_user_site.py @@ -206,8 +206,8 @@ def test_install_user_in_global_virtualenv_with_conflict_fails(self): result2 = run_pip('install', '--user', 'INITools==0.1', expect_error=True) resultp = env.run('python', '-c', "import pkg_resources; print(pkg_resources.get_distribution('initools').location)") dist_location = resultp.stdout.strip() - assert result2.stdout.startswith("Will not install to the user site because it will lack sys.path precedence to %s in %s" - %('INITools', dist_location)), result2.stdout + assert "Will not install to the user site because it will lack sys.path precedence to %s in %s" \ + % ('INITools', dist_location) in result2.stdout, result2.stdout def test_uninstall_from_usersite(self):