Check for --complete-platforms match when --resolve-local-platforms

[huonw]

This makes --resolve-local-platforms work better with --complete-platforms: if there's an external complete platform specified for a platform that matches a local interpreter, the local interpreter's complete platform is checked against that external complete platform. If the local interpreter supports any additional tags, the local interpreter is not used.

If the local interpreter/environment is newer (supports more tags), the old behaviour would risk including wheels that aren't compatible with the external complete platform. That is, execution will fail, see pantsbuild/pants#17621 for an example.

Fixes #1899

[jsirois]

Thanks for working on this @huonw.

I'm not fully convinced, but I think subset is the right choice still. The only reason to use --resolve-local-platforms at all is to make more resolves succeed by bringing sdists in play. If you want an "optimal" resolve and you have an environment that matches the target host, then you don't use --platform or --complete-patform, you just use --python to say this exact one right here - we have our ducks in a row and this matches production.

Maybe Pants does need to grow knobs as I think you suggested in another thread. Right now it tries to do the right thing with 0 knobs for the lambda case.

[jsirois]

This language is not correct, I think, whichever way the subset vs exact match decision goes. At least it contradicts the PyPA thinking. In that thinking, if a tag matches, a wheel is compatible. Period. Calling one of those matches incorrect takes some extra explaining / opining. There is a built in concept of ~"better" match since the tags have a ranked ordering, but that still doesn't seem to be captured by "incorrect".

https://peps.python.org/pep-0425/#id1

[huonw]

Ah, thanks, I see what you mean: if the comment still applies after I switch to doing a subset computation, I'll reword it to something like "wheels with less preferred tags" to match the language of PEP 425.

[huonw]

I'm not fully convinced, but I think subset is the right choice still. The only reason to use --resolve-local-platforms at all is to make more resolves succeed by bringing sdists in play. If you want an "optimal" resolve and you have an environment that matches the target host, then you don't use --platform or --complete-patform, you just use --python to say this exact one right here - we have our ducks in a row and this matches production.

Ah, okay, and correspondingly, one can avoid using a "suboptimal" resolve due to a more limited local platform by not passing --resolve-local-platforms and only passing --complete-platforms. (The only hitch is users having the ability to disable --resolve-local-platforms for wrappers like python_awslambda.)

I think that makes sense, I'll switch to doing a subset check. I'm not sure of the exact check, but I suspect:

1. for supported_tags, doing set(candidate_complete.supported_tags) <= set(requested_complete.supported_tags) is enough? (Something like all(requested_complete.supported_tags.rank(tag) is not None for tag in candidate_complete.supported_tags) isn't required.)
2. marker_environment doesn't need to be checked (the 'interesting' parts are already captured by the Platform used in the dict for matching)

Does that sound correct? Thanks!

[jsirois]

I think you've got that all right @huonw.

[huonw]

I've made that update, added a test, and also realised that the previous approach was fundamentally broken (I think an interpreter needs to actually be rejected if any of its intersecting platforms are incompatible, while the previous version continued to accepted it).

[jsirois]

I won't have time to dive deep until later, but Pex supports Python 2.7 still, so watch that syntax.

[huonw]

No worries.

Thanks for the 2.7 tip, I'll fix up the various errors (tomorrow, probably).

[jsirois]

Thanks @huonw - this looks good. I'll merge and get this out in a release shortly. You should be able to configure the version and known_versions [pex-cli] subsystem options in your Pants config to then be up and running with this at that point.

[huonw]

Woohoo, thank you!