Skip to content

Commit

Permalink
Merge pull request #1162 from clarete/develop
Browse files Browse the repository at this point in the history
Cmds {install,wheel} should return proper error code
  • Loading branch information
qwcode committed Aug 27, 2013
2 parents 1bc31ec + 9584608 commit ecec569
Show file tree
Hide file tree
Showing 7 changed files with 42 additions and 10 deletions.
11 changes: 9 additions & 2 deletions pip/basecommand.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
from pip.log import logger
from pip.download import urlopen
from pip.exceptions import (BadCommand, InstallationError, UninstallationError,
CommandError)
CommandError, PreviousBuildDirError)
from pip.backwardcompat import StringIO
from pip.baseparser import ConfigOptionParser, UpdatingDefaultsHelpFormatter
from pip.status_codes import SUCCESS, ERROR, UNKNOWN_ERROR, VIRTUALENV_NOT_FOUND
from pip.status_codes import (SUCCESS, ERROR, UNKNOWN_ERROR, VIRTUALENV_NOT_FOUND,
PREVIOUS_BUILD_DIR_ERROR)
from pip.util import get_prog


Expand Down Expand Up @@ -137,6 +138,12 @@ def main(self, args, initial_options):
# and when it is done, isinstance is not needed anymore
if isinstance(status, int):
exit = status
except PreviousBuildDirError:
e = sys.exc_info()[1]
logger.fatal(str(e))
logger.info('Exception information:\n%s' % format_exc())
store_log = True
exit = PREVIOUS_BUILD_DIR_ERROR
except (InstallationError, UninstallationError):
e = sys.exc_info()[1]
logger.fatal(str(e))
Expand Down
3 changes: 2 additions & 1 deletion pip/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,8 @@ def run(self, options, args):
requirement_set.create_bundle(self.bundle_filename)
logger.notify('Created bundle in %s' % self.bundle_filename)
except PreviousBuildDirError:
return
options.no_clean = True
raise
finally:
# Clean up
if (not options.no_clean) and ((not options.no_install) or options.download_dir):
Expand Down
4 changes: 2 additions & 2 deletions pip/commands/wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ def run(self, options, args):
)
wb.build()
except PreviousBuildDirError:
return
options.no_clean = True
raise
finally:
if not options.no_clean:
requirement_set.cleanup_files()

7 changes: 2 additions & 5 deletions pip/req.py
Original file line number Diff line number Diff line change
Expand Up @@ -1067,16 +1067,13 @@ def prepare_files(self, finder, force_root_egg_info=False, bundle=False):
# inconsistencies are logged later, but do not fail the
# installation.
elif os.path.exists(os.path.join(location, 'setup.py')):
msg = textwrap.dedent("""
raise PreviousBuildDirError(textwrap.dedent("""
pip can't proceed with 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(msg)
raise e
""" % (req_to_install, location)))
else:
## FIXME: this won't upgrade when there's an existing package unpacked in `location`
if req_to_install.url is None:
Expand Down
1 change: 1 addition & 0 deletions pip/status_codes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
ERROR = 1
UNKNOWN_ERROR = 2
VIRTUALENV_NOT_FOUND = 3
PREVIOUS_BUILD_DIR_ERROR = 4
NO_MATCHES_FOUND = 23
5 changes: 5 additions & 0 deletions tests/functional/test_install_cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from tests.lib import tests_data, reset_env, find_links
from tests.lib.local_repos import local_checkout
from tests.lib.path import Path
from pip.locations import write_delete_marker_file
from pip.status_codes import PREVIOUS_BUILD_DIR_ERROR


def test_cleanup_after_install():
Expand Down Expand Up @@ -135,7 +137,10 @@ def test_cleanup_prevented_upon_build_dir_exception():
script = reset_env()
build = script.venv_path/'build'/'simple'
os.makedirs(build)
write_delete_marker_file(script.venv_path/'build')
build.join("setup.py").write("#")
result = script.pip('install', '-f', find_links, '--no-index', 'simple', expect_error=True)

assert result.returncode == PREVIOUS_BUILD_DIR_ERROR
assert "pip can't proceed" in result.stdout, result.stdout
assert exists(build)
21 changes: 21 additions & 0 deletions tests/functional/test_wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

from pip import wheel
from pip.download import path_to_url as path_to_url_d
from pip.locations import write_delete_marker_file
from pip.status_codes import PREVIOUS_BUILD_DIR_ERROR
from tests.lib import tests_data, reset_env, pyversion_nodot, path_to_url, find_links


Expand Down Expand Up @@ -96,3 +98,22 @@ def test_pip_wheel_source_deps():
wheel_file_path = script.scratch/'wheelhouse'/wheel_file_name
assert wheel_file_path in result.files_created, result.stdout
assert "Successfully built source" in result.stdout, result.stdout


def test_pip_wheel_fail_cause_of_previous_build_dir():
"""Test when 'pip wheel' tries to install a package that has a previous build directory"""

script = reset_env()
script.pip_install_local('wheel')

# Given that I have a previous build dir of the `simple` package
build = script.venv_path / 'build' / 'simple'
os.makedirs(build)
write_delete_marker_file(script.venv_path / 'build')
build.join('setup.py').write('#')

# When I call pip trying to install things again
result = script.pip('wheel', '--no-index', '--find-links=%s' % find_links, 'simple==3.0', expect_error=True)

# Then I see that the error code is the right one
assert result.returncode == PREVIOUS_BUILD_DIR_ERROR

0 comments on commit ecec569

Please sign in to comment.