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

Update test suite to use virtualenv >= 20.0.0 #11288

Merged
merged 10 commits into from
Oct 27, 2022

Conversation

uranusjr
Copy link
Member

@uranusjr uranusjr commented Jul 22, 2022

Let’s restart #7718, we must do this before 3.11 is out.

This is mostly #8441 re-applied to main with some very minor changes.

@uranusjr uranusjr added the skip news Does not need a NEWS file entry (eg: trivial changes) label Jul 22, 2022
@uranusjr uranusjr force-pushed the virtualenv-20-try2 branch 3 times, most recently from 5aa8f00 to 6601dd5 Compare July 22, 2022 07:25
@pradyunsg
Copy link
Member

pradyunsg commented Jul 22, 2022

FWIW, I've been looking into this as well (https://github.com/pradyunsg/pip/tree/improve-test-isolation), although I'm trying there to move us off of virtualenv entirely and that involves getting the venv-based setup to be at-parity with the existing virtualenv-based setup (which it's not).

I've still not gotten all the tests to pass locally though, so I didn't file a PR for that yet. 😅

@uranusjr
Copy link
Member Author

My current impression is if we manage to get new virtualenv to work, we should be very close with replacing it altogether with venv. The main blocker to virtualenv>=20 is how PEP 503 controls global site-packages access and sitecustomize differently, and if we manage to fix that, most of the venv incompatibilities automatically goes away.

@pradyunsg
Copy link
Member

pradyunsg commented Jul 22, 2022

Yea, that's my understanding as well. I figured that it's not that big of a hop to try complete removal in the first shot -- the problems that need to be solved are roughly the same and it removed the conditionals for which "venv backend" we're using, which I was found were making it difficult for me to keep track of what's happening in the code.

The thing that I last struggled with is that the code was still picking up the sitecustomize from the base environment -- thanks to Homebrew having one -- which borked my Homebrew installation of Python and, I ended up stopping after fixing that mess for the third time and stopped at some point while trying to understand where it was being added in site.py and how to prevent that from happening again (i.e. not leaking global site-customise into the isolated test environment). 😅

Don't enable user site in template venv

IIRC, a bunch of tests rely on the virtualenv having user-site visible -- which isn't a problem since we have the isolate fixture which makes the user's home directory point to a temp directory, during the pip execution. I think I hit this as part of working on that branch as well. 😅

@pfmoore
Copy link
Member

pfmoore commented Jul 22, 2022

PEP 503

Typo? Sorry, I couldn't work out what the correct PEP number was.

@pradyunsg
Copy link
Member

pradyunsg commented Jul 22, 2022

I'm gonna guess that was supposed to be https://peps.python.org/pep-0405/ (for pyvenv.cfg).

@sbidoul sbidoul added this to the 22.2.1 milestone Jul 23, 2022
@sbidoul sbidoul added the type: maintenance Related to Development and Maintenance Processes label Jul 23, 2022
@sbidoul
Copy link
Member

sbidoul commented Jul 26, 2022

@uranusjr @pradyunsg By reading the above and looking at the number of failing tests it seems you have a good sense of what needs to be done and have made good progress.

With the RM hat on, I'm trying to get a feeling of when I'll push 22.2.1 out and if it should wait for this in order to have more assurance that everything works fine on Python 3.11.

So no pressure at all, of course, but if you have hints to share about your plans regarding this topic that'll help me.

@uranusjr
Copy link
Member Author

uranusjr commented Jul 27, 2022

Don’t let this block the release. This doesn’t need to be tied to one since the changes should be only in the test suite (aside from trivial renames).

@sbidoul sbidoul removed this from the 22.2.1 milestone Jul 27, 2022
@pradyunsg
Copy link
Member

I missed this mention, but yea, this doesn’t block a bug fix. It’s more a logistical item needed for testing 3.11 and for reducing long term pain in this space.

@uranusjr
Copy link
Member Author

uranusjr commented Oct 27, 2022

The main changes are made to accomodate bad interactions between distutils and the new virtualenv hacks. Since old virtualenv only does not work in modern Python versions, I decided to only use new virtualenv with Python 3.10 and later and leave the old test setups as-is. A few tests also have the same distutils issues due to relying on setup.py install, and are rewritten to use wheels instead.

@uranusjr uranusjr marked this pull request as ready for review October 27, 2022 03:27
@uranusjr uranusjr marked this pull request as draft October 27, 2022 12:04
@uranusjr
Copy link
Member Author

uranusjr commented Oct 27, 2022

I think this actually works now; the only failure is the submodule one.

The modern virtual environment structure does not allow us to enable "fake user site" while disabling the global site, so we need to do more fine-grained configuration to correctly set up test environments for each test case.

With this done, we can also properly support the stdlib venv ad the test environment backend, since it basically works identically with modern virtualenv. The incompatible_with_test_venv marker is thus removed.

@uranusjr uranusjr marked this pull request as ready for review October 27, 2022 16:09
uranusjr and others added 10 commits October 28, 2022 01:51
Well they are. At least not "regular" anymore.
Co-Authored-By: Lumir Balhar <lbalhar@redhat.com>
pip uses distutils (instead of sysconfig) for Python < 3.10, which has
awkward path issues when faking a user site.
The old INITools tests rely on setup.py, which relies on distutils and
generates a ton of issues. Build fake wheels directly to avoid dealing
with them.
The modern virtual environment structure does not allow us to enable
"fake user site" while disabling the global site, so we need to do more
fine-grained configuration to correctly set up test environments for
each test case.

With this done, we can also properly support the stdlib venv ad the test
environment backend, since it basically works identically with modern
virtualenv. The incompatible_with_test_venv is thus removed.
@uranusjr uranusjr merged commit d0d5b42 into pypa:main Oct 27, 2022
@uranusjr uranusjr deleted the virtualenv-20-try2 branch October 27, 2022 18:26
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
skip news Does not need a NEWS file entry (eg: trivial changes) type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants