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

rez.pip: Support python 2 executable on Windows (796) #798

Merged

Conversation

JeanChristopheMorinPerso
Copy link
Member

Fixes #796.

On Windows, versions of Python less than 2 doesn't have versioned executable... So I'm basically just adding a check for windows and only append the version when not under windows.

@JeanChristopheMorinPerso JeanChristopheMorinPerso added bug rez-pip ingesting py pkgs into rez (pip, wheels, etc) os:windows Windows-specific labels Nov 12, 2019
@bfloch
Copy link
Contributor

bfloch commented Nov 12, 2019

Another way to deal with it is https://www.python.org/dev/peps/pep-0397/

Python Launcher for Windows Version 3.7.5150.1013

usage:
py [launcher-args] [python-args] script [script-args]

Launcher arguments:

-2     : Launch the latest Python 2.x version
-3     : Launch the latest Python 3.x version
-X.Y   : Launch the specified Python version
     The above all default to 64 bit if a matching 64 bit python is present.
-X.Y-32: Launch the specified 32bit Python version
-X-32  : Launch the latest 32bit Python X version
-X.Y-64: Launch the specified 64bit Python version
-X-64  : Launch the latest 64bit Python X version
-0  --list       : List the available pythons
-0p --list-paths : List with paths

Not sure if it is guaranteed to be installed but once it is there is solves a bunch of issues.
It also respects #! shebang to some degree.

@JeanChristopheMorinPerso
Copy link
Member Author

@bfloch thanks for pointing this out. I just don't know if it's available in all Python versions. I think we can go with the current solution as it's quite simple and won't bring us much support I think.

@instinct-vfx
Copy link
Contributor

I think python launcher was added with 3.3 but i am not sure. 2.7 does not ship with it.

Copy link
Contributor

@instinct-vfx instinct-vfx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the easiest way to fix the issue.

@bfloch
Copy link
Contributor

bfloch commented Nov 13, 2019

On windows both Python 2.7 and 3.x will be plain python.exe.
If you have and 3.4+ (I believe) installed you will have the launcher.

My thought was that a more reliable detection would be on windows to try py.exe and let it handle it. They already seem to have accounted for all the cases:
https://github.com/python/cpython/blob/master/PC/launcher.c

If it is not available just roll as is - since in this case it is likely you only have python 2.x installed anyway.

I just can't see how which + python.exe would have a chance to find the right version on Windows. It pretty much depends on the order of installation, no?

@nerdvegas
Copy link
Contributor

Happy to go with this until proven otherwise that we need something more complex.

This and some other recent PR merges coming soon.

Cheers all
A

@bfloch
Copy link
Contributor

bfloch commented Nov 24, 2019

Fair. I have not yet studied py.exe thoroughly to give a definite recommendation.

@nerdvegas nerdvegas merged commit 152b341 into AcademySoftwareFoundation:master Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug os:windows Windows-specific rez-pip ingesting py pkgs into rez (pip, wheels, etc)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

find_pip_from_context failing on Windows platform
4 participants