rust-lang / rustup

The Rust toolchain installer
https://rust-lang.github.io/rustup/
Apache License 2.0
6.19k stars 893 forks source link

"rustup self uninstall" causes data loss, leaves behind only cargo binaries #1072

Open chrivers opened 7 years ago

chrivers commented 7 years ago

I just had a bizzare experience with rustup - unfortunately, one that lost me quite a few hours of work (which would have been many times more, without a backup).

Here's a rough sketch of what happened, as far as I can tell:

1) Install "stable" and "nightly" using rustup

2) rustup self update, to ensure newest

3) Update "stable" and "nightly" again, to ensure newest

4) I had a problem where ~/.cargo/bin was added to my ~/.zshrc, which gave me $PATH inclusion for the commands in interactive shells. However, I have some build scripts for various things that like to call cargo or rust as well, and these fail, since they're not running under interactive shells.

5) On my workstation, I have ~/bin in the default path, to allow users to install local binaries. This solves the problem, as long as cargo and rustc are symlinked to ~/bin.

6) To make it "just work", I symlinked ~/.cargo/bin to ~/bin, so rustup would effectively install in ~/bin.

Since I had some more problems, I finally decided to try the following commands:

$ rustup toolchain uninstall stable $ rustup toolchain uninstall nightly $ rustup self uninstall

To my shock, I see that my ~/bin folder has been all but completely destroyed. To provide some salt for the wound, the only files NOT removed, are the rustup ones:

$ ls -l ~/bin
total 67740
-rwxrwxr-x 1 chrivers chrivers 10649744 Apr 17 22:41 cargo
-rwxr-xr-x 6 chrivers chrivers  9785208 Apr 17 22:13 rls
-rwxr-xr-x 6 chrivers chrivers  9785208 Apr 17 22:13 rustc
-rwxr-xr-x 6 chrivers chrivers  9785208 Apr 17 22:13 rustdoc
-rwxr-xr-x 6 chrivers chrivers  9785208 Apr 17 22:13 rust-gdb
-rwxr-xr-x 6 chrivers chrivers  9785208 Apr 17 22:13 rust-lldb
-rwxr-xr-x 6 chrivers chrivers  9785208 Apr 17 22:13 rustup

Whiskey Tango Foxtrot.

The dirs .cargo and .rustup are gone, as are all my binaries and shell scripts. This is a pretty catastrophic result.

Diggsey commented 7 years ago

This is pretty much expected behaviour. Uninstall procedure is as follows:

In general, it's a really bad idea to symlink directories "owned" by other packages to ones you're going to store your own data in, and then to uninstall those packages. Data loss is pretty much guaranteed.

At best, we can try to be more defensive, eg. check if ~/.cargo/bin is a symlink, cancel the uninstall, or skip that part of uninstallation and show a warning.

chrivers commented 7 years ago

Respectfully, unless I misunderstand what you are saying, this is crazy behaviour.

From your description, step 3 is to delete ~/.cargo/bin, and step 4 is to delete ~/.cargo. Why not just delete ~/.cargo?

I am very surprised about your claim that data loss is "pretty much guaranteed". While I completely agree that users should not feel free to poke and prod around in a programs semi-private data, this flies in the face of all reasonable expectation.

But more importantly, it doesn't seem to make sense?

1) If the end step is to delete .cargo, why bother with .cargo/bin at all?

2) Suppose the user installs rustup, does "cargo install ripgrep" to get a cool new grep tool. Then they decides they've had enough rust for now, and run "rustup self uninstall". Now they lose ripgrep, but they keep rustup.

.. did I misunderstand something? Or it this really the expected result?

Diggsey commented 7 years ago
  1. Deleting ~/.cargo directly will fail on systems with mandatory file locking (such as windows) because the currently executing program (rustup) resides inside that directory. To avoid this, uninstallation is done in several steps, with the final step being to delete the running executable itself. That way only the final step need differ across operating systems (on windows self deletion is very complicated).

  2. They will lose both ripgrep and rustup. In your case the failure to delete the rustup binaries was unexpected, and was due to ~/.cargo/bin being a symlink. The linux implementation of the final step^ is to simply delete ~/.cargo since it doesn't need to care about file locking. However, the default behaviour of a directory deletion is not to recurse into symlinks, and hence the rustup binaries are missed. Since the only unexpected behaviour here was caused by the user messing with the installation, the only "fix" is to try to be more defensive about that.

gyscos commented 7 years ago

Regarding the brutal deletion of all binaries there, I thought cargo maintained a list of the installed binaries - couldn't that be used to only uninstall what was installed previously?

Diggsey commented 7 years ago

@gyscos That seems possible, we can parse the .crates.toml file for installed binaries and delete them one-by-one (we can't be sure there will be a working cargo to call out to).

This issue can track the addition of that feature.

Diggsey commented 7 years ago

@brson does this seem like a good solution to you?

chrivers commented 7 years ago

That would be a much better solution, yes. And as I said, I'd always argue for erring on the side of caution, when it comes to deleting files in the users home dir.

Is the precaution about not following symlinks implemented in the meantime? I think that sounds like a good, simple safeguard.