classification
Title: ensurepip discards pip's return code which leads to broken venvs
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Igor Filatov, Mariatta, Martin Vielsmaier, dstufft, ncoghlan
Priority: normal Keywords: patch

Created on 2017-09-05 20:11 by Igor Filatov, last changed 2017-09-25 06:33 by Igor Filatov. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 3626 merged Igor Filatov, 2017-09-17 12:14
PR 3683 merged python-dev, 2017-09-21 10:07
PR 3734 merged Igor Filatov, 2017-09-24 19:54
Messages (11)
msg301367 - (view) Author: Igor Filatov (Igor Filatov) * Date: 2017-09-05 20:11
ensurepip 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 directory

There 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.
msg302594 - (view) Author: Martin Vielsmaier (Martin Vielsmaier) Date: 2017-09-20 05:47
I guess this is also the root cause for the problem I reported on virtualenv: https://github.com/pypa/virtualenv/issues/1074
msg302666 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-21 00:45
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+
msg302679 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-21 10:07
New changeset 9adda0cdf89432386b7a04444a6199b580d287a1 by Nick Coghlan (Igor Filatov) in branch 'master':
bpo-31351: Set return code in ensurepip when pip fails (GH-3626)
https://github.com/python/cpython/commit/9adda0cdf89432386b7a04444a6199b580d287a1
msg302744 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017-09-22 13:45
New changeset eef49f5dd021d15396551880cf451042a79a1107 by Mariatta (Miss Islington (bot)) in branch '3.6':
bpo-31351: Set return code in ensurepip when pip fails (GH-3626) (GH-3683)
https://github.com/python/cpython/commit/eef49f5dd021d15396551880cf451042a79a1107
msg302745 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017-09-22 13:57
This has been backported to 3.6. Is backport to 2.7 needed?
msg302845 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-24 10:37
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.
msg302879 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2017-09-24 18:45
Thanks :) 
Since Miss Islington couldn't backport this, I'll leave it to the patch author or the core dev who merged the PR ;)
msg302900 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-25 01:03
New changeset cf7197ae43767c4346864e5b41246f628edd9b51 by Nick Coghlan (Igor Filatov) in branch '2.7':
[2.7] bpo-31351: Set return code in ensurepip when pip fails (GH-3734)
https://github.com/python/cpython/commit/cf7197ae43767c4346864e5b41246f628edd9b51
msg302901 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-09-25 01:04
And done - thanks for the report and PRs, Igor!
msg302913 - (view) Author: Igor Filatov (Igor Filatov) * Date: 2017-09-25 06:33
Thanks for the reviews, Nick!
History
Date User Action Args
2017-09-25 06:33:17Igor Filatovsetmessages: + msg302913
2017-09-25 01:04:52ncoghlansetstatus: open -> closed
resolution: fixed
messages: + msg302901

stage: patch review -> resolved
2017-09-25 01:03:27ncoghlansetmessages: + msg302900
2017-09-24 19:54:21Igor Filatovsetstage: backport needed -> patch review
pull_requests: + pull_request3720
2017-09-24 18:45:35Mariattasetmessages: + msg302879
2017-09-24 10:37:07ncoghlansetstage: patch review -> backport needed
messages: + msg302845
versions: - Python 3.6, Python 3.7
2017-09-22 13:57:14Mariattasetmessages: + msg302745
2017-09-22 13:45:40Mariattasetnosy: + Mariatta
messages: + msg302744
2017-09-21 10:07:56python-devsetstage: commit review -> patch review
pull_requests: + pull_request3672
2017-09-21 10:07:47ncoghlansetmessages: + msg302679
2017-09-21 00:45:46ncoghlansetstage: patch review -> commit review
messages: + msg302666
versions: + Python 2.7, - Python 3.4, Python 3.5
2017-09-20 15:27:41ned.deilysetnosy: + ncoghlan, dstufft
2017-09-20 05:47:51Martin Vielsmaiersetnosy: + Martin Vielsmaier
messages: + msg302594
2017-09-17 12:14:19Igor Filatovsetkeywords: + patch
stage: patch review
pull_requests: + pull_request3615
2017-09-05 20:11:57Igor Filatovcreate