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

pip wheel should ensure the produced wheels are valid #9206

Closed
stefansjs opened this issue Dec 2, 2020 · 9 comments · Fixed by #9320
Closed

pip wheel should ensure the produced wheels are valid #9206

stefansjs opened this issue Dec 2, 2020 · 9 comments · Fixed by #9320
Labels
C: build logic Stuff related to metadata generation / wheel generation C: wheel The wheel format and 'pip wheel' command PEP implementation Involves some PEP
Milestone

Comments

@stefansjs
Copy link

stefansjs commented Dec 2, 2020

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 #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 brainwane added PEP implementation Involves some PEP state: needs eyes Needs a maintainer/triager to take a closer look labels Dec 2, 2020
@brainwane
Copy link
Contributor

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
Copy link
Author

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 #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
Copy link
Member

uranusjr commented Dec 3, 2020

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 uranusjr changed the title pip wheel can produce binaries with versions that pip install doesn't like pip wheel should ensure the produced wheels are valid Dec 3, 2020
@uranusjr uranusjr added C: build logic Stuff related to metadata generation / wheel generation C: wheel The wheel format and 'pip wheel' command labels Dec 3, 2020
@uranusjr
Copy link
Member

uranusjr commented Dec 3, 2020

Quoting #9199 (comment)

[…] 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
Copy link
Member

pradyunsg commented Dec 3, 2020

  • 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
Copy link
Author

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
Copy link
Author

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
Copy link
Member

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
Copy link
Contributor

jwodder commented Dec 3, 2020

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.

@pradyunsg pradyunsg modified the milestones: 20.3.2, 20.3.3 Dec 5, 2020
bors bot referenced this issue in duckinator/emanate Jan 31, 2021
215: Update pip to 21.0.1 r=duckinator a=pyup-bot


This PR updates [pip](https://pypi.org/project/pip) from **20.3.3** to **21.0.1**.



<details>
  <summary>Changelog</summary>
  
  
   ### 21.0.1
   ```
   ===================

Bug Fixes
---------

- commands: debug: Use packaging.version.parse to compare between versions. (`9461 &lt;https://github.com/pypa/pip/issues/9461&gt;`_)
- New resolver: Download and prepare a distribution only at the last possible
  moment to avoid unnecessary network access when the same version is already
  installed locally. (`9516 &lt;https://github.com/pypa/pip/issues/9516&gt;`_)

Vendored Libraries
------------------

- Upgrade packaging to 20.9
   ```
   
  
  
   ### 21.0
   ```
   =================

Deprecations and Removals
-------------------------

- Drop support for Python 2. (`6148 &lt;https://github.com/pypa/pip/issues/6148&gt;`_)
- Remove support for legacy wheel cache entries that were created with pip
  versions older than 20.0. (`7502 &lt;https://github.com/pypa/pip/issues/7502&gt;`_)
- Remove support for VCS pseudo URLs editable requirements. It was emitting
  deprecation warning since version 20.0. (`7554 &lt;https://github.com/pypa/pip/issues/7554&gt;`_)
- Modernise the codebase after Python 2. (`8802 &lt;https://github.com/pypa/pip/issues/8802&gt;`_)
- Drop support for Python 3.5. (`9189 &lt;https://github.com/pypa/pip/issues/9189&gt;`_)
- Remove the VCS export feature that was used only with editable VCS
  requirements and had correctness issues. (`9338 &lt;https://github.com/pypa/pip/issues/9338&gt;`_)

Features
--------

- Add ``--ignore-requires-python`` support to pip download. (`1884 &lt;https://github.com/pypa/pip/issues/1884&gt;`_)
- New resolver: Error message shown when a wheel contains inconsistent metadata
  is made more helpful by including both values from the file name and internal
  metadata. (`9186 &lt;https://github.com/pypa/pip/issues/9186&gt;`_)

Bug Fixes
---------

- Fix a regression that made ``pip wheel`` do a VCS export instead of a VCS clone
  for editable requirements. This broke VCS requirements that need the VCS
  information to build correctly. (`9273 &lt;https://github.com/pypa/pip/issues/9273&gt;`_)
- Fix ``pip download`` of editable VCS requirements that need VCS information
  to build correctly. (`9337 &lt;https://github.com/pypa/pip/issues/9337&gt;`_)

Vendored Libraries
------------------

- Upgrade msgpack to 1.0.2.
- Upgrade requests to 2.25.1.

Improved Documentation
----------------------

- Render the unreleased pip version change notes on the news page in docs. (`9172 &lt;https://github.com/pypa/pip/issues/9172&gt;`_)
- Fix broken email link in docs feedback banners. (`9343 &lt;https://github.com/pypa/pip/issues/9343&gt;`_)


.. note

    You should *NOT* be adding new change log entries to this file, this
    file is managed by towncrier. You *may* edit previous change logs to
    fix problems like typo corrections or such.

    To add a new change log entry, please see
        https://pip.pypa.io/en/latest/development/contributing/#news-entries

.. towncrier release notes start
   ```
   
  
  
   ### 20.3.4
   ```
   ===================

Features
--------

- ``pip wheel`` now verifies the built wheel contains valid metadata, and can be
  installed by a subsequent ``pip install``. This can be disabled with
  ``--no-verify``. (`9206 &lt;https://github.com/pypa/pip/issues/9206&gt;`_)
- Improve presentation of XMLRPC errors in pip search. (`9315 &lt;https://github.com/pypa/pip/issues/9315&gt;`_)

Bug Fixes
---------

- Fixed hanging VCS subprocess calls when the VCS outputs a large amount of data
  on stderr. Restored logging of VCS errors that was inadvertently removed in pip
  20.2. (`8876 &lt;https://github.com/pypa/pip/issues/8876&gt;`_)
- Fix error when an existing incompatibility is unable to be applied to a backtracked state. (`9180 &lt;https://github.com/pypa/pip/issues/9180&gt;`_)
- New resolver: Discard a faulty distribution, instead of quitting outright.
  This implementation is taken from 20.2.2, with a fix that always makes the
  resolver iterate through candidates from indexes lazily, to avoid downloading
  candidates we do not need. (`9203 &lt;https://github.com/pypa/pip/issues/9203&gt;`_)
- New resolver: Discard a source distribution if it fails to generate metadata,
  instead of quitting outright. This implementation is taken from 20.2.2, with a
  fix that always makes the resolver iterate through candidates from indexes
  lazily, to avoid downloading candidates we do not need. (`9246 &lt;https://github.com/pypa/pip/issues/9246&gt;`_)

Vendored Libraries
------------------

- Upgrade resolvelib to 0.5.4.
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pip
  - Changelog: https://pyup.io/changelogs/pip/
  - Homepage: https://pip.pypa.io/
</details>



Co-authored-by: pyup-bot <github-bot@pyup.io>
yxieca pushed a commit to sonic-net/sonic-buildimage that referenced this issue Apr 29, 2021
Why I did it
for 201811 build image, swsssdk wheel package is not getting build due to redis==2.10.6 requirement issue. Notied that after upgrading pip to 20.3.3, we are experiencling this issue.

the issue

21:10:41  /sonic/src/sonic-py-swsssdk /sonic
21:10:41  running test
21:10:41  Searching for redis==2.10.6
21:10:41  Reading https://pypi.python.org/simple/redis/
21:10:41  Couldn't find index page for 'redis' (maybe misspelled?)
21:10:41  Scanning index of all packages (this may take a while)
21:10:41  Reading https://pypi.python.org/simple/
21:10:41  No local packages or download links found for redis==2.10.6
21:10:41  error: Could not find suitable distribution for Requirement.parse('redis==2.10.6')
21:10:41  [  FAIL LOG END  ] [ target/python-wheels/swsssdk-2.0.1-py3-none-any.whl ]
21:10:41  slave.mk:422: recipe for target 'target/python-wheels/swsssdk-2.0.1-py3-none-any.whl' failed
21:10:41  make: *** [target/python-wheels/swsssdk-2.0.1-py3-none-any.whl] Error 1
21:10:43  Makefile.work:132: recipe for target 'target/sonic-aboot-broadcom.swi' failed
21:10:43  make[1]: *** [target/sonic-aboot-broadcom.swi] Error 2
21:10:43  make[1]: Leaving directory '/data/johnar/workspace/broadcom/buildimage-brcm-201811'
21:10:43  Makefile:6: recipe for target 'target/sonic-aboot-broadcom.swi' failed
So, what I have understood till now, if pip v20.3.3 is able to produce a wheel that is not installable because it raises pip._vendor.packaging.version.InvalidVersion or some error like that (resource- > pypa/pip#9206), it just raises an exception when building the wheel.

SO, this issue occurs when we pinned down pip to 20.3.3 in short.

As of now, there are two solutions mentioned below.

pin down pip to 20.3.3(which it is) and explicitly install packages.
pin down pip to 20.3.4(pip wheel now verifies the built wheel contains valid metadata, and can be installed by a subsequent pip install.)(resource-> https://pip.pypa.io/en/stable/news/) -- didn't try yet
How I did it
Install nose explicitly for mockredispy
Install redis==2.10.6 for swsssdk tests.

How to verify it
Run local build after removing all previously built dockers.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 4, 2021
@pradyunsg pradyunsg removed the state: needs eyes Needs a maintainer/triager to take a closer look label Dec 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: build logic Stuff related to metadata generation / wheel generation C: wheel The wheel format and 'pip wheel' command PEP implementation Involves some PEP
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants