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.

Sign up for GitHub

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

Add a trampoline variant that just executes python #8637

Merged
merged 8 commits into from
Oct 29, 2024
Merged

Add a trampoline variant that just executes python #8637

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9f7fd2e
Add a trampoline variant that executes `python`
zanieb Oct 28, 2024
7909dcc
Bump rust toolchain version
zanieb Oct 28, 2024
357431b
Add `uv-trampoline-builder` crate for shared trampoline launcher cons…
zanieb Oct 28, 2024
c7fdb95
Update build instructions
zanieb Oct 28, 2024
f446582
Move tests to new crate
zanieb Oct 28, 2024
c29b9d9
Bump allowed launcher size
zanieb Oct 28, 2024
b420b40
Rebuild bundled trampolines
zanieb Oct 28, 2024
cbb90ba
Separate size tests
zanieb Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions crates/uv-trampoline-builder/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,14 @@ mod test {
#[test]
#[cfg(all(windows, target_arch = "x86"))]
fn test_launchers_are_small() {
// At time of writing, they are 45kb~ bytes.
// At time of writing, they are ~80kb.
assert!(
super::LAUNCHER_I686_GUI.len() < 45 * 1024,
super::LAUNCHER_I686_GUI.len() < 80 * 1024,
Copy link
Member

Choose a reason for hiding this comment

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

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.)

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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).

Copy link
Member Author

Choose a reason for hiding this comment

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

See #8637 (comment) cbb90ba

"GUI launcher: {}",
super::LAUNCHER_I686_GUI.len()
);
assert!(
super::LAUNCHER_I686_CONSOLE.len() < 45 * 1024,
super::LAUNCHER_I686_CONSOLE.len() < 80 * 1024,
"CLI launcher: {}",
super::LAUNCHER_I686_CONSOLE.len()
);
Expand All @@ -213,14 +213,14 @@ mod test {
#[test]
#[cfg(all(windows, target_arch = "x86_64"))]
fn test_launchers_are_small() {
// At time of writing, they are 45kb~ bytes.
// At time of writing, they are ~80kb.
assert!(
super::LAUNCHER_X86_64_GUI.len() < 45 * 1024,
super::LAUNCHER_X86_64_GUI.len() < 80 * 1024,
"GUI launcher: {}",
super::LAUNCHER_X86_64_GUI.len()
);
assert!(
super::LAUNCHER_X86_64_CONSOLE.len() < 45 * 1024,
super::LAUNCHER_X86_64_CONSOLE.len() < 80 * 1024,
"CLI launcher: {}",
super::LAUNCHER_X86_64_CONSOLE.len()
);
Expand All @@ -229,14 +229,14 @@ mod test {
#[test]
#[cfg(all(windows, target_arch = "aarch64"))]
fn test_launchers_are_small() {
// At time of writing, they are 45kb~ bytes.
// At time of writing, they are ~80kb.
assert!(
super::LAUNCHER_AARCH64_GUI.len() < 45 * 1024,
super::LAUNCHER_AARCH64_GUI.len() < 80 * 1024,
"GUI launcher: {}",
super::LAUNCHER_AARCH64_GUI.len()
);
assert!(
super::LAUNCHER_AARCH64_CONSOLE.len() < 45 * 1024,
super::LAUNCHER_AARCH64_CONSOLE.len() < 80 * 1024,
"CLI launcher: {}",
super::LAUNCHER_AARCH64_CONSOLE.len()
);
Expand Down
Loading