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 configuring the toolchain fetch strategy #4601

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

zanieb
Copy link
Member

@zanieb zanieb commented Jun 27, 2024

Adds a toolchain-fetch option alongside toolchain-preference with automatic (default) and manual values allowing automatic toolchain fetches to be disabled (replaces #4425). When manual, toolchains must be installed with uv toolchain install.

Note this was previously implemented with if-necessary, always, never variants but the interaction between this and toolchain-preference was too confusing. By reducing to a binary option, things should be clearer. The if-necessary behavior moved to toolchain-preference=installed. See #4601 (comment) and #4601 (comment)

@zanieb zanieb added configuration Settings and such preview Experimental behavior labels Jun 27, 2024
@zanieb zanieb force-pushed the zb/toolchain-fetch-strategy branch 2 times, most recently from 43e6040 to 6d8ae94 Compare June 27, 2024 21:17
@zanieb zanieb marked this pull request as ready for review June 27, 2024 21:32
crates/uv-cli/src/lib.rs Outdated Show resolved Hide resolved
@zanieb
Copy link
Member Author

zanieb commented Jun 27, 2024

I'm still not sure what our testing strategy is for toolchains since I don't want to fetch them in unit tests.

I tested various combinations of --toolchain-preference and --toolchain-fetch locally.

crates/uv-toolchain/src/discovery.rs Outdated Show resolved Hide resolved
crates/uv-cli/src/lib.rs Outdated Show resolved Hide resolved
crates/uv-cli/src/lib.rs Outdated Show resolved Hide resolved
crates/uv-toolchain/src/discovery.rs Outdated Show resolved Hide resolved
@zanieb zanieb force-pushed the zb/toolchain-fetch-strategy branch 4 times, most recently from 75e4ecf to 6ceeeca Compare July 1, 2024 21:38
@zanieb zanieb force-pushed the zb/toolchain-fetch-strategy branch from 6ceeeca to 5fed93b Compare July 2, 2024 01:47
@zanieb zanieb enabled auto-merge (squash) July 2, 2024 01:47
@zanieb zanieb merged commit 6799cc8 into main Jul 2, 2024
47 checks passed
@zanieb zanieb deleted the zb/toolchain-fetch-strategy branch July 2, 2024 01:54
zanieb added a commit that referenced this pull request Jul 2, 2024
I think `--toolchain-preference system` is sufficiently clear and
`--toolchain-preference prefer-system` is excessively verbose. This was
discussed in the original pull request at
#4424 but because we had a case for
preferring "installed managed" toolchains I was hesitant to change it.
Now that I've dropped that in #4601, I think we can drop the prefix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
configuration Settings and such preview Experimental behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants