Skip to content

Commit

Permalink
Fix Rye not using user-chosen toolchain as default during installation (
Browse files Browse the repository at this point in the history
#1054)

Fixes #1024

Currently:
* When installation calls `ensure_self_venv_with_toolchain` to bootstrap
rye internals it would pass the `toolchain_version_request` variable as
the version to install
* If the `RYE_TOOLCHAIN_VERSION` env var is not set, then the value of
`toolchain_version_request` remains None
* During installation, `prompt_for_default_toolchain` is called to
prompt the user for a version to select, asking the user `"Which version
of Python should be used as default toolchain?"`
* This function stores the user's selection in the `config_doc` TOML
configuration and returns nothing
* The user's toolchain version selection is thus never passed to the
internals bootstrapping call

Changes:
* This fix makes `toolchain_version_request` mutable so it can be
updated by the user's input
* Function `prompt_for_default_toolchain` returns the resolved version
that the user wants, which may be the input they typed or the default
value
* The result is stored in `toolchain_version_request` so it can be used
during the internals bootstrapping

---------

Co-authored-by: Charlie Marsh <[email protected]>
  • Loading branch information
pjdon and charliermarsh authored Apr 27, 2024
1 parent 9a01fb1 commit fd4f8ca
Showing 1 changed file with 9 additions and 4 deletions.
13 changes: 9 additions & 4 deletions rye/src/cli/rye.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ fn perform_install(

// Also pick up the target version from the RYE_TOOLCHAIN_VERSION
// environment variable.
let toolchain_version_request = match toolchain_version {
let mut toolchain_version_request = match toolchain_version {
Some(version) => Some(version),
None => match env::var("RYE_TOOLCHAIN_VERSION") {
Ok(val) => Some(val.parse()?),
Expand Down Expand Up @@ -624,12 +624,17 @@ fn perform_install(
// can fill in the default.
if !matches!(mode, InstallMode::NoPrompts) {
if toolchain_path.is_none() {
prompt_for_default_toolchain(
// Prompt the user for the default toolchain version.
let user_version_request = prompt_for_default_toolchain(
toolchain_version_request
.clone()
.unwrap_or(SELF_PYTHON_TARGET_VERSION),
config_doc,
)?;

// If the user has selected a toolchain version, we should use that as the default,
// unless the `RYE_TOOLCHAIN_VERSION` environment variable is set.
toolchain_version_request = toolchain_version_request.or(Some(user_version_request));
} else {
prompt_for_toolchain_later = true;
}
Expand Down Expand Up @@ -792,7 +797,7 @@ fn add_rye_to_path(mode: &InstallMode, shims: &Path, ask: bool) -> Result<(), Er
fn prompt_for_default_toolchain(
default_toolchain: PythonVersionRequest,
config_doc: &mut toml_edit::DocumentMut,
) -> Result<(), Error> {
) -> Result<PythonVersionRequest, Error> {
let choice = dialoguer::Input::with_theme(tui_theme())
.with_prompt("Which version of Python should be used as default toolchain?")
.default(default_toolchain.clone())
Expand All @@ -808,7 +813,7 @@ fn prompt_for_default_toolchain(
})
.interact_text()?;
toml::ensure_table(config_doc, "default")["toolchain"] = toml_edit::value(choice.to_string());
Ok(())
Ok(choice)
}

pub fn auto_self_install() -> Result<bool, Error> {
Expand Down

0 comments on commit fd4f8ca

Please sign in to comment.