diff --git a/pip/commands/install.py b/pip/commands/install.py index 99947bdff9e..7b496a21a93 100644 --- a/pip/commands/install.py +++ b/pip/commands/install.py @@ -259,28 +259,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() + 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_install or options.download_dir: + requirement_set.cleanup_files(bundle=self.bundle) - 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) if options.target_dir: if not os.path.exists(options.target_dir): os.makedirs(options.target_dir) diff --git a/pip/req.py b/pip/req.py index dbe7af27b7f..bce01838571 100644 --- a/pip/req.py +++ b/pip/req.py @@ -1014,9 +1014,16 @@ 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 build dir exists, it's too risky to keep going + #version inconsistencies are logged later, but do not stop installation + if os.path.exists(os.path.join(location, 'setup.py')): + msg = 'Will not install requirement %s due to pre-existing build dir %s' % (req_to_install, location) + logger.fatal(msg) + raise InstallationError(msg) + if not os.path.exists(os.path.join(location, 'setup.py')): ## FIXME: this won't upgrade when there's an existing package unpacked in `location` if req_to_install.url is None: diff --git a/tests/packages/README.txt b/tests/packages/README.txt index 0d25eb59d91..5924daaf140 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 +------------------------ +This package throws exception during 'setup.py 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_basic.py b/tests/test_basic.py index d5a054b14fb..325eb58901e 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -207,6 +207,23 @@ def test_bad_install_with_no_download(): "an equivalent install with --no-install?" in result.stdout +def test_install_no_reuse_existing_build_dir(): + """ + Test install doesn't reuse existing build dir + """ + find_links = 'file://' + join(here, 'packages') + env = reset_env() + + # do a --no-install install which will create a build dir + result = run_pip('install', '-f', find_links, '--no-index', '--no-install', 'simple==2.0') + build_folder = env.venv/'build'/'simple' + assert build_folder in result.files_created, str(result) + + #install should fail due to pre-existing build dir + result = run_pip('install', '--upgrade', '-f', find_links, '--no-index', 'simple', expect_error=True) + assert 'Will not install requirement simple due to pre-existing build' in result.stdout + + def test_install_dev_version_from_pypi(): """ Test using package==dev. diff --git a/tests/test_cleanup.py b/tests/test_cleanup.py index c57db91ee3d..2800688323e 100644 --- a/tests/test_cleanup.py +++ b/tests/test_cleanup.py @@ -1,7 +1,7 @@ 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, pyversion) from tests.local_repos import local_checkout from tests.path import Path @@ -135,3 +135,28 @@ 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() + find_links = 'file://' + join(here, 'packages') + #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() + find_links = 'file://' + join(here, 'packages') + #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_user_site.py b/tests/test_user_site.py index e4f5c73a8f5..09b956d17f9 100644 --- a/tests/test_user_site.py +++ b/tests/test_user_site.py @@ -220,8 +220,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):