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

Add extra checks when installing Rust - display some warning messages, if needed #366

Merged
merged 3 commits into from
Nov 27, 2023

Conversation

andreibancioiu
Copy link
Contributor

@andreibancioiu andreibancioiu commented Nov 27, 2023

Display WARN messages if CARGO_HOME and RUSTUP_HOME are set before the installation:

https://rust-lang.github.io/rustup/installation/index.html#choosing-where-to-install

Unset the variables if installing rust with --overwrite.

@andreibancioiu andreibancioiu self-assigned this Nov 27, 2023
@@ -273,10 +273,13 @@ def is_installed(self, tag: str) -> bool:
return all(dependency is not None for dependency in dependencies)

def install(self, overwrite: bool) -> None:
self._check_install_env()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that if someone uses the --overwrite flag, the existing CARGO_HOME and RUSTUP_HOME should reset to the default location, as if rust was not previously installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, we should do that 🙌

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@andreibancioiu andreibancioiu merged commit 3068191 into main Nov 27, 2023
9 checks passed
@andreibancioiu andreibancioiu deleted the extra-warn-rust-install-11-27 branch November 27, 2023 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants