Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
iafilatov mannequin opened this issue Sep 5, 2017 · 11 comments
Closed

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

iafilatov mannequin opened this issue Sep 5, 2017 · 11 comments
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@iafilatov
Copy link
Mannequin

iafilatov mannequin commented 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']

    @iafilatov
    Copy link
    Mannequin Author

    iafilatov mannequin commented Sep 5, 2017

    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.

    @iafilatov iafilatov mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Sep 5, 2017
    @MartinVielsmaier
    Copy link
    Mannequin

    MartinVielsmaier mannequin commented Sep 20, 2017

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

    @ncoghlan
    Copy link
    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+

    @ncoghlan
    Copy link
    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

    @Mariatta
    Copy link
    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

    @Mariatta
    Copy link
    Member

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

    @ncoghlan
    Copy link
    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.

    @ncoghlan ncoghlan removed the 3.7 (EOL) end of life label Sep 24, 2017
    @Mariatta
    Copy link
    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 ;)

    @ncoghlan
    Copy link
    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

    @ncoghlan
    Copy link
    Contributor

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

    @iafilatov
    Copy link
    Mannequin Author

    iafilatov mannequin commented Sep 25, 2017

    Thanks for the reviews, Nick!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants