Conversation
zanieb
added
cli
|
It turns out cargo-dist supports aliases! but I don't think that's actually what we want here, we have more control this way. |
|
I think we'd follow this with some special-casing to the help menu to display |
|
This is great, but one early concern: will this double the size of the wheel, by shipping two separate binaries that are mostly identical? |
|
@charliermarsh great question — ideally we'd make this binary teeeenie tiny. I'll investigate that but it sounds like I'll need to duplicate a lot more code? |
|
Mm yeah each of these is going to be large... Maybe a cargo-dist alias is the way to go. |
|
@zanieb can't Edit: Or, potentially, an alias, and simply detect |
|
Yeah I think we're going to need to do something like that. I think the binary size increase in unacceptable for us and there's no reason for
|
|
Alright, this is nice and spicy. |
| let result = run(); | ||
| match result { | ||
| // Fail with 2 if the status cannot be cast to an exit code | ||
| Ok(status) => u8::try_from(status.code().unwrap_or(2)).unwrap_or(2).into(), |
There was a problem hiding this comment.
This feels a little dubious, but is probably fine in practice. The documentation suggests the status code is just truncated to a u8 on Unix anyway? Definitely open to alternative approaches here.
| let uv = bin.join("uv"); | ||
| let args = ["tool", "run"] |
There was a problem hiding this comment.
In the future, this might invoke a hidden uv x so we can special case the help menu and such?
charliermarsh
left a comment
charliermarsh
left a comment
There was a problem hiding this comment.
How did you think about the tradeoffs between this approach and using a symlink?
I'm actually shocked that this is 456 KB, so big! I wonder if we can strip it? https://github.com/johnthagen/min-sized-rust?tab=readme-ov-file#strip-symbols-from-binary
On unix definitely, a lot of binaries rely on moonlighting as different binaries, and i found it to also work on windows. Given that cargo-dist supports them, can we build on their experience? |
|
cargo-dist uses various methods depending on the distribution target, not just a symbolic link. I worry about having a different implementation across platforms, the complexity of handling this as a distribution special-case, and the loss of control. Here we have a single consistent implementation that we can invoke via I think that we can and probably should explore alternative approaches, but this seems like the lower-risk approach to start with. |
|
I can't find an obvious way to strip just one of the binaries. |
This is pulled out of #4632 — a user noted that it would be useful to use the `uv` crate from Rust. This makes it way easier to invoke `uv` from Rust with arbitrary arguments as well as use various functionality in the `uv` crate. Note this is no longer needed for #4632 and is not particularly urgent.
4 participants
Closes #4476
Originally, this used the changes in #4642 to invoke
main()from auvxbinary. This had the benefit ofuvxbeing entirely standalone at the cost of doubling our artifact size. We think that's the incorrect trade-off.Instead, we assume
uvxis always next touvand create a tiny binary (<1MB) that invokesuvin a child process. This seems preferable to acargo-distalias because we have more control over it. This binary should "just work" for all of our cargo-dist distributions and installers, but we'll need to add a new entry point for our PyPI distribution. I'll probably tackle support there separately?This includes some small overhead:
I presume there may be some other downsides to a child process? The wrapper is a little awkward. We could consider
execvbut this is complicated across platforms. An example implementation of that over in monotrail.