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

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

Open
chrivers opened this issue Apr 17, 2017 · 7 comments
Open
Milestone

Comments

@chrivers
Copy link

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
Copy link
Contributor

Diggsey commented Apr 17, 2017

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

  • Delete toolchains (~/.rustup)
  • Delete cargo caches (everything in ~/.cargo except bin)
  • Delete cargo-installed binaries (everything in ~/.cargo/bin except for rustup executables)
  • Delete rustup binaries (~/.cargo)

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
Copy link
Author

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
Copy link
Contributor

Diggsey commented Apr 17, 2017

  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
Copy link

gyscos commented May 5, 2017

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
Copy link
Contributor

Diggsey commented May 5, 2017

@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 Diggsey closed this as completed May 5, 2017
@Diggsey Diggsey reopened this May 5, 2017
@Diggsey
Copy link
Contributor

Diggsey commented May 5, 2017

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

@chrivers
Copy link
Author

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.

@djc djc added this to the 1.29.0 milestone Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants