Add support for Pip 22.2.2. #1893
Conversation
jsirois
force-pushed
the
issues/1805/final
branch
4 times, most recently
from
7e178a9
to
ad96cad
Compare
Pex can now support multiple Pip versions and be configured to fallback as needed when newer Pip does not support a specified interpreter. Fixes pex-tool#1805
jsirois
force-pushed
the
issues/1805/final
branch
from
ad96cad
to
0c7feed
Compare
|
Lots of plumbing in this one that I found no obvious way to break up. I'll add pointers to the non-plumbing bits of interest. |
| @@ -201,6 +211,20 @@ def analyze(self, line): | |||
| self._saved.add(pin) | |||
| return self.Continue() | |||
|
|
|||
| match = re.search( | |||
There was a problem hiding this comment.
This is the new PEP-691 interaction needed for Pip 22.2.2 since it no longer logs hashes along with URLs. The vendored Pip never triggers this code path because it does not log this "Fetched page ..." line.
| @@ -154,6 +154,11 @@ def supported_platform_tag(self, *_args, **_kwargs): | |||
| check(*args, **kwargs) for check in supported_checks | |||
| ) | |||
|
|
|||
| # N.B.: This patch is a noop for the 20.3.4-patched Pip but is required in newer Pip. | |||
There was a problem hiding this comment.
This was the only change actually required to get Pip 22.2.2 working. The PEP-691 work was just needed to make locking comparably fast with vendored Pip (without PEP-691 each artifact is downloaded just so it can be hashed locally using 22.2.2).
|
A final apology in advance. This is largish and sprawling. The plumbing tendrils for the new pip-version go pretty wide and deep. |
| pytest -n auto tests/integration {posargs:-vvs} | ||
| setenv = |
There was a problem hiding this comment.
A lesson we already know over in Pants. Users may use the build tool but they probably don't understand it. When I took up work on Pex after Wickman / Wilson I did not know tox. I still do not know tox as well as I should. I did enough to get by. I now, however, understand the tox "generative names" concept better and realized this was all un-needed dup. The _printenv command added though helped me be sure of this.
| @@ -22,54 +22,38 @@ jobs: | |||
| - name: Noop | |||
| run: "true" | |||
| checks: | |||
| name: TOXENV=${{ matrix.tox-env }} | |||
| name: TOXENV=format-check,typecheck,vendor-check,package -- --additional-format sdist --additional-format wheel | |||
There was a problem hiding this comment.
Just helping clear space. All Combined this is ~5 minutes whereas the fastest test shards are 8. With the 3 job headspace cleared, the overall job count went from 23 -> 27. Previously 4 mac shards, now 5.
|
And I'll respond to any feedback from Windows. Wish me luck. |
| max_parallel_jobs = attr.ib(default=None) # type: Optional[int] | ||
| pip = attr.ib(init=False) # type: Pip | ||
|
|
||
| @pip.default |
There was a problem hiding this comment.
Took me a little spelunking of the attrs docs to grok this, and particularly the use of the partially-initialized self, but it's neat!
There was a problem hiding this comment.
Yes, attrs is quite a bit nicer than dataclasses.
| @@ -258,24 +269,25 @@ def resolve_from_lock( | |||
| # errors under the right interleaving of processes and threads and download artifact targets. | |||
| file_lock_style = FileLockStyle.BSD | |||
|
|
|||
| # We eagerly initialize a Pip tool for reasons alluded to above, creation of the Pip tool venv | |||
There was a problem hiding this comment.
I don't quite grok why this is no longer required as a result of this change. Perhaps it wasn't required even before this change? AFAICT the pip venv now gets created lazily in the ArtifactDownloader.pip factory, as it would have before?
There was a problem hiding this comment.
Exactly, it was not required before. I was attempting to substitute clarity when what i really want is some more sane way to deal with this. The only requirement is that the Pip venv is built before the thread pool is used below and that was satisfied either by passing in a pre-built Pip in the obvious way here or via the default / factory used in ArtifactDownloader. What this is all hammering on about is that ArtifactDownloader used to just use get_pip() lazily in-line where it needed it; that was the real problem. The deeper problem is POSIX locks vs BSD. I could switch AtomicDirectory to just always use BSD and then the issue goes away, but as things stand I try to maintain compatibility with pre-existing users of PEX + NFS in all codepaths possible.
Pex can now support multiple Pip versions and be configured to fallback
as needed when newer Pip does not support a specified interpreter.
Fixes #1805