fix: stop deleting $CARGO_HOME during uninstall by Cloud0310 · Pull Request #4861 · rust-lang/rustup

Cloud0310 · 2026-05-18T07:21:32Z

fix #285
This PR fix the cargo bin nuke and the path removing possilble toutoc issue.
Here's the implementation details:

compare cargo bin dir content to rustup binary, if it's a link, remove it
then, we remove the rustup binary, on success, we then remove the cargo bin from path

This prevents rustup self uninstall from nuking out the $CARGO_HOME/bin and possible toctou problem of removing up the path.

Cloud0310 · 2026-05-19T13:42:17Z

I think I may need more tests against this PR, this includes:

$CARGO_HOME/bin with unrelated files
empty bin cleanup test

Should there be more tests? Please let me know @FranciscoTGouveia @rami3l .

Cloud0310 · 2026-05-20T18:20:13Z

I've add more tests about the PR here. Please review.

rami3l

LGTM modulo some tiny nits, nice work :D

View changes since this review

Avoid removing $CARGO_HOME when running uninstall cleanup. Combine all follow-up work for PR rust-lang#4861: - keep $CARGO_HOME/bin when it is non-empty - cover cargo bin cleanup behavior in uninstall tests - update uninstall cleanup comments and related docs - rename cleanup internals for clearer intent

djc

I think this PR needs a bunch of work, it seems to have a lot of changes together in a single commit that are pretty messy. It would be good if the commit history made it clear that there are a sequence of smaller changes being made.

View changes since this review

Avoid removing $CARGO_HOME when running uninstall cleanup. Combine all follow-up work for PR rust-lang#4861: - keep $CARGO_HOME/bin when it is non-empty - cover cargo bin cleanup behavior in uninstall tests - update uninstall cleanup comments and related docs - rename cleanup internals for clearer intent

rami3l · 2026-06-01T14:37:09Z

@Cloud0310 Since your PR needs more work, I've marked this PR as draft.

Please address or at least respond to all open threads before marking this PR as "ready for review" again, at which point the team will return for another round of review, thanks 🙏

rami3l

This is looking much better than before! I have a few final nits; address them in addition to the remaining ones and we should be good to go 🙏

View changes since this review

…odule

rami3l · 2026-06-08T18:23:29Z

+    // the running executable and on Windows can't be unlinked until the process exits.
+    // see: windows::{complete_windows_uninstall,clean_cargo_bin}
+    #[cfg(windows)]
+    windows::clean_cargo_bin(no_modify_path, process)?;


I have the impression that we are first adding this line in a previous commit, then removing and re-adding it here?

View changes since the review

rami3l · 2026-06-08T18:24:13Z

+/// 2. Remove cargo home except $CARGO_HOME/bin
+/// 3. Remove other tools in $CARGO_HOME/bin.
+/// 4. Remove rustup binary file and links to rustup.
+/// 5. Upon successfully removing $CARGO_HOME/bin, clean up $PATH.


You didn't mention the fact that it also removes $CARGO_HOME as the final step (in this commit).

View changes since the review

rami3l · 2026-06-08T18:25:06Z

        .filter_map(|entry| {
            let path = entry.unwrap().path();
            let name = path.file_name()?.to_str()?;
+            // On Windows, this binary is cleaned up on exit


This part should also be absorbed to the commit where the next line is introduced.

View changes since the review

Cloud0310 force-pushed the main branch from 64f3d84 to 1e3a7f7 Compare May 18, 2026 12:11

FranciscoTGouveia reviewed May 19, 2026

View reviewed changes

Comment thread tests/suite/cli_self_upd.rs

Cloud0310 marked this pull request as ready for review May 19, 2026 10:37

Cloud0310 force-pushed the main branch from 07d31c8 to b6195c7 Compare May 19, 2026 13:22

rami3l reviewed May 19, 2026

View reviewed changes

Comment thread src/cli/self_update.rs Outdated

Cloud0310 requested a review from FranciscoTGouveia May 20, 2026 03:55

Cloud0310 force-pushed the main branch from b6195c7 to 48acf08 Compare May 20, 2026 06:54

djc reviewed May 20, 2026

View reviewed changes

Comment thread src/cli/self_update/windows.rs Outdated

Comment thread src/cli/self_update/windows.rs Outdated

Comment thread src/cli/self_update/windows.rs Outdated

Comment thread src/cli/self_update.rs Outdated

Comment thread src/cli/self_update.rs Outdated

Cloud0310 force-pushed the main branch from 48acf08 to b509a72 Compare May 20, 2026 14:03

Cloud0310 force-pushed the main branch from 5a91816 to 905c540 Compare May 20, 2026 18:33

rami3l reviewed May 21, 2026

View reviewed changes

Comment thread src/cli/self_update/unix.rs Outdated

rami3l approved these changes May 21, 2026

View reviewed changes

Comment thread src/cli/self_update.rs Outdated

Comment thread tests/suite/cli_self_upd.rs Outdated

Comment thread tests/suite/cli_self_upd.rs

Cloud0310 force-pushed the main branch 2 times, most recently from 4a4880a to 971dffa Compare May 22, 2026 08:13

Cloud0310 force-pushed the main branch from 971dffa to b7bddd9 Compare May 22, 2026 08:26

Cloud0310 force-pushed the main branch from b7bddd9 to 01cfd07 Compare May 22, 2026 08:45

This comment has been minimized.

Sign in to view

Cloud0310 force-pushed the main branch from 01cfd07 to 61b3e49 Compare May 22, 2026 09:00

djc reviewed May 22, 2026

View reviewed changes

Comment thread src/cli/self_update/unix.rs Outdated

Comment thread src/cli/self_update/windows.rs Outdated

Comment thread src/cli/self_update/windows.rs Outdated

Comment thread src/cli/self_update/windows.rs

Cloud0310 mentioned this pull request May 22, 2026

Update contribute docs with cross platform code development guide #4864

Closed

rami3l mentioned this pull request May 22, 2026

Clarify in the dev guide what is expected from a PR #4865

Closed

Cloud0310 force-pushed the main branch from 61b3e49 to a80abc9 Compare May 22, 2026 16:28

Cloud0310 force-pushed the main branch 7 times, most recently from 2acb9ec to 1c41abe Compare June 5, 2026 16:09

refactor: rename delete_rustup_and_cargo_home to clean_cargo_bin

754b48d

Cloud0310 force-pushed the main branch 2 times, most recently from 781981d to 197f611 Compare June 6, 2026 17:04

rami3l approved these changes Jun 7, 2026

View reviewed changes

Comment thread src/cli/self_update.rs Outdated

Comment thread src/cli/self_update/unix.rs Outdated

Cloud0310 force-pushed the main branch 4 times, most recently from dac53d8 to 3c0d7e4 Compare June 7, 2026 15:43

refactor(cli/self-update): move unix::clean_cargo_bin() to parent m…

f2bfa78

…odule

Cloud0310 force-pushed the main branch 2 times, most recently from af1c3ac to 60c433c Compare June 8, 2026 18:02

Cloud0310 marked this pull request as ready for review June 8, 2026 18:04

Cloud0310 changed the title ~~fix!: stop deleting $CARGO_HOME during uninstall~~ fix: stop deleting $CARGO_HOME during uninstall Jun 8, 2026

rami3l approved these changes Jun 8, 2026

View reviewed changes

Cloud0310 added 7 commits June 9, 2026 02:38

refactor: pass PATH cleanup to clean_cargo_bin

0f086b3

doc: add uninstall process overall docstring

92964af

fix: preserve CARGO_HOME during uninstall

dd55cc5

fix: remove rustup proxy links during uninstall

48307ed

test: cover cargo bin removal during uninstall

ed751b4

test: cover cargo bin PATH cleanup during uninstall

64736cc

test: cover Windows cargo bin cleanup during uninstall

ca2bba6

Cloud0310 force-pushed the main branch from 60c433c to ca2bba6 Compare June 8, 2026 18:43

Conversation

Cloud0310 commented May 18, 2026

Uh oh!

Uh oh!

Cloud0310 commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Cloud0310 commented May 20, 2026

Uh oh!

Uh oh!

rami3l left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

djc left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rami3l commented Jun 1, 2026

Uh oh!

rami3l left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rami3l Jun 8, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rami3l Jun 8, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rami3l Jun 8, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Cloud0310 commented May 19, 2026 •

edited

Loading

rami3l left a comment •

edited by rustbot

Loading

djc left a comment •

edited by rustbot

Loading

rami3l left a comment •

edited by rustbot

Loading

rami3l Jun 8, 2026 •

edited by rustbot

Loading

rami3l Jun 8, 2026 •

edited by rustbot

Loading

rami3l Jun 8, 2026 •

edited by rustbot

Loading