Add a trampoline variant that just executes `python` #8637

zanieb · 2024-10-28T15:57:06Z

Currently, our trampoline is used to convert <command> [args] to python <command> [args] for script entrypoints installed into virtual environments. For #8458, it'd be nice to convert a shim python3.12 [args] to python [args]. Here, we modify the trampolines to support this use-case.

The only change we really need here is to avoid injecting <command> into the child process. We change the "magic number" at the end of the trampoline executables from UVUV to UVSC and UVPY which define "script" and "python" variants to the trampoline. We then omit the <command> injection in the latter case. We also omit writing the zip script payload when constructing the trampoline.

To support construction of the new variant, a new uv-trampoline-builder crate is introduced — this avoids requirements on uv-install-wheel in future work. I also use uv-trampoline-builder to consolidate some of the test setup for uv-trampoline.

There should be no backwards compatibility concerns, since trampolines are fully self-referential.

I rebased to fix the commits at the end, as this took many iterations to get working via CI. This should roughly be reviewable by commit if you prefer.

crates/uv-trampoline/src/bounce.rs

zanieb · 2024-10-28T19:56:06Z

crates/uv-install-wheel/src/wheel.rs

-///
-/// <https://github.com/pypa/pip/blob/fd0ea6bc5e8cb95e518c23d901c26ca14db17f89/src/pip/_vendor/distlib/scripts.py#L248-L262>
-#[allow(unused_variables)]
-pub(crate) fn windows_script_launcher(


This moved ~unchanged into uv-trampoline-builder

zanieb · 2024-10-28T19:56:20Z

crates/uv-install-wheel/src/wheel.rs

@@ -1075,54 +969,6 @@ mod test {
        Ok(())
    }

-    #[test]
-    #[cfg(all(windows, target_arch = "x86"))]
-    fn test_launchers_are_small() {


This moved into uv-trampoline-builder, size limit bumped

zanieb · 2024-10-28T19:56:59Z

crates/uv-trampoline-builder/src/lib.rs

+
+#[cfg(all(test, windows))]
+#[allow(clippy::print_stdout)]
+mod test {


These tests pulled from uv-trampoline::tests::harness and uv-install-wheel

zanieb · 2024-10-28T19:57:04Z

crates/uv-trampoline-builder/src/lib.rs

+///
+/// Sort of equivalent to a `python` symlink on Unix.
+#[allow(unused_variables)]
+pub fn windows_python_launcher(


This is new

zanieb · 2024-10-28T19:57:13Z

crates/uv-trampoline-builder/src/lib.rs

+///
+/// <https://github.com/pypa/pip/blob/fd0ea6bc5e8cb95e518c23d901c26ca14db17f89/src/pip/_vendor/distlib/scripts.py#L248-L262>
+#[allow(unused_variables)]
+pub fn windows_script_launcher(


This was copied ~unchanged

zanieb · 2024-10-28T19:57:44Z

crates/uv-trampoline/tests/harness.rs

@@ -1,269 +0,0 @@
-use std::io::{Cursor, Write};


This moved ~unchanged into uv-trampoline-builder

…truction utilities

samypr100 · 2024-10-28T23:21:40Z

The trampolines increased from ~45kb to ~80kb

Weird, I noticed none of the code added should've increased it this much

Decided to build it locally (release) it's still in the ~45kb. We should probably update the sizes test to keep using the release build size, not the dev size 😅

Dev Sizes were about ~80kb

zanieb · 2024-10-28T23:40:04Z

Ohhh I see what's happening here. I was also surprised they increased in size. Thanks for pointing that out. That's... annoying to fix.

BurntSushi

LGTM. How did you resolve the test failures from yesterday?

crates/uv-install-wheel/src/wheel.rs

crates/uv-trampoline/src/bounce.rs

BurntSushi · 2024-10-29T11:52:21Z

crates/uv-trampoline/src/bounce.rs

-            Self::Script => &[b'U', b'V', b'S', b'C'],
-            Self::Python => &[b'U', b'V', b'P', b'Y'],
+            Self::Script => b"UVSC",
+            Self::Python => b"UVPY",


Ah whoops, you already did this. :P

Sorry; this is one of those things that wasn't worth moving in the rebase since it was a mess

BurntSushi · 2024-10-29T11:54:19Z

crates/uv-trampoline-builder/src/lib.rs

        assert!(
-            super::LAUNCHER_I686_GUI.len() < 45 * 1024,
+            super::LAUNCHER_I686_GUI.len() < 80 * 1024,


Would using cfg(not(debug_assertions)) as a proxy for "compiling with optimizations" help here? Otherwise, you can't cfg on opt-level at present. So probably the only alternative is a Cargo feature. (Assuming I'm understanding the problem correctly.)

That'd be interesting. I can also just.. run these tests separately from the ones we run to test changes to the trampolines. In CI, we replace the production trampolines with a debug build.

Is the threshold bump still relevant? In the git diff it looks like the binaries even shrank a little

Since I moved the tests, they run against the debug builds we create for testing.

I can fix that somehow, or we can just assert on the size of the debug builds (80kb is still very reasonable).

See #8637 (comment) cbb90ba

zanieb · 2024-10-29T13:59:27Z

@BurntSushi I resolved the test failures by moving the tests from uv-trampoline/src/lib.rs into uv-trampoline-builder/src/lib.rs. Still no idea what was going on there.

zanieb · 2024-10-29T14:20:18Z

Now we test both explicitly

zanieb · 2024-10-29T14:21:03Z

Thanks for the reviews!

Currently, our trampoline is used to convert `<command> [args]` to `python <command> [args]` for script entrypoints installed into virtual environments. For #8458, it'd be nice to convert a shim `python3.12 [args]` to `python [args]`. Here, we modify the trampolines to support this use-case. The only change we really need here is to avoid injecting `<command>` into the child process. We change the "magic number" at the end of the trampoline executables from `UVUV` to `UVSC` and `UVPY` which define "script" and "python" variants to the trampoline. We then omit the `<command>` injection in the latter case. We also omit writing the zip script payload. To support construction of the new variant, a new `uv-trampoline-builder` crate is introduced — this avoids requirements on `uv-install-wheel` in future work. I also use `uv-trampoline-builder` to consolidate some of the test setup for `uv-trampoline`. There should be no backwards compatibility concerns, since trampolines are fully self-referential. I rebased to fix the commits at the end, as this took many iterations to get working via CI. This should roughly be reviewable by commit if you prefer.

…8663) Incorporating #8637 into #8458 - Adds `python-managed` feature selection to Windows CI for `python install` tests - Adds trampoline sniffing utilities to `uv-trampoline-builder` - Uses a trampoline to install Python executables into the `PATH` on Windows

konstin reviewed Oct 28, 2024

View reviewed changes

crates/uv-trampoline/src/bounce.rs Show resolved Hide resolved

zanieb force-pushed the zb/bounce-python branch 3 times, most recently from 4084c76 to 868c15a Compare October 28, 2024 18:01

zanieb temporarily deployed to uv-test-publish October 28, 2024 18:13 — with GitHub Actions Inactive

zanieb temporarily deployed to uv-test-publish October 28, 2024 18:48 — with GitHub Actions Inactive

zanieb temporarily deployed to uv-test-publish October 28, 2024 18:55 — with GitHub Actions Inactive

zanieb temporarily deployed to uv-test-publish October 28, 2024 19:07 — with GitHub Actions Inactive

zanieb force-pushed the zb/bounce-python branch from 7cf64ed to 5529ab6 Compare October 28, 2024 19:12

zanieb temporarily deployed to uv-test-publish October 28, 2024 19:14 — with GitHub Actions Inactive

zanieb temporarily deployed to uv-test-publish October 28, 2024 19:25 — with GitHub Actions Inactive

zanieb requested review from konstin and BurntSushi October 28, 2024 19:30

zanieb marked this pull request as ready for review October 28, 2024 19:38

zanieb mentioned this pull request Oct 28, 2024

Install versioned Python executables into the bin directory during uv python install #8458

Merged

zanieb commented Oct 28, 2024

View reviewed changes

zanieb added 6 commits October 28, 2024 15:14

Add a trampoline variant that executes python

9f7fd2e

Bump rust toolchain version

7909dcc

Add uv-trampoline-builder crate for shared trampoline launcher cons…

357431b

…truction utilities

Update build instructions

c7fdb95

Move tests to new crate

f446582

Bump allowed launcher size

c29b9d9

zanieb force-pushed the zb/bounce-python branch from 3157eb4 to c29b9d9 Compare October 28, 2024 20:15

zanieb temporarily deployed to uv-test-publish October 28, 2024 20:18 — with GitHub Actions Inactive

Rebuild bundled trampolines

b420b40

zanieb temporarily deployed to uv-test-publish October 28, 2024 20:47 — with GitHub Actions Inactive

zanieb changed the title ~~Add a trampoline variant that executes python~~ Add a trampoline variant that just executes python Oct 28, 2024

zanieb mentioned this pull request Oct 28, 2024

Update Windows trampolines for compatibility with Python 3.7 and earlier #8649

Open

konstin approved these changes Oct 29, 2024

View reviewed changes

BurntSushi approved these changes Oct 29, 2024

View reviewed changes

Separate size tests

cbb90ba

zanieb temporarily deployed to uv-test-publish October 29, 2024 14:19 — with GitHub Actions Inactive

zanieb merged commit c335dc5 into main Oct 29, 2024
63 checks passed

zanieb deleted the zb/bounce-python branch October 29, 2024 14:21

zanieb added the internal A refactor or improvement that is not user-facing label Oct 29, 2024

zanieb mentioned this pull request Oct 29, 2024

Add support for installing versioned Python executables on Windows #8663

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a trampoline variant that just executes `python` #8637

Add a trampoline variant that just executes `python` #8637

zanieb commented Oct 28, 2024 •

edited

Loading

zanieb Oct 28, 2024

zanieb Oct 28, 2024

zanieb Oct 28, 2024

zanieb Oct 28, 2024

zanieb Oct 28, 2024

zanieb Oct 28, 2024

samypr100 commented Oct 28, 2024 •

edited

Loading

zanieb commented Oct 28, 2024

BurntSushi left a comment

BurntSushi Oct 29, 2024

zanieb Oct 29, 2024

BurntSushi Oct 29, 2024

zanieb Oct 29, 2024

konstin Oct 29, 2024

zanieb Oct 29, 2024

zanieb Oct 29, 2024

zanieb commented Oct 29, 2024

zanieb commented Oct 29, 2024

zanieb commented Oct 29, 2024

Add a trampoline variant that just executes python #8637

Add a trampoline variant that just executes python #8637

Conversation

zanieb commented Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samypr100 commented Oct 28, 2024 • edited Loading

zanieb commented Oct 28, 2024

BurntSushi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zanieb commented Oct 29, 2024

zanieb commented Oct 29, 2024

zanieb commented Oct 29, 2024

Add a trampoline variant that just executes `python` #8637

Add a trampoline variant that just executes `python` #8637

zanieb commented Oct 28, 2024 •

edited

Loading

samypr100 commented Oct 28, 2024 •

edited

Loading