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

Windows: python and python3 don't point to the same installation #915

Closed
sbrodehl opened this issue Nov 11, 2021 · 8 comments · Fixed by #917
Closed

Windows: python and python3 don't point to the same installation #915

sbrodehl opened this issue Nov 11, 2021 · 8 comments · Fixed by #917

Comments

@sbrodehl
Copy link
Contributor

sbrodehl commented Nov 11, 2021

Description

Hi guys!

I am building wheels with cibw, but noticed some issues with windows builds, where installed packages during CIBW_BEFORE_BUILD step would not show up as installed during the build step.
As it turn out, python3 is not pointing to the installed python version, but tp the hosts python version (?).

Here is an example from the build log linked to, where cp37 is used:

cmd path
python3 /c/hostedtoolcache/windows/Python/3.10.0/x64/python3
python /c/cibw/python/python.3.7.9/tools/python

This differs from the behavior in linux and macos, where both python and python3 point to the same installation, examples below.

platform cmd path
linux python3 /opt/python/cp310-cp310/bin/python3
linux python /opt/python/cp310-cp310/bin/python
macos python3 /Library/Frameworks/Python.framework/Versions/3.7/bin/python3
macos python /tmp/cibw_bin/python (which looks like a link to the version above)

It would be great, if python on windows builds can also point to the installed cpython version, what do you think?

Build log

https://github.com/lukeparser/lukeparser/runs/4182800043?check_suite_focus=true

CI config

https://github.com/lukeparser/lukeparser/blob/88fcaf2de9599c62fffd26c4f85fee99c17278ec/.github/workflows/cibuildwheel.yml#L100

@sbrodehl
Copy link
Contributor Author

On a second note, cibuildwheel explicitly calls python -m pip wheel ... - on every platform, see windows.py#L196, macos.py#L224, linux.py#L198 - which may not be the python version/installation cibuildwheel is invoked with (e.g. python3 -m cibuildwheel ...).

As a user I would expect that cibuildwheel would use the same one.

A few sanity checks are performed to ensure the installed cpython version matches the one python links to (see linux.py#L164, macos.py#L208, windows.py#L178).
Here, a check if the invoked from python and the actually used python matched would be very usefull to the user, too!

@henryiii
Copy link
Contributor

henryiii commented Nov 12, 2021

On note one, that looks like a bug, we should be making sure both python and python3 are valid (and we do that on macOS, it seems, and I know manylinux does this, just not Windows). I'm a little surprised it's not there, but we should add it. How do you do that on Windows? I know symlinking isn't always available on Windows.

On your second note, you never want cibuildwheel to use the host Python. That is only used to call cibuildwheel; there are no special requirements on it; on macOS, it probably will always be an unacceptable version of Python to build with (only official downloads have the proper compilation settings), and besides, it's not even related to the version you are trying to build. Python 3.10 host can build wheels for all versions of Python, it's not using the host Python 3.10.

@sbrodehl
Copy link
Contributor Author

Yes, you are right. For a moment I forgot that there is another layer of abstraction in there. Sorry for the confusion!
But, the python used in before_all or before_build can differ from the one used by cibw, which can go unnoticed. A check here would be nice, but cumbersome to implement, I guess.

Back to the original issue: nuget only installs python.exe, there is a note for the microsoft store package, which installs commands with version number suffixes, but other than that ...
I know there are some issues with symlinking on windows, so can we just copy python.exe and call it python3.exe?

@joerick
Copy link
Contributor

joerick commented Nov 12, 2021

Thanks for the detailed bug report, @sbrodehl ! I agree, this looks like a bug on Windows. I think that Windows doesn't normally have the python3 alias, but if runners have it available on PATH, that's gonna cause issues.

We already do symlinking with the pypy install on windows to make pypy3.exe available as python.exe,

(installation_path / "python.exe").symlink_to(installation_path / "pypy3.exe")

so I expect Path.symlink_to() should work.

@sbrodehl
Copy link
Contributor Author

Great, if it works this would be my go to solution. I've gone ahead and drafted a pull request to add the symlink.

Do you think that we need to make sure there is no python3.exeon PATH beforehand?

@sbrodehl
Copy link
Contributor Author

Hey @joerick, can you please explain briefly (or point to further reading) what issues can arise if runners have a python3 alias in PATH?

@henryiii
Copy link
Contributor

This issue, I believe is what he meant. :) According to PEP 394 the "correct" term for Python is python3, and python is optional. Admittedly, the original PEP excluded Windows, but most CI systems try to make Windows Unix-like so that the same commands work across a build matrix.

@sbrodehl
Copy link
Contributor Author

Thank you very much!

Feel free to link this issue to #917, or I will close it when the PR gets merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants