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

Prefer stdlib contextlib over contextlib2 when available #9384

Merged
merged 1 commit into from
Feb 19, 2021
Merged

Prefer stdlib contextlib over contextlib2 when available #9384

merged 1 commit into from
Feb 19, 2021

Conversation

jdufresne
Copy link
Contributor

ExitStack and suppress are available from stdlib contextlib on all
supported Python versions.

contextlib2 remains for the nullcontext context manager, which isn't
available in Python 3.6.

@uranusjr
Copy link
Member

Is nullcontext the only remaining usage of contextlib2? This is trivial to implement, and we can (should?) drop the dependency entirely if that’s the case.

@jdufresne
Copy link
Contributor Author

When I run tox -e vendoring, the file src/pip/_vendor/contextlib2.LICENSE.txt comes back. Is there something else I need to change?

@jdufresne
Copy link
Contributor Author

Thanks for the review! The latest revision removes contextlib2 entirely and adds the suggested compat shim. This is used in testing only, so it is placed in the tests/ directory.

@pradyunsg
Copy link
Member

Is there something else I need to change?

Not as far as I can see.

Is that behaviour still happening for you (I'd be surprised, since CI seems to be happy; and we're running it on CI)?

@jdufresne
Copy link
Contributor Author

Okay, it looks like my /tmp/vendoring-cache/ directory contained stale data. After removing it, the license no longer comes back. Thanks.

@pradyunsg
Copy link
Member

Ah, interesting! I'll investigate this sometime next year -- would you be able to file an issue over in pradyunsg/vendoring for this?

@uranusjr
Copy link
Member

This will need to be rebased to master to fix the test suite for setuptools 52.0.

@uranusjr
Copy link
Member

Rebase needed!

@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Feb 18, 2021
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Feb 19, 2021
@jdufresne
Copy link
Contributor Author

rebased

tests/lib/server.py Outdated Show resolved Hide resolved
Removes the vendored contextlib2.

ExitStack and suppress are available from stdlib contextlib on all
supported Python versions.

The nullcontext context manager which isn't available in Python 3.6, but
the function is simple to implement. Once Python 3.6 support is dropped,
so too can the compat shim.
@uranusjr uranusjr merged commit f3f4ef2 into pypa:master Feb 19, 2021
@jdufresne jdufresne deleted the contextlib branch August 2, 2021 15:15
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants