pip wheel should ensure the produced wheels are valid

[stefansjs]

pip v20.3 is able to produce a wheel that is not installable because it raises pip._vendor.packaging.version.InvalidVersion

What did you want to do?

Trying to build a wheel locally is uninstallable because the version is said to be invalid. From my reading of https://www.python.org/dev/peps/pep-0440/#pre-release-separators the version seems valid.

I would expect pip to either raise an exception when building the wheel, or allow the package to be installed.

I think it may have to do with https://github.com/pypa/pip/pull/9085.

Output

(aubio) $ pip install -f file://`pwd` aubio=='0.4.3_libaubio'
Looking in indexes: https://pypi.org/simple, http://my.hosted.simple.index.server/pip
Looking in links: file:///Users/stefansullivan/code/aubio/build
ERROR: Could not find a version that satisfies the requirement aubio==0.4.3_libaubio
ERROR: No matching distribution found for aubio==0.4.3_libaubio

(aubio) $ pip install -f file://`pwd` aubio=='0.4.3-libaubio'
Looking in indexes: https://pypi.org/simple, http://my.hosted.simple.index.server/pip
Looking in links: file:///Users/stefansullivan/code/aubio/build
ERROR: Exception:
Traceback (most recent call last):
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_vendor/resolvelib/resolvers.py", line 171, in _merge_into_criterion
    crit = self.state.criteria[name]
KeyError: 'aubio'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_internal/cli/base_command.py", line 210, in _main
    status = self.run(options, args)
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_internal/cli/req_command.py", line 180, in wrapper
    return func(self, options, args)
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_internal/commands/install.py", line 318, in run
    requirement_set = resolver.resolve(
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_internal/resolution/resolvelib/resolver.py", line 121, in resolve
    self._result = resolver.resolve(
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_vendor/resolvelib/resolvers.py", line 445, in resolve
    state = resolution.resolve(requirements, max_rounds=max_rounds)
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_vendor/resolvelib/resolvers.py", line 310, in resolve
    name, crit = self._merge_into_criterion(r, parent=None)
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_vendor/resolvelib/resolvers.py", line 173, in _merge_into_criterion
    crit = Criterion.from_requirement(self._p, requirement, parent)
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_vendor/resolvelib/resolvers.py", line 82, in from_requirement
    if not cands:
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_vendor/resolvelib/structs.py", line 124, in __bool__
    return bool(self._sequence)
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_internal/resolution/resolvelib/found_candidates.py", line 96, in __bool__
    return any(self)
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_internal/resolution/resolvelib/found_candidates.py", line 20, in _deduplicated_by_version
    for candidate in candidates:
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_internal/resolution/resolvelib/factory.py", line 198, in iter_index_candidates
    yield self._make_candidate_from_link(
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_internal/resolution/resolvelib/factory.py", line 144, in _make_candidate_from_link
    self._link_candidate_cache[link] = LinkCandidate(
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_internal/resolution/resolvelib/candidates.py", line 290, in __init__
    wheel_version = Version(wheel.version)
  File "/Users/stefansullivan/.local/share/virtualenvs/aubio-E3yrlxqC/lib/python3.8/site-packages/pip/_vendor/packaging/version.py", line 298, in __init__
    raise InvalidVersion("Invalid version: '{0}'".format(version))
pip._vendor.packaging.version.InvalidVersion: Invalid version: '0.4.3-libaubio'

Additional information

pip version: 20.3 python version: 3.8, although I think the bug exists in multiple versions

steps to reproduce are roughly:

git clone https://github.com/aubio/aubio
cd aubio
git checkout 0.4.3
pipenv
pip install --upgrade pip==20.3
mkdir build
cd build
pip wheel ..
# Both of the following will produce the errors above
pip install -f file://`pwd` aubio==0.4.3_libaubio
pip install -f file://`pwd` aubio==0.4.3-libaubio

[brainwane]

Hello and thank you for your bug report! I'm sorry you're having trouble right now. Thank you for sharing your report with us.

(If you don't mind, please also tell us what could have happened differently so you could have tested and caught and reported this during the pip resolver beta period.)

Are you using Python 2.7 or 3.8? You mentioned "2.8" which (as far as I know) does not exist. (If you are using Python 2.x: Please upgrade to Python 3; pip will drop Python 2 support in January.)

[stefansjs]

Sorry, yes it is python3.8. I updated the original message so that nobody else gets confused.

Regarding beta, I'm honestly not sure. I'm not too familiar with the current testing/release procedures for pip. It seems like some test cases could be implemented to test some of the stranger PEP440 version strings. Or a requirements.txt/pipfile with some known odd version strings.

In this case it was a simple round-trip of pip wheel ... followed by pip install ... that demonstrates the issue.

It's unclear to me if pypi would have rejected this wheel or not, but for a local wheel cache and/or simple index server it would be easy to make this mistake.

It also looks like this PR https://github.com/pypa/pip/pull/9085 is probably where the regression comes from. It's honestly such a straight-forward improvement that it's the type of thing that nobody would reasonably anticipated. My guess is that using the same implementation in more places will at least prevent the local wheel cache issue. I don't know if it will break backwards-compatibility with wheels already in pypi, let alone all the private index servers out there.

So this may be 2 discussion happening in parallel. How should packaging.Version handle this particular version string? And how should pip handle this version when building the wheel?

[uranusjr]

There are two parts to this issue. The first is that pip wheel can happily produce invalid wheels that fail subsequent installation. The failure during installation is the same as #9188.

I’m modifying the title to more focus on the first part, since the second is already tracked elsewhere.

---

The implementation of pip wheel essentially calls the build backend (PEP 517, or legacy setuptools) to build a wheel for it. So the root cause is in the build backend; they should not produce invalid wheels, especially in PEP 517 mode. But we cannot possibly guarantee to users all backends would get things right, so pip wheel likely should implement some verification to ensure the produced wheels is actually valid, and tell users they should report it to the build backend.

[uranusjr]

Quoting https://github.com/pypa/pip/pull/9199#issuecomment-737883775

[…] 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.

Assuming #9199 will be a part of 20.3.x, I’d propose adding

• A deprecation message in wheel-building logic in the same release that contains the PR. After a wheel is built, check its metadata and file name to make sure both version fields are compatible with PEP 440. If not, emit a deprecation warning saying that non-PEP 440 versions in wheels may break in near future, and the user should contact the build backend to fix this. (TODO: How do we tell the name of the build backend a package is using?)
• The same deprecation message in the new resolver, when a downloaded wheel has a non-PEP 440 version in the file name. We can’t check the metadata in this case, but since the new resolver also ensures the metadata matches file name later in the process, having non-PEP 440 version there would make resolution fail anyway.

[pradyunsg]

• If not, emit a deprecation warning saying that non-PEP 440 versions in wheels may break in near future, and the user should contact the build backend to fix this.

It's probably easier to say "reach out to the package maintainers"?

[stefansjs]

I'm not sure about pip's coding standard, but I'm definitely a fan of fail early, fail often. Personally I would vote for an actual error, not a warning, if an invalid wheel is produced by pip wheel. It would be really nice-to-have a verification step before the wheel is output.

I totally understand that you can't guarantee the implementation of all build backends, but in this case I'm building using setuptools and wheel, which are the default build backend. Should I file a bug with a different project? It does kinda feel like the default backend provided with pip should work out of the box. Not sure if I'm misunderstanding delegation of responsibilities here.

[stefansjs]

The failure during installation is the same as #9188

Is it? My understanding is that 0.4.3-libaubio is a valid PEP 440 version with a post-release qualifier. At least, PEP 440 is a bit vague about whether or not arbitrary suffixes are allowed. If it is valid, then the packaging.Version validation is being too strict.

[pradyunsg]

I'm not sure about pip's coding standard, but I'm definitely a fan of fail early, fail often. Personally I would vote for an actual error, not a warning, if an invalid wheel is produced by pip wheel.

I think that's where we'd want to be. However, we also want the users who are generating invalid wheels today to be able to have time to transition, and informing them via warnings is one of the better mechanisms we have.

[jwodder]

The failure during installation is the same as #9188

Is it? My understanding is that 0.4.3-libaubio is a valid PEP 440 version with a post-release qualifier. At least, PEP 440 is a bit vague about whether or not arbitrary suffixes are allowed. If it is valid, then the packaging.Version validation is being too strict.

"-libaubio" is not a valid post-release segment. The spec states "The post-release segment consists of the string .post, followed by a non-negative integer value." Later it allows a hyphen as an alternate spelling of .post, but the "non-negative integer" restriction remains. The only part of a PEP 440 version that can contain arbitrary letters (i.e., not restricted to numbers) is the local version identifier, which starts with +, though note that locally-versioned assets aren't allowed to be uploaded to PyPI.