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 multiple toolchains to be requested in uv toolchain install #4334

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Jun 14, 2024

Allows installation of multiple toolchains in a single invocation because I don't want to be limited to one! Most of the implementation for concurrent downloads ported from cargo dev fetch-python.

@zanieb zanieb added the preview Experimental behavior label Jun 14, 2024
@zanieb zanieb force-pushed the zb/toolchain-install-many branch from 7d30506 to ab7d5ae Compare June 15, 2024 00:54
crates/uv/src/commands/toolchain/install.rs Outdated Show resolved Hide resolved
crates/uv/src/commands/toolchain/install.rs Outdated Show resolved Hide resolved
@zanieb zanieb force-pushed the zb/toolchain-request-key branch 3 times, most recently from 35a2730 to 31a937a Compare June 17, 2024 16:22
@zanieb zanieb force-pushed the zb/toolchain-install-many branch 2 times, most recently from 7b18221 to 677b392 Compare June 17, 2024 16:30
@zanieb zanieb force-pushed the zb/toolchain-install-many branch 2 times, most recently from 278c5c0 to af9eeb1 Compare June 17, 2024 16:56
@zanieb zanieb requested a review from BurntSushi June 17, 2024 16:58
if targets.is_empty() {
writeln!(
printer.stderr(),
"A toolchain is already installed. Use `uv toolchain install <request>` to install a specific toolchain.",
Copy link
Member

Choose a reason for hiding this comment

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

I think the idea here is that if this message is printed, then I think it's guaranteed that some other "Found installed toolchain ..." message is printed above it, right?

Copy link
Member

Choose a reason for hiding this comment

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

Also, should this say, "all requested toolchains are already installed"? Instead of just saying only a single toolchain is already installed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that's the idea. The output is actually pretty reasonable with both messages, one is listing discovered toolchains and the other summarizes the result of the whole operation.

We only say this when the targets are empty, which means we're looking for any managed toolchain. This is a special case added in #4248

Base automatically changed from zb/toolchain-request-key to main June 17, 2024 18:11
@zanieb zanieb force-pushed the zb/toolchain-install-many branch from af9eeb1 to 25427db Compare June 17, 2024 18:16
@zanieb zanieb enabled auto-merge (squash) June 17, 2024 18:18
@zanieb zanieb merged commit fdcdc2c into main Jun 17, 2024
47 checks passed
@zanieb zanieb deleted the zb/toolchain-install-many branch June 17, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Experimental behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants