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

Rename python3.13t.exe to python.exe in Windows free-threaded distributions #373

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Oct 17, 2024

Addressing astral-sh/uv#8298

@zanieb zanieb changed the title Include python.exe copy of python3.13t.exe in Windows free-threaded distributions Rename python3.13t.exe to python.exe in Windows free-threaded distributions Oct 17, 2024
zanieb added a commit to astral-sh/uv that referenced this pull request Oct 17, 2024
A temporary fix for #8298 while we
wait for my slower upstream fix at
astral-sh/python-build-standalone#373

I think we'll want this machinery anyway to ensure that the various
executable names are available? Otherwise we need to special-case all
the `python` names in `uv run`?

We don't have unit test coverage of managed downloads, so I added an
[integration
test](https://github.com/astral-sh/uv/actions/runs/11394150653/job/31703956805?pr=8310)
similar to what we have for Linux.
@indygreg
Copy link
Collaborator

Reading the linked issue, I guess we're going the rename route instead of an additional copy?

On Linux and macOS before freethreaded builds the canonical binary was pythonX.Y and we had symlinks from pythonX and python to it. I have a light preference towards shipping copies of the exe to maintain the same logical layout. But if that's not how the official Windows installers work, then maybe the single python.exe is best.

@zanieb
Copy link
Member Author

zanieb commented Oct 18, 2024

Reading the linked issue, I guess we're going the rename route instead of an additional copy?

Yeah, I initially thought the copy was a better option but I trust Sam and Steve's opinions.

It seems simplest to maintain consistency with the existing Windows distributions first, i.e., by using python.exe. I like the idea of matching the Linux and macOS layouts, but parity with Linux and macOS is tricky, since symlinks are generally a privileged operation on Windows. My experience thus far in uv is that Windows Python executables we discover have the same name across versions. I think they avoided this with the free-threaded executable in the official installer to avoid breaking people's systems, but we don't really have that problem here.

@zanieb zanieb marked this pull request as ready for review October 30, 2024 12:51
@zanieb zanieb requested a review from charliermarsh October 30, 2024 12:52
@@ -1759,6 +1759,15 @@ def build_cpython(
log("copying %s to %s" % (source, dest))
shutil.copyfile(source, dest)

# Rename to `python.exe` when an alternative executable is built, e.g., when
# free-threading is enabled the name is `python3.13t.exe`.
canonical_python_exe = install_dir / "python.exe"
Copy link
Member

Choose a reason for hiding this comment

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

Is install_dir the same as out_dir / "python" / "install"?

Copy link
Member

Choose a reason for hiding this comment

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

I'm just comparing to python_exe = out_dir / "python" / "install" / python_exe below.

Copy link
Member Author

Choose a reason for hiding this comment

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

canonical_python_exe = install_dir / "python.exe"
if not canonical_python_exe.exists():
os.rename(
install_dir / python_exe,
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to instead test whether python_exe is "python.exe"?

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered both option and don't see a strong case for either one. I think this approach avoids coupling assumptions about the name and mildly prefer it for that reason.

@zanieb zanieb merged commit ee9062d into main Oct 31, 2024
316 checks passed
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 this pull request may close these issues.

3 participants