fixed uv can't create .venv for cpython-x86 on Windows #2707
Conversation
Adaptation to the win32 platform is added.
|
Related to astral-sh/rye/issues/952 |
| if platform_info[0] == "win32": | ||
| operating_system, version_arch = "win", "i386" | ||
| else: | ||
| raise ValueError(f"Unsupported Platform: {platform_info[0]}") |
There was a problem hiding this comment.
We use "result": "error" and handle the error in rust code instead of raising exceptions
There was a problem hiding this comment.
I know, but this is python code, and this function is just to get platform information. For unsupported platforms, you should also exit here. I've thought of two solutions. One is to just exit and add exception printing, and the other is to raise an exception. I think using an exception here might be appropriate.
There was a problem hiding this comment.
If the python code raises an exception, the rust caller doesn't know what went wrong: Is the platform not supported? Is the version not supported? Is this python not a python? Is it a permission error? In rust we get an exit status, stdout and stderr, we can't even tell if we started an actual python interpreter.
Showing a python stacktrace is verbose and not helpful to the user. A user only needs one line (platform not supported) and does not need to see implementation details of our interpreter checking code. Instead, we return that the script ran successfully but the platform was not supported here and transform it into a rust error here. This way we get a clear, concise error message:
$ uv venv
× Can't use Python at `/home/konsti/projects/uv/.venv/bin/python3`
╰─▶ Unknown operation system: `wedontknowthisos`
There was a problem hiding this comment.
Ok, thanks for your answer
| [operating_system, version_arch] = sysconfig.get_platform().split("-", 1) | ||
| platform_info = sysconfig.get_platform().split("-", 1) | ||
| if len(platform_info) == 1: | ||
| if platform_info[0] == "win32": |
There was a problem hiding this comment.
Do you have some context on why 32-bit windows is the only platform where that happens, and can you add a comment above the line explaining the special case?
There was a problem hiding this comment.
https://docs.python.org/3/library/sysconfig.html#sysconfig.get_platform
According to the documentation, sysconfig.get_platform () returns "win32" when using the x86 version of Windows. This is a known issue. I'll add the documentation to the notes later.
|
A "check system" CI job would make sense for that. |
|
@Zander-1024 Do you want to add a check system check or should we merge without? |
Zander-1024
force-pushed
the
main
branch
2 times, most recently
from
6fd53d4
to
5d11389
Compare
|
Sorry, I don't know where the problem with this ci build lies. Looking at the print, I should have installed pylint. |
|
The system check tests require the Python you are testing with to be the default on the system e.g. |
|
Thanks! |
Adaptation to the win32 platform is added.
https://docs.python.org/3/library/sysconfig.html#sysconfig.get_platform
Summary
fixed uv can't create .venv for cpython-x86 on Windows
uv can't create .venv for cpython-x86 on Windows