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

Allow relative Python executable paths in Windows trampoline #3717

Merged
merged 4 commits into from
May 22, 2024

Conversation

ofek
Copy link
Contributor

@ofek ofek commented May 21, 2024

Summary

This is a prerequisite for #3669

Test Plan

Download one of the standalone distributions on Windows then use its Python to run the following script and then run the scripts it creates (only pip):

from __future__ import annotations

import sys
import sysconfig
from contextlib import closing
from importlib.metadata import entry_points
from io import BytesIO
from os.path import relpath
from pathlib import Path
from tempfile import TemporaryDirectory
from zipfile import ZIP_DEFLATED, ZipFile, ZipInfo

# Change this line to your real path
LAUNCHERS_DIR = Path('C:\\Users\\ofek\\Desktop\\code\\uv\\crates\\uv-trampoline\\target\\x86_64-pc-windows-msvc\\release')
SCRIPT_TEMPLATE = """\
#!{executable}
# -*- coding: utf-8 -*-
import re
import sys
from {module} import {import_name}
if __name__ == "__main__":
    sys.argv[0] = re.sub(r"(-script\\.pyw|\\.exe)?$", "", sys.argv[0])
    sys.exit({function}())
"""


def select_entry_points(ep, group):
    return ep.select(group=group) if sys.version_info[:2] >= (3, 10) else ep.get(group, [])


def main():
    interpreters_dir = Path(sys.executable).parent
    scripts_dir = Path(sysconfig.get_path('scripts'))

    ep = entry_points()
    for group, interpreter_name, launcher_name in (
        ('console_scripts', 'python.exe', 'uv-trampoline-console.exe'),
        ('gui_scripts', 'pythonw.exe', 'uv-trampoline-gui.exe'),
    ):
        interpreter = interpreters_dir / interpreter_name
        relative_interpreter_path = relpath(interpreter, scripts_dir)
        launcher_data = (LAUNCHERS_DIR / launcher_name).read_bytes()

        for script in select_entry_points(ep, group):
            # https://github.com/astral-sh/uv/tree/main/crates/uv-trampoline#how-do-you-use-it
            with closing(BytesIO()) as buf:
                # Launcher
                buf.write(launcher_data)

                # Zipped script
                with TemporaryDirectory() as td:
                    zip_path = Path(td) / 'script.zip'
                    with ZipFile(zip_path, 'w') as zf:
                        # Ensure reproducibility
                        zip_info = ZipInfo('__main__.py', (2020, 2, 2, 0, 0, 0))
                        zip_info.external_attr = (0o644 & 0xFFFF) << 16

                        module, _, attrs = script.value.partition(':')
                        contents = SCRIPT_TEMPLATE.format(
                            executable=relative_interpreter_path,
                            module=module,
                            import_name=attrs.split('.')[0],
                            function=attrs
                        )
                        zf.writestr(zip_info, contents, compress_type=ZIP_DEFLATED)

                    buf.write(zip_path.read_bytes())

                # Interpreter path
                interpreter_path = relative_interpreter_path.encode('utf-8')
                buf.write(interpreter_path)

                # Interpreter path length
                interpreter_path_length = len(interpreter_path).to_bytes(4, 'little')
                buf.write(interpreter_path_length)

                # Magic number
                buf.write(b'UVUV')

                script_data = buf.getvalue()

            script_path = scripts_dir / f'{script.name}.exe'
            script_path.write_bytes(script_data)


if __name__ == '__main__':
    main()

@@ -275,7 +275,26 @@ fn find_python_exe(executable_name: &CStr) -> CString {
String::from("Failed to close file handle")
});

path
if !path.as_bytes().starts_with(b"..") {
Copy link
Member

Choose a reason for hiding this comment

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

Is there no has_root or similar that we can use here? There are lots of relative paths that won't necessarily start with this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've never done work without the standard library so I'm not sure what is possible.

Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

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

Can you share the bigger picture on how you plan to use or integrate this?

@ofek
Copy link
Contributor Author

ofek commented May 21, 2024

Sure, basically the script I put in the description I will use to finalize an entire Python installation until the mentioned feature request is implemented and I can install packages with that flag instead of running that script as a postprocessing step.

@ofek
Copy link
Contributor Author

ofek commented May 21, 2024

@charliermarsh charliermarsh self-requested a review May 21, 2024 23:25
@charliermarsh
Copy link
Member

Thanks. I want to play around with the code a bit before merging to see if it can be generalized at all.

@charliermarsh charliermarsh self-assigned this May 21, 2024
@charliermarsh
Copy link
Member

I will try to get to it tonight though.

@charliermarsh
Copy link
Member

I tried to invert the condition to make it a bit more general (e.g., shouldn't ./path/to/python work too?). Would you mind re-testing?

@ofek
Copy link
Contributor Author

ofek commented May 22, 2024

Just tried locally, it still works!

@ofek
Copy link
Contributor Author

ofek commented May 22, 2024

  • I guess that should work as well but I can't imagine a scenario in which the Python interpreter would be inside the scripts directory other than a virtual environment and I don't see a reason to pre-build one nor do I think that would work in practice. Better to be more correct just in case though 😄
  • Do these get updated automatically after a merge? https://github.com/astral-sh/uv/tree/main/crates/uv-trampoline/trampolines

@charliermarsh
Copy link
Member

I think I have to rebuild them as part of the PR.

@ofek
Copy link
Contributor Author

ofek commented May 22, 2024

How do you rebuild them, locally?

@charliermarsh
Copy link
Member

I followed the cross compilation instructions in the uv-trampoline README. I installed cargo-xwin, then ran (e.g.) cargo +nightly-2024-03-19 xwin build --release --target aarch64-pc-windows-msvc --bin.

@charliermarsh
Copy link
Member

I'm having trouble verifying them locally though. (I can't get them to fail.)

@ofek
Copy link
Contributor Author

ofek commented May 22, 2024

Fail how?

@charliermarsh
Copy link
Member

I just want to make sure they're actually being used. So I tried (e.g.) overwriting the x86 trampolies with the ARM trampolines but my commands are still working which seems wrong.

@ofek
Copy link
Contributor Author

ofek commented May 22, 2024

@charliermarsh
Copy link
Member

Yeah, although I think I'm getting it to fail as expected now.

@charliermarsh charliermarsh merged commit 82820d0 into astral-sh:main May 22, 2024
45 checks passed
@charliermarsh
Copy link
Member

For future reference: changing just the trampolines doesn't trigger a rebuild (with cargo run), you need to make a change to the crate itself it seems. That's why I wasn't seeing the expected failures.

@ofek
Copy link
Contributor Author

ofek commented May 22, 2024

Thank you very much!

@charliermarsh
Copy link
Member

Thanks Ofek!

@charliermarsh
Copy link
Member

Improved the README a bit: #3731

@ofek ofek deleted the relative-trampoline branch May 22, 2024 02:32
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