clean build dir after errors, and don't reuse a pre-existing build dir

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)

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:

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

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.

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()

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):