-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
A minimal build backend for uv: uv_build #11446
base: konsti/refactor-build-binaries
Are you sure you want to change the base?
A minimal build backend for uv: uv_build #11446
Conversation
5aadae4
to
6123fe0
Compare
72875c8
to
6fff5b2
Compare
We only support `uv_build` as build backend to avoid the duplication and confusion about configuration.
6fff5b2
to
b62aa08
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looked over everything but the python
Cargo.toml
Outdated
# Profile for uv-build | ||
[profile.minimal-size] | ||
inherits = "release" | ||
opt-level = "z" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
panic = "abort" may also reduce size if we're desperate for every byte: https://doc.rust-lang.org/cargo/reference/profiles.html#panic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably worth at least checking how much it saves us
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added it for, it gets us down to 1217927 bytes on linux. I'm not sure with we want to deploy that, we've had very few panics in the past but if the happen the panic message is the only indication we get.
crates/uv-build/src/main.rs
Outdated
@@ -0,0 +1,92 @@ | |||
#![allow(clippy::print_stdout)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love to have a comment here explaining why we are doing this. just to produce an anyhow error instead of panic when stdout is closed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've mostly done it for consistency with writeln!(printer.stdout(), ...)
we're using everywhere else, i don't feel strongly about.
[build-system] | ||
requires = ["maturin>=1.0,<2.0"] | ||
build-backend = "maturin" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't know why but this is incredibly funny to me (it makes sense)
[bans] | ||
multiple-versions = "allow" | ||
deny = [ | ||
{ crate = "rustls", reason = "The build backend does not need network access" }, | ||
{ crate = "openssl", reason = "The build backend does not need network access" }, | ||
{ crate = "reqwest", reason = "The build backend does not need network access" }, | ||
{ crate = "schemars", reason = "JSON Schema generation is a development feature, not a runtime feature" }, | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rad
# uv-build | ||
- name: "Build wheels uv-build - x86_64" | ||
uses: PyO3/maturin-action@v1 | ||
with: | ||
target: x86_64 | ||
args: --profile minimal-size --locked --out crates/uv-build/dist -m crates/uv-build/Cargo.toml | ||
- name: "Upload wheels uv-build" | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: wheels_uv_build-macos-x86_64 | ||
path: crates/uv-build/dist | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I was gonna say this will slow down releases a lot if it's serial but in theory this thing should build super fast so it's fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's 22s on a fresh target directory for me locally. I had initially put them in succession to reuse the target dir, but that mostly doesn't apply anymore with the different opt mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah basically nothing to share
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, to configure backend, we should use tool.uv.build-backend
, but since uv_build
is now standalone package could expect it to be changed?
IMO, tool.uv.build
or tool.uv-build
is closer to uv_build
, the package name.
if os.environ.get("UV_PREVIEW"): | ||
__all__ = [ | ||
"find_uv_bin", | ||
# PEP 517 hook `build_sdist`. | ||
"build_sdist", | ||
# PEP 517 hook `build_wheel`. | ||
"build_wheel", | ||
# PEP 660 hook `build_editable`. | ||
"build_editable", | ||
# PEP 517 hook `get_requires_for_build_sdist`. | ||
"get_requires_for_build_sdist", | ||
# PEP 517 hook `get_requires_for_build_wheel`. | ||
"get_requires_for_build_wheel", | ||
# PEP 517 hook `prepare_metadata_for_build_wheel`. | ||
"prepare_metadata_for_build_wheel", | ||
# PEP 660 hook `get_requires_for_build_editable`. | ||
"get_requires_for_build_editable", | ||
# PEP 660 hook `prepare_metadata_for_build_editable`. | ||
"prepare_metadata_for_build_editable", | ||
] | ||
else: | ||
__all__ = ["find_uv_bin"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider either remove or change the message to use uv_build
instead.
Lines 33 to 46 in a4bd73f
def __getattr__(attr_name: str) -> object: | |
if attr_name in { | |
"build_sdist", | |
"build_wheel", | |
"build_editable", | |
"get_requires_for_build_sdist", | |
"get_requires_for_build_wheel", | |
"prepare_metadata_for_build_wheel", | |
"get_requires_for_build_editable", | |
"prepare_metadata_for_build_editable", | |
}: | |
err = f"Using `uv.{attr_name}` is not allowed. The uv build backend requires preview mode to be enabled, e.g., via the `UV_PREVIEW=1` environment variable." | |
raise AttributeError(err) | |
uv itself is a large package with many dependencies and lots of features. To build a package using the uv build backend, you shouldn't have to download and install the entirety of uv. For platform where we don't provide wheels, it should be possible and fast to compile the uv build backend. To that end, we're introducing a python package that contains a trimmed down version of uv that only contains the build backend, with a minimal dependency tree in rust.
The
uv_build
package is publish from CI just like uv itself. It is part of the workspace, but has much less dependencies for its own binary. We're using cargo deny to enforce that the network stack is not part of the dependencies. A new build profile ensure we're getting the minimum possible binary size for a rust binary.