Skip to content

ensurepip discards pip's return code which leads to broken venvs #75532

Closed
Closed
ensurepip discards pip's return code which leads to broken venvs#75532
Labels
stdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

Description

@iafilatov
iafilatov
mannequin
opened on Sep 5, 2017
BPO 31351
Nosy @ncoghlan, @dstufft, @Mariatta, @iafilatov
PRs
  • bpo-31351: Fix RC 0 in ensurepip when pip fails #3626
  • [3.6] bpo-31351: Set return code in ensurepip when pip fails (GH-3626) #3683
  • [2.7] bpo-31351: Set return code in ensurepip when pip fails (GH-3626) #3734
    		•

    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:

    assignee = None
closed_at = <Date 2017-09-25.01:04:52.559>
created_at = <Date 2017-09-05.20:11:57.651>
labels = ['type-bug', 'library']
title = "ensurepip discards pip's return code which leads to broken venvs"
updated_at = <Date 2017-09-25.06:33:17.551>
user = 'https://github.com/iafilatov'

    bugs.python.org fields:

    activity = <Date 2017-09-25.06:33:17.551>
actor = 'Igor Filatov'
assignee = 'none'
closed = True
closed_date = <Date 2017-09-25.01:04:52.559>
closer = 'ncoghlan'
components = ['Library (Lib)']
creation = <Date 2017-09-05.20:11:57.651>
creator = 'Igor Filatov'
dependencies = []
files = []
hgrepos = []
issue_num = 31351
keywords = ['patch']
message_count = 11.0
messages = ['301367', '302594', '302666', '302679', '302744', '302745', '302845', '302879', '302900', '302901', '302913']
nosy_count = 5.0
nosy_names = ['ncoghlan', 'dstufft', 'Mariatta', 'Martin Vielsmaier', 'Igor Filatov']
pr_nums = ['3626', '3683', '3734']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue31351'
versions = ['Python 2.7']

    Activity

    1. iafilatov commented on Sep 5, 2017

      @iafilatov
      iafilatovmannequin
      on Sep 5, 2017
      MannequinAuthor

      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.

    2. iafilatov
      added
      3.7 (EOL)end of life
      stdlibStandard Library Python modules in the Lib/ directory
      type-bugAn unexpected behavior, bug, or error
      on Sep 5, 2017

    3. MartinVielsmaier commented on Sep 20, 2017

      @MartinVielsmaier
      MartinVielsmaiermannequin
      on Sep 20, 2017
      Mannequin

      I guess this is also the root cause for the problem I reported on virtualenv: pypa/virtualenv#1074

    4. ncoghlan commented on Sep 21, 2017

      @ncoghlan
      ncoghlan
      on Sep 21, 2017
      Contributor

      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+

    5. ncoghlan commented on Sep 21, 2017

      @ncoghlan
      ncoghlan
      on Sep 21, 2017
      Contributor

      New changeset 9adda0c by Nick Coghlan (Igor Filatov) in branch 'master':
      bpo-31351: Set return code in ensurepip when pip fails (GH-3626)
      9adda0c

    6. Mariatta commented on Sep 22, 2017

      @Mariatta
      Mariatta
      on Sep 22, 2017
      Member

      New changeset eef49f5 by Mariatta (Miss Islington (bot)) in branch '3.6':
      bpo-31351: Set return code in ensurepip when pip fails (GH-3626) (GH-3683)
      eef49f5

    7. Mariatta commented on Sep 22, 2017

      @Mariatta
      Mariatta
      on Sep 22, 2017
      Member

      This has been backported to 3.6. Is backport to 2.7 needed?

    8. ncoghlan commented on Sep 24, 2017

      @ncoghlan
      ncoghlan
      on Sep 24, 2017
      Contributor

      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.

    9. ncoghlan
      removed
      3.7 (EOL)end of life
      on Sep 24, 2017

    10. Mariatta commented on Sep 24, 2017

      @Mariatta
      Mariatta
      on Sep 24, 2017
      Member

      Thanks :)
      Since Miss Islington couldn't backport this, I'll leave it to the patch author or the core dev who merged the PR ;)

    11. ncoghlan commented on Sep 25, 2017

      @ncoghlan
      ncoghlan
      on Sep 25, 2017
      Contributor

      New changeset cf7197a by Nick Coghlan (Igor Filatov) in branch '2.7':
      [2.7] bpo-31351: Set return code in ensurepip when pip fails (GH-3734)
      cf7197a

    12. ncoghlan commented on Sep 25, 2017

      @ncoghlan
      ncoghlan
      on Sep 25, 2017
      Contributor

      And done - thanks for the report and PRs, Igor!

    13. ncoghlan
      closed this as completedon Sep 25, 2017

    14. iafilatov commented on Sep 25, 2017

      @iafilatov
      iafilatovmannequin
      on Sep 25, 2017
      MannequinAuthor

      Thanks for the reviews, Nick!

    15. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      stdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions