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

Fix Rye not using user-chosen toolchain as default during installation #1054

Merged
merged 2 commits into from
Apr 27, 2024

Conversation

pjdon
Copy link
Contributor

@pjdon pjdon commented Apr 27, 2024

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

@charliermarsh
Copy link
Member

Thanks, this makes sense. I just tweaked the code a bit to respect the RYE_TOOLCHAIN_VERSION if it is set. I could see an argument either way... but I think the set env var overriding a prompted "default" version is reasonable.

@charliermarsh charliermarsh added the bug Something isn't working label Apr 27, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) April 27, 2024 12:04
@charliermarsh charliermarsh merged commit fd4f8ca into astral-sh:main Apr 27, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rye doesn't respect the chosen toolchain version during installation
2 participants