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

Are UV Trampolines required on non-Windows systems? #2687

Closed
bengsparks opened this issue Feb 16, 2025 · 9 comments
Closed

Are UV Trampolines required on non-Windows systems? #2687

bengsparks opened this issue Feb 16, 2025 · 9 comments
Assignees

Comments

@bengsparks
Copy link

The work done in #2665 will cause the .exes from astral's uv to be fetched for non-windows systems; is this necessary?
If not, this could be fixed with checks against sys.platform or os.name here and here.

@jsirois
Copy link
Member

jsirois commented Feb 16, 2025

It is necessary. See last paragraph for why.

@bengsparks do you have a use case where you need to build a Pex sdist, wheel or PEX? They should only be fetched when building a Pex sdist or wheel, not when using one of these (or the Pex PEX). For a pre-built Pex sdist, wheel or PEX the stubs are included as resources.

They are needed in all 3 cases regardless of what platform the Pex artifact ends up on since none of the Pex artifacts are (currently) platform-specific. For example, the wheel is and always has been universal py2.py3-none-any.

@jsirois jsirois self-assigned this Feb 16, 2025
@bengsparks
Copy link
Author

I came across this behavior when building the pex package itself, specifically for linux and darwin, and was surprised to see .exes being added to the package during the build process, so I thought I'd follow up here.
I should have framed the issue in more of a questioning manner, apologies for that. I suppose the question is more "is this necessary for the pex package itself"

@jsirois
Copy link
Member

jsirois commented Feb 16, 2025

@bengsparks can you provide even more information? Why are you re-packaging? Are you re-packaging without a fork? With a fork you could make the edit of course. Perhaps you have a fork but wish to maintain no or less long term patches?

Basically fill me way in please on the surrounding context.

The best I can imagine is something like _PEX_I_REALLY_WANT_TO_BUILD_YOU_WITHOUT_WINDOWS_MULTIPLATFORM_SUPPORT_AND_PROMISE_NOT_TO_DISTRIBUTE_YOU_WHERE_WINDOWS_USERS_LURK_=1 build .. I'd want a pretty good reason to add that env var support though.

@jsirois
Copy link
Member

jsirois commented Feb 16, 2025

I should have framed the issue in more of a questioning manner, apologies for that. I suppose the question is more "is this necessary for the pex package itself"

It most definitely is since Pex has supported producing multi-platform PEXes from ~day 1. At least 10 years anyway. In other words, you can build a Mac PEX from Linux or a Mac + Linux PEX from Linux or ... when the windows support is done, add that into the mix and match. Without Windows stubs Pex would have to have code to tell you it was built neutered when erroring out or else it would have to fetch the stubs just in time. I didn't entertain the latter since there are many Pex users in locked down environments that use alternate PyPIs - so this would necessitate an option set to grab the stubs from elsewhere, etc. I provide such options for --scie, but it would be nice to avoid and just ship the embedded resources, since so much effort was put into making them small by Nathaniel Smith. A further option would be to check the binary stubs in. This is what Pip does, for example. I wanted to avoid that though.

@bengsparks
Copy link
Author

bengsparks commented Feb 16, 2025

The whole context is that NixOS/nixpkgs#382636 encountered a build failure due to the exes being fetched during build time, which is forbidden. This is how I originally came across this topic.

In this PR, a patch was added that simply removes the fetching of these exes, but I don't know enough about the package to confidently state that that is the correct thing to do.

But if I understand your explanation correctly, it means that the patch in the PR is wrong, as a user could intend to build a PEX on platforms supported by nix (linux + darwin), and distribute the result to Windows users

@jsirois
Copy link
Member

jsirois commented Feb 16, 2025

Thanks for the background. That helps a ton.

But if I understand your explanation correctly, it means that the patch in the PR is wrong, as a user could intend to build a PEX on platforms supported by nix (linux + darwin), and distribute the result to Windows users

Correct.

Does your build process allow for fetching the stubs elsewise and then pointing the Pex build process at them if I were to add a flag or env var or some such knob? I'd like to weigh that against giving in and checking in the stubs. Or, alternatively, having the build process clone the uv repo or using a git sub-module for said-same.

@bengsparks
Copy link
Author

bengsparks commented Feb 16, 2025

Yes, we can fetch the stubs prior to build time, and simply copy them into the correct location.
You wouldn't have to change anything to your build process, as pex_build does not attempt to fetch anything if the trampolines are already there

Nix snippets if you're interested ```nix uv-trampoline = { aarch64-gui = fetchurl { url = "https://raw.githubusercontent.com/astral-sh/uv/refs/tags/0.5.29/crates/uv-trampoline/trampolines/uv-trampoline-aarch64-gui.exe"; hash = "sha256-mb8x1FpyH+wy11X5YgWfqh/VUwBb62M4Zf9aFr5V4EE="; };

aarch64-console = fetchurl {
url = "https://raw.githubusercontent.com/astral-sh/uv/refs/tags/0.5.29/crates/uv-trampoline/trampolines/uv-trampoline-aarch64-console.exe";
hash = "sha256-1S2aM+6CV7rKz+3ncM5X7kk7uDQeuha1+8lUEMYC75k=";
};

x86_64-gui = fetchurl {
url = "https://raw.githubusercontent.com/astral-sh/uv/refs/tags/0.5.29/crates/uv-trampoline/trampolines/uv-trampoline-x86_64-gui.exe";
hash = "sha256-icnp1oXrOZpc+dHTGvDbTHjr+D8M0eamvRrC9bPI/KI=";
};

x86_64-console = fetchurl {
url = "https://raw.githubusercontent.com/astral-sh/uv/refs/tags/0.5.29/crates/uv-trampoline/trampolines/uv-trampoline-x86_64-console.exe";
hash = "sha256-nLopBrlCMMFjkKVRlY7Ke2zFGpQOyF5mSlLs0d7+HRQ=";
};
};

preBuild = ''
mkdir -p pex/windows/stubs
cp ${uv-trampoline.aarch64-gui} pex/windows/stubs/uv-trampoline-aarch64-gui.exe
cp ${uv-trampoline.aarch64-console} pex/windows/stubs/uv-trampoline-aarch64-console.exe
cp ${uv-trampoline.x86_64-gui} pex/windows/stubs/uv-trampoline-x86_64-gui.exe
cp ${uv-trampoline.x86_64-console} pex/windows/stubs/uv-trampoline-x86_64-console.exe
'';

</details>

@jsirois
Copy link
Member

jsirois commented Feb 16, 2025

Yes, we can fetch the stubs prior to build time, and simply copy them into the correct location.

Aha. Ok. Well with the minimal bit I've looked at all this, that sounds basically par for the course of what you buy into when you have a build process like this; so I think that would be great. Do that!

@bengsparks
Copy link
Author

Awesome, thanks for your time :)

@bengsparks bengsparks changed the title Do not fetch windows stubs on non-windows systems. Are UV Trampolines required on non-Windows systems? Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants