Repository navigation
ensurepip discards pip's return code which leads to broken venvs #75532
Description
Activity
iafilatov commented
on Sep 5, 2017 iafilatovmannequinMannequinAuthorensurepip runs pip with a function that discards the result code that pip produces:
def _run_pip(args, additional_paths=None): # ... pip.main(args)
pip.main() is designed not to raise command exceptions. Instead it returns exit codes which are to be passed to sys.exit(). Since ensurepip does not process the return value of pip.main() in any way, python -m ensurepip always exits with 0 on such exceptions (even though there's a stack trace, printing of which is handled by pip). EC should be 0 only in case of success.
This is an issue in venv because it runs ensurepip in a subprocess and cannot detect a failure:
def _setup_pip(self, context): # ... cmd = [context.env_exe, '-Im', 'ensurepip', '--upgrade', '--default-pip'] subprocess.check_output(cmd, stderr=subprocess.STDOUT)
And this leads to broken venvs in some cases. The easiest way to reproduce:
$ mkdir venv-test $ cd venv-test/ $ echo garbage > setup.cfg $ python3 -m venv broken $ broken/bin/pip bash: broken/bin/pip: No such file or directoryThere are no errors until you need pip. The culprit is this:
$ broken/bin/python -Im ensurepip --upgrade --default-pip Ignoring indexes: https://pypi.python.org/simple Collecting setuptools Collecting pip Collecting pkg_resources Installing collected packages: setuptools, pip, pkg-resources Exception: Traceback (most recent call last): File "/tmp/tmpgjpdbf_e/pip-8.1.1-py2.py3-none-any.whl/pip/basecommand.py", line 209, in main status = self.run(options, args) File "/tmp/tmpgjpdbf_e/pip-8.1.1-py2.py3-none-any.whl/pip/commands/install.py", line 335, in run prefix=options.prefix_path, File "/tmp/tmpgjpdbf_e/pip-8.1.1-py2.py3-none-any.whl/pip/req/req_set.py", line 732, in install **kwargs File "/tmp/tmpgjpdbf_e/pip-8.1.1-py2.py3-none-any.whl/pip/req/req_install.py", line 837, in install self.move_wheel_files(self.source_dir, root=root, prefix=prefix) File "/tmp/tmpgjpdbf_e/pip-8.1.1-py2.py3-none-any.whl/pip/req/req_install.py", line 1039, in move_wheel_files isolated=self.isolated, File "/tmp/tmpgjpdbf_e/pip-8.1.1-py2.py3-none-any.whl/pip/wheel.py", line 247, in move_wheel_files prefix=prefix, File "/tmp/tmpgjpdbf_e/pip-8.1.1-py2.py3-none-any.whl/pip/locations.py", line 141, in distutils_scheme d.parse_config_files() File "/usr/lib/python3.5/distutils/dist.py", line 395, in parse_config_files parser.read(filename) File "/usr/lib/python3.5/configparser.py", line 696, in read self._read(fp, filename) File "/usr/lib/python3.5/configparser.py", line 1077, in _read raise MissingSectionHeaderError(fpname, lineno, line) configparser.MissingSectionHeaderError: File contains no section headers. file: 'setup.cfg', line: 1 'garbage\n'
But any other exception during pip any_command would've had the same effect. This is hard to diagnose because no output is produced by subprocess due to the exception.
ensurepip should propagate the code returned by pip and pass it to sys.exit(). Alternatively ensurepip can have it's own EC for cases when pip.main(args) != 0.
MartinVielsmaier commented
on Sep 20, 2017 MartinVielsmaiermannequinMannequinI guess this is also the root cause for the problem I reported on virtualenv: pypa/virtualenv#1074
3.4 and 3.5 are in security fix only mode now, so the change won't be backported there. However, it will be applicable to 2.7, so I've updated the impacted version list accordingly.
Igor's suggested approach in the PR looks reasonable to me as a first step, as it focuses just on the command line interface, and defers the question of how to handle error reporting in the ensurepip.bootstrap() API.
For ensurepip.bootstrap(), I'm then inclined to leave it alone in the maintenance releases, and have it start raising an exception for pip failures in 3.7+
This has been backported to 3.6. Is backport to 2.7 needed?
Aye, it is - while there's no venv integration in 2.7 (since there's no venv module), ensurepip itself is essentially identical across the two version, so 2.7 will suffer from the same problem.
Thanks :)
Since Miss Islington couldn't backport this, I'll leave it to the patch author or the core dev who merged the PR ;)And done - thanks for the report and PRs, Igor!
iafilatov commented
on Sep 25, 2017 iafilatovmannequinMannequinAuthorThanks for the reviews, Nick!
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: