fix: stop remove $CARGO_HOME on uninstallation#4861
Conversation
|
I think I may need more tests against this PR, this includes:
Should there be more tests? Please let me know @FranciscoTGouveia @rami3l . |
| pub(crate) fn delete_rustup_and_cargo_home(process: &Process) -> Result<()> { | ||
| let cargo_home = process.cargo_home()?; | ||
| utils::remove_dir("cargo_home", &cargo_home) | ||
| pub(crate) fn remove_rustup_executable(process: &Process, no_modify_path: bool) -> Result<()> { |
There was a problem hiding this comment.
Maybe delete_rustup_and_bin_dir() would be a better name for this, wdyt?
There was a problem hiding this comment.
Reasonable to me, I guess we may even need to add extra comments for this function?
It's actually does:
- remove executable
- cleanup
$CARGO_HOME/binon empty - cleanup
$PATH
So, I propose the new name as delete_exe_and_cargo_bin?
And we should add comments for this function for clarification.
There was a problem hiding this comment.
I may be overlooking something, but $CARGO_HOME/bin will never be empty.
The rustup binary will always be there, right?
There was a problem hiding this comment.
@FranciscoTGouveia The original intention at least is that on Unix we should remove the rustup shims in one go; on Windows we remove all the rustup shims except the potentially current running binary which then gets removed in another process.
When the other review comments are addressed I think we can do a final round of check about this.
| .spawn() | ||
| .context(CliError::WindowsUninstallMadness)?; | ||
|
|
||
| // clean up PATH and dir |
There was a problem hiding this comment.
Nit: Maybe extract this as a helper function?
fix rust-lang#285 This contains: - initial logic for detecting $CARGO_HOME/bin is empty - refactor`delete_rustup_and_cargo_home` to `delete_rustup_bin`. TODOs: - add further test
4 participants
fix #285
This PR fix the cargo bin nuke and the path removing possilble toutoc issue.
Here's the implementation details:
This prevents
rustup self uninstallfrom nuking out the$CARGO_HOME/binand possible toctou problem of removing up the path.