Skip candidate not providing valid metadata #9264
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
uranusjr
force-pushed
the
new-resolver-dont-abort-on-inconsistent-candidate
branch
3 times, most recently
from
ae8d0e0 to
9405d00
Compare
This is done by catching InstallationError from the underlying distribution preparation logic. There are three cases to catch: 1. Candidates from indexes. These are simply ignored since we can potentially satisfy the requirement with other candidates. 2. Candidates from URLs with a dist name (PEP 508 or #egg=). A new UnsatisfiableRequirement class is introduced to represent this; it is like an ExplicitRequirement without an underlying candidate. As the name suggests, an instance of this can never be satisfied, and will cause eventual backtracking. 3. Candidates from URLs without a dist name. This is only possible for top-level user requirements, and no recourse is possible for them. So we error out eagerly. The InstallationError raised during distribution preparation is cached in the factory, like successfully prepared candidates, since we don't want to repeatedly try to build a candidate if we already know it'd fail. Plus pip's preparation logic also does not allow packages to be built multiple times anyway.
uranusjr
force-pushed
the
new-resolver-dont-abort-on-inconsistent-candidate
branch
from
9405d00 to
d45541c
Compare
|
in pip-20.3.2 ? |
6 tasks
pfmoore
approved these changes
Dec 12, 2020
|
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 |
|
Comments added + resolved conflicts with master. |
|
Eh damn it, the XML-RPC tests are acting up again. |
pradyunsg
approved these changes
Dec 12, 2020
|
HMMM... one of the nuances here, that I’m only thinking of after the merge: if I install package that can’t be installed because no wheels are compatible and I’m missing a build dependency or whatever, pip will work through the entire set of versions... that’s not ideal. :( |
|
Hmm... I guess this is OK to release tho, and we can deal with any reactions as they come in. :) |
Of course, technically, an older version might have different build dependencies and work. So that's actually correct behaviour. Not really what the user wants, of course, but technically correct 🙁 We could really do with working out a good heuristic for stopping because we've got to versions that are "too old to be worth considering", but I honestly have no idea how we'd do that. That question's being handled on another issue, though, so it's offtopic here. |
This was referenced Dec 15, 2020
This was referenced Dec 15, 2020
bors bot
referenced
this pull request
in duckinator/emanate
Dec 15, 2020
204: Update pip to 20.3.3 r=duckinator a=pyup-bot This PR updates [pip](https://pypi.org/project/pip) from **20.3.1** to **20.3.3**. <details> <summary>Changelog</summary> ### 20.3.3 ``` =================== Bug Fixes --------- - Revert "Skip candidate not providing valid metadata", as that caused pip to be overeager about downloading from the package index. (`9264 <https://github.com/pypa/pip/issues/9264>`_) ``` ### 20.3.2 ``` =================== Features -------- - New resolver: Resolve direct and pinned (``==`` or ``===``) requirements first to improve resolver performance. (`9185 <https://github.com/pypa/pip/issues/9185>`_) - Add a mechanism to delay resolving certain packages, and use it for setuptools. (`9249 <https://github.com/pypa/pip/issues/9249>`_) Bug Fixes --------- - New resolver: The "Requirement already satisfied" log is not printed only once for each package during resolution. (`9117 <https://github.com/pypa/pip/issues/9117>`_) - Fix crash when logic for redacting authentication information from URLs in ``--help`` is given a list of strings, instead of a single string. (`9191 <https://github.com/pypa/pip/issues/9191>`_) - New resolver: Correctly implement PEP 592. Do not return yanked versions from an index, unless the version range can only be satisfied by yanked candidates. (`9203 <https://github.com/pypa/pip/issues/9203>`_) - New resolver: Make constraints also apply to package variants with extras, so the resolver correctly avoids backtracking on them. (`9232 <https://github.com/pypa/pip/issues/9232>`_) - New resolver: Discard a candidate if it fails to provide metadata from source, or if the provided metadata is inconsistent, instead of quitting outright. (`9246 <https://github.com/pypa/pip/issues/9246>`_) Vendored Libraries ------------------ - Update vendoring to 20.8 Improved Documentation ---------------------- - Update documentation to reflect that pip still uses legacy resolver by default in Python 2 environments. (`9269 <https://github.com/pypa/pip/issues/9269>`_) ``` </details> <details> <summary>Links</summary> - PyPI: https://pypi.org/project/pip - Changelog: https://pyup.io/changelogs/pip/ - Homepage: https://pip.pypa.io/ </details> 205: Update pytest to 6.2.1 r=duckinator a=pyup-bot This PR updates [pytest](https://pypi.org/project/pytest) from **6.1.2** to **6.2.1**. *The bot wasn't able to find a changelog for this release. [Got an idea?](https://github.com/pyupio/changelogs/issues/new)* <details> <summary>Links</summary> - PyPI: https://pypi.org/project/pytest - Homepage: https://docs.pytest.org/en/latest/ </details> Co-authored-by: pyup-bot <[email protected]>
uranusjr
deleted the
new-resolver-dont-abort-on-inconsistent-candidate
branch
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I had a galaxy-brain moment and realised the resolver does not really need to offer additional hooks for this. Since the candidate sequence is already implemented lazily, all we need to do is to check if the build fails during iteration, and skip the candidate when it happens.
Technical details (taken directly from the commit message):
This is done by catching InstallationError from the underlying distribution preparation logic. There are three cases to catch:
#egg=). A new UnsatisfiableRequirement class is introduced to represent this; it is like an ExplicitRequirement without an underlying candidate. As the name suggests, an instance of this can never be satisfied, and will cause eventual backtracking.The InstallationError raised during distribution preparation is cached in the factory, like successfully prepared candidates, since we don't want to repeatedly try to build a candidate if we already know it'd fail. Plus pip's preparation logic also does not allow packages to be built multiple times anyway.
Fix #9203, and also fix #9246
without changingwith minimal change to the underlying build code.