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

specify (interpreter-constraint) x (platform), and pass --python-version to pip regardless of a local binary #1033

Closed
cosmicexplorer opened this issue Sep 19, 2020 · 8 comments · Fixed by #1047
Assignees
Labels
answered API Issues that relate to changes in the public Pex API enhancement resolver

Comments

@cosmicexplorer
Copy link
Contributor

cosmicexplorer commented Sep 19, 2020

When i run pex.resolver.resolve_multi(), it asks for both interpreters and platforms separately. A few questions:

  1. Is it possible to construct a PythonInterpreter without actually having a matching interpreter binary on the local disk?
  2. If the answer to (1) is "no", is there any way to create a pex file with requirements downloaded for an interpreter I don't have on my local machine?
  3. If we are restricting PythonInterpreter to only local files as per (1), why wouldn't the platform argument be something we could infer from the interpreter binary path? It looks like PythonInterpreter has this supported_platforms property. Is there a reason we wouldn't just use that, instead of having to spread that information over a separate platforms kwarg to resolve_multi()?

It seems like (interpreter-constraint) x (platform) are the axes that pip actually uses to resolve, and I'm wondering why we specify those separately, and why we require an interpreter necessarily on the local disk. One alternative might be:

# Monolithic resolve method, instead of a separate resolve_multi().
def resolve(..., matching_interpreters=None):
    if matching_interpreters is None:
        if interpreter is None and platform is None:
            matching_interpreters = [MatchingInterpreter.from_interpreter(PythonInterpreter.get())]
        else:
            # Produces a list of `MatchingInterpreter`s with all combinations of interpreter and platform.
            matching_interpreters = (InterpreterSet(interpreters=[interpreter])
                                     .for_platforms(platforms=[platform]))
    ...

# Create a declarative interpreter spec matching a python 3.6 interpreter 
# on the local machine  -- this is possible currently by iterating over interpreters and checking
# their compatibility. This is a translation of what is possible today.
# This line *does* touch the filesystem, to iterate over interpreters until a matching one is found.
whatever_the_current_36_is = MatchingInterpreter.from_constraints(['CPython==3.6.*']).

# This line, and all the rest, do *not* touch the filesystem or perform any syscalls.
# If this is in the `matched_interpreters` list when calling resolve(), a pip job will be executed with:
# --python-version 3.6.11 --platform linux_x86_64 --abi cp36m
linux_cp_36_m = MatchingInterpreter('CPython==3.6.11', platform=Platform.create('linux_x86_64-cp-36-m'))

# If we pretend we found a 3.6.11 python interpreter in the first example, the above line would be equivalent
# to the below line, which uses a utility method `.copy()`. This is the direct translation of the current
# method of creating a multiplatform pex, which depends on finding an interpreter on the local disk, and
# then providing a set of other platforms to fetch for (totally separate from that local interpreter version)
# discarding the `.platform` of that local interpreter (necessitating the redundant 'current' shorthand).
linux_cp_36_m = whatever_the_current_36_is.copy(platform=Platform.create('linux_x86_64-cp-36-m'))

# Ask for wheels matching the most recent macosx_10_XY_x86_64 platform,
# because that has the widest compatibility, and many wheels are simply published
# with a platform matching the publisher's laptop, which may be very recent, for no reason.
osx_36_abi3 = MatchingInterpreter('CPython==3.6.11', platform=Platform.most_recent_osx(abi_tag='abi3'))

# Note that *only* == interpreter constraints are allowed when using the normal constructor.
# This is a possible convenience method to match CPython specifically, with shorter strings.
# This should produce the exact same result as the previous line.
 osx_36_abi3 = MatchingInterpreter.CPython('3.6.11', platform=Platform.create('macosx_10_14_x86_64-cp36-abi3'))

If this was to be entertained, it seems that it could subsume the functionality of the existing resolve() method entirely (translating interpreters and platforms to a list of MatchingInterpreters), and therefore could avoid breaking any existing users of the pex library API at all (while closing #969).

We would have to figure out how to appropriately error when:

  1. the user specifies an interpreter version (or platform) that doesn't exist on their machine, and
  2. some requirements are platform-specific, and there are no prebuilt wheels for that (interpreter-version) x (platform) pair.

However, I believe we already error out today in this case, if we request e.g. linux_86_64-cp36-abi3 in platforms, and there are no prebuilt wheels available for that platform. So we should be able to extend or keep the same error message as before when things don't match up -- we would just be adding a new failure state (which just errors out much earlier today, at interpreter selection time), by allowing the user to resolve for interpreters they don't have on their system.

Regarding the Pex CLI, I believe we could consider an --explicit-python-version option corresponding to the first argument of MatchingInterpreter(...), aka:

> pex --explicit-python-version CPython==3.6.11 ...

This flag would be mutually exclusive with both --python and --interpreter-constraints, but could be applied multiple times to resolve for multiple explicit interpreter versions.

@cosmicexplorer
Copy link
Contributor Author

It would actually be very very nice if we could make the abi_tag into a python 3 enum (along with values for PEX_* config vars and cli args, but that would be more an internal-facing change). For now it's fine to just have a nice error message for invalid tags.

@cosmicexplorer
Copy link
Contributor Author

cosmicexplorer commented Sep 23, 2020

Hmm. Upon reviewing open issues I find that the problem this tries to solve is very similar to #1020, but could be said to extend the goals of that ticket to the Pex library API and CLI, and perhaps clinching a solution to #969.

@jsirois
Copy link
Member

jsirois commented Sep 27, 2020

1st - to be clear - Pex and Pip are offering two unequal forms of resolving. I'll stick to Pex terminology but its a 1-1 map into Pip:

  1. Full featured: You have a local interpreter that can be used to build a wheel when no wheel is resolvable but an sdist is.
  2. "Foreign": Either the platform you're resolving for is foreign (You're trying to build a platform-specific OSX PEX on linux) or its not, but the targeted interpreter is not present (You're trying to build a Python 2.7 PEX for linux on a linux machine with no Python 2.7 interpreter).
  1. Is it possible to construct a PythonInterpreter without actually having a matching interpreter binary on the local disk?

No.

  1. If the answer to (1) is "no", is there any way to create a pex file with requirements downloaded for an interpreter I don't have on my local machine?

Yes - specify the exact platform of that interpreter. IE: the most specific wheel tag it supports.

I'll stop there since it may be the case that the preamble above clears up misconceptions about the role of PythonInterpreter vs Platform when resolving distributions. In short, PythonInterpreters and Platforms should form a disjoint set of DistributionTargets you're attempting to build the PEX for.

@jsirois
Copy link
Member

jsirois commented Sep 27, 2020

... and perhaps clinching a solution to #969

#969 already has a solution fully outlined. Its just a name change and the collapse of two functions, one that takes lists and one that takes single items into the one that takes lists since its the most general.

@jsirois
Copy link
Member

jsirois commented Sep 27, 2020

After reading through all your stuff, I think your use of (interpreter-constraint) x (platform) sums up the confusion. It is not in fact a cross product that's used to resolve, its (interpreter-constraint) | (platform) - ie a heterogenous set of individual targets to resolve for, some local interpreters, some foreign platforms. The code is here:
https://github.com/pantsbuild/pex/blob/b9217232a913e8d32fb78e8c1625f7b56e05f613/pex/resolver.py#L958-L975

If I'm understanding the confusion correctly, maybe you could contribute an API doc fix to clear things up for you?

@cosmicexplorer
Copy link
Contributor Author

I think you have absolutely understood the confusion correctly and an API doc fix sounds just right. Would that be in the docstring for the resolve/resolve_multi() methods?

Yes - specify the exact platform of that interpreter. IE: the most specific wheel tag it supports.

This is exactly the answer I was missing earlier! I think that the two methods of resolution you specified map directly onto the operations I was thinking of.

Thanks so much for your extremely thorough response!

@jsirois
Copy link
Member

jsirois commented Sep 27, 2020

Would that be in the docstring for the resolve/resolve_multi() methods?

I think so, yes. You're the API reader here though so you know better where the words should go and what the words should have been,

@cosmicexplorer
Copy link
Contributor Author

Great! I will think about what would seem most effective!

@jsirois jsirois self-assigned this Sep 27, 2020
cosmicexplorer added a commit that referenced this issue Sep 28, 2020
…s in the library API and CLI (#1047)

@jsirois read my mind in #1033 (comment) after I asked why Pex couldn't resolve for an interpreter it doesn't have locally. This was in fact already possible, and he suggested a documentation fix to make it more clear. These changes make it clear that:
- `platform{,s}` takes precedence over `interpreter{,s}` when resolving wheels
- `interpreter{,s}` are used to seed `platform{,s}` if not provided
- matching `interpreter{,s}` must be provided for all `platform{,s}` if any distributions need to be built from source

Closes #1033.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered API Issues that relate to changes in the public Pex API enhancement resolver
Projects
None yet
2 participants