Optimize upgrade of already-satisfied pinned requirement

[andersk]

Example: after installing six 1.12.0, pip install -Uv six==1.12.0 now returns immediately, instead of going to the index to check for a version that can’t possibly be considered better.

This optimization is most significant when upgrading via a requirements file with many pinned versions and some non-pinned versions.

[andersk]

Updated as requested.

[pradyunsg]

We'd want to add tests for this -- I'm thinking a bit about how that'd work. We'll want to start having unit tests for the resolver, and this functionality might just be the best reason to justify adding it.

[pradyunsg]

@andersk Would you be OK if I pick this PR up from here?

I'll have to figure out how we'd want to unit-test the resolver, in a situation where we're planning to replace it in the future.

[andersk]

Go for it, thanks!

[pradyunsg]

Thank you for the contribution! It's much appreciated!

I'd thought about doing this a while back, but had dropped the ball on this because... "life happened". I doubt I'd have remembered that this can be done unless you'd filed this PR!

[pradyunsg]

I'll merge this in as-is since to the best of my judgement, this works exactly how we want it to. :)

[xavfernandez]

Unfortunately PEP 440 does not allow such optimization.
pip install -U six==1.12.0 should check its indexes and find links location for a possible 1.12.0+foobar version that should be preferred to the currently 1.12.0 installed one.

[pradyunsg]

I'll revert this, to have a broader discussion about this change then. :)

[xavfernandez]

Cf #770, #771, #2114, #2493, #2631 and maybe others ;)

Maybe a new value in --upgrade-strategy could now allow us to trigger this new behavior ?

[pradyunsg]

Reverted in #7158.

>>> from packaging.version import parse
>>> v1 = parse("1.12.0")
>>> v2 = parse("1.12.0+foobar")
>>> v2 == v1
False
>>> v2 > v1
True
>>> from packaging.specifiers import SpecifierSet
>>> spec1 = SpecifierSet("== 1.12.0")
>>> v1 in spec1
True
>>> v2 in spec1
True

[pradyunsg]

and maybe others ;)

Amazing. I'm not the first one to make this mistake then. :P

[pradyunsg]

Maybe a new value in --upgrade-strategy could now allow us to trigger this new behavior ?

Yea, we could. I'm thinking that we may want some way to "ignore local versions from <this source>" and just have pypi.org on that list by default.

[pfmoore]

lol this is the sort of example that makes me think that no matter how simple something in packaging looks, there will always be a complication someone didn't think if :-(

Maybe a new value in --upgrade-strategy

Maybe, but I'm wary of adding yet more choices to the UI and making the user decide. I think we should try to aim for deciding what the right behaviour should be (not that it's easy to do that!), and then implementing that and not making the user choose - this does mean we need to be prepared to defend our choice, of course. I quite like @pradyunsg's idea that "is allowed to host local versions" should be a property of the package source. (Isn't there an issue somewhere that talks about creating a "repository" abstraction which pulls together stuff that's linked to the index like this?)

[pradyunsg]

(Isn't there an issue somewhere that talks about creating a "repository" abstraction which pulls together stuff that's linked to the index like this?)

I am looking for this, I swear I remember us having this discussion.

[pradyunsg]

FWIW, we have a PackageIndex model -- https://github.com/pypa/pip/blob/master/src/pip/_internal/models/index.py

[pradyunsg]

Okay, my GitHub-foo isn't strong enough right now, to find our past discussion on that.

If someone could file a new issue for discussion of this stuff, that'd be great!

[pfmoore]

I'll hunt for it later (maybe tomorrow, I'm busy this evening). I think the key term was "repo", FWIW.

[chrahunt]

If you're thinking about #4263, that's a good one. It could clean things up quite a bit.

[pfmoore]

Yep, that was the one, thanks @chrahunt

[andersk]

Hmm. This optimization should still be allowed in the case where the requirement is === (arbitrary equality), which has no special treatment of local versions, right?

[andersk]

Or if a hash is specified, since a different local version can’t match the same hash?

[pfmoore]

All I'd say is that having found the problem with the original version of this PR (and seeing how many people had made the same mistake previously!) I'd be nervous about making any assumptions here :-)

[andersk]

Well I’d counter that, seeing how many people had made the same mistake previously, there’s clearly some demand for this optimization, so maybe it’s worth putting a bit more careful thought into what we can improve without sacrificing correctness?

[pradyunsg]

maybe it’s worth putting a bit more careful thought into what we can improve without sacrificing correctness?

It definitely is.