Allow legacy version in wheel metadata

[uranusjr]

Fix #9188.

I took a look at the code that crashed and realised it was only performing a sanity check (and is arguably not even necessary if you walk through all the code paths that go through it). Although it’s likely a good idea to start requiring PEP 440 versions for wheels, IMO it should not be done in this part, but when the wheel is actually downloaded.

This PR simply makes the check more lax to pass the sanity check. I’ll open another issue to work on logic to implement PEP 440 enforcement if it is accepted as a good idea.

[uranusjr]

The PEP 440 check should be done here instead:

pip/src/pip/_internal/resolution/resolvelib/candidates.py

Lines 204 to 214 in 30eeb9c

	def _check_metadata_consistency(self, dist):
	# type: (Distribution) -> None
	"""Check for consistency of project name and version of dist."""
	# TODO: (Longer term) Rather than abort, reject this candidate
	# and backtrack. This would need resolvelib support.
	name = canonicalize_name(dist.project_name)
	if self._name is not None and self._name != name:
	raise MetadataInconsistent(self._ireq, "name", dist.project_name)
	version = dist.parsed_version
	if self._version is not None and self._version != version:
	raise MetadataInconsistent(self._ireq, "version", dist.version)

[pradyunsg]

Although it’s likely a good idea to start requiring PEP 440 versions for wheels, IMO it should not be done in this part, but when the wheel is actually downloaded.

This PR simply makes the check more lax to pass the sanity check.

Agreed, and on board. :3

I’ll open another issue to work on logic to implement PEP 440 enforcement if it is accepted as a good idea.

From a change management perspective: we've already "broken" users who want the lax/no PEP 440 enforcement, so it'd be better overall to also (re-)implement the PEP 440 strictness w/ better error messaging in the same bugfix release, so we're not flip-flopping on this issue. It has already eaten into our churn budget, so... might as well not pay the churn cost again?

In other words, yes I like the idea, and (puts on RM hat) that change either blocks the merge of this PR or you can include it here (takes off hat).

[uranusjr]

PR updated. Let’s use #9206 to track the deprecation warning addition.

[uranusjr]

This should be ready now #9320 is merged (closing #9206).

[BrownTruck]

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!

[pradyunsg]

Man, I've forgotten all the context around this, but the change does what it says in the title. :)

[pradyunsg]

Wait, if we're enforcing that built wheels follow PEP 440, why do we want to relax pip install now? Especially because packaging is going to be dropping LegacyVersion soon anyway.

See comment above.

[pradyunsg]

Yea, re-reading the issue, I think what we have to do now is improve the error message and to not permit legacy versions in wheels.

[uranusjr]

Technically wheels can use non-PEP-440 version, if the .dist-info it contains uses Metadata 1.0. But we don’t know a wheel’s metadata version just by looking at its filename, so we allow it here. A later check during prepare() will make sure that wheel is actually using Metadata 1.0, and complain if not (and skip the wheel altogether). This makes the check more technically correct (to avoid possible spec police complaining PEP 427 doesn’t actually enforce PEP 440), and also allows us to emit a more accurate error message to the actual issue (which is the more important benefit to me).

[uranusjr]

Although, after checking, it seems like we haven’t applied the verification to downloaded wheels (only built wheels now), so that should happen first.

[pfmoore]

I've also lost any context on this, but honestly, I think we should be enforcing newer standards. I'm fine with saying that the current behaviour was an unintentionally abrupt deprecation (🙂) and "fix" it by allowing it again for a version or two with a deprecation warning, but given that when support for legacy versions is removed is out of our hands because we rely on packaging, I'm also fine with just saying sorry, consider the deprecation of the old resolver as the deprecation period for this as well.

As a broader point, maybe we should make a "statement of direction" in the docs that explicitly confirms that pip (along with the wider packaging ecosystem) is moving towards stricter enforcement of up to date published standards, and as a consequence we will be gradually removing support for "legacy" behaviours. Users who currently rely on legacy behaviours should actively look at updating to follow current standards, and if they decide not to (or can't) must be prepared at some point to pin their version of pip, and understand that they won't be able to take advantage of improvements to pip after that point.

(It probably won't make much practical difference, as people will either not read the note, or assume their usage is OK, or otherwise not do anything about it. But at least we will have done our part by publicising our intent.)

[BrownTruck]

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!

[pradyunsg]

It's been long enough that I don't think we need to do this.

In any case, this PR needs updating anyway; so if there's any interest in this still, the interested folks can file a new PR to move this forward. :)