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

Update uninstall methods for single user #8675

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

DynamicApproach
Copy link

Motivation

Updates documentation and adds an uninstall script for the single user installation.

Context

Documentation for single user installation seems to have fallen behind - This tries to update it and add a script to uninstall it when needed. Previously documentation only had a non-sudo command that requires permissions and lacked all files needed to be removed.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

If you have a [single-user installation](./installing-binary.md#single-user-installation) of Nix, uninstall it by running:
### Alternative:
#### Step 1:
Run the script located at `scripts/uninstall-single-user.sh`
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk Jul 9, 2023

Choose a reason for hiding this comment

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

Imagine you're reading this as a first time user that got in trouble with installation or just wants to bail out. Where is that location? What exactly would you have to do to run that script? Currently there is no indication at all, and you "just have to know" that you mean something in the GitHub repository. I insist we avoid this "just have to know" situation at all cost. It got Nix the reputation of being badly documented and inscrutable.

Usually at this point I'd suggest to make it a link to the repo, but that script must match the current Nix version to be sure it will do the right thing. Also it would be quite a few additional steps to run it off the repo. What about we include it as a command in the binary? We could use that opportunity to take inventory of all the files Nix touches, in one central location.

Copy link
Author

Choose a reason for hiding this comment

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

That's a really good point, thanks! I definitely agree with you, and I'd like to help Nix be more consistent; swapping this to part of the binary makes sense and I can get to work on that.

Copy link
Author

Choose a reason for hiding this comment

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

@fricklerhandwerk Where should I put it in the dir structure? I don't typically develop in C++ but I'm learning as I go 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll discuss among maintainers how to proceed here in general.

@fricklerhandwerk
Copy link
Contributor

Triaged in Nix team meeting 2023-07-10:

  • @roberth @Ericson2314: agree on a "cumulative" uninstaller, taking into account all files that might have been touched in the past
  • @edolstra: the Determinate Systems installer already does all of that, it seems like duplicate work
  • @fricklerhandwerk: question is which path to pursue
    • quick hack along the lines of this PR (short-term)
    • adopt DetSys installer (mid-term)
    • make Nix more self-contained (long-term)

@DynamicApproach does the DetSys installer address your problem? If not, it could be a to-do item for the authors.
You could also join the installer working group and address the issues directly. The group is still meeting regularly, just no notes were published recently.

@DynamicApproach
Copy link
Author

DynamicApproach commented Jul 13, 2023

@fricklerhandwerk Thanks for the update! I'm not an expert on this, so definitely take it with a grain of salt!

does the DetSys installer address your problem? If not, it could be a to-do item for the authors. You could also join the installer working group and address the issues directly. The group is still meeting regularly, just no notes were published recently.

This is a mixed bag, while it does have an uninstall action, this also requires the user to install it? Seems like it could be confusing for new users to install something, just to uninstall two things.
While this could be solved through DetSys, I personally think Nix should have something built-in and aim for a long term approach of being self contained, even if it does start with an initial quick-hack solution. The other part of the reasoning is for automation purposes - it's always nice to have an uninstall command when you need it without the extra effort/time.

I'll definitely see how I can help out there! Thanks for the info!!

@abathur
Copy link
Member

abathur commented Jul 13, 2023

@fricklerhandwerk: question is which path to pursue

  • quick hack along the lines of this PR (short-term)
  • adopt DetSys installer (mid-term)
  • make Nix more self-contained (long-term)

Incremental work is underway to get an experimental installer (based on the DetSys installer) hosted under nixos.org. I'm not sure how quickly this will manifest, but as soon as it does I think we can probably start suggesting that installer's uninstall option in support contexts.

I don't (personally) think the time to review, test, and provide ongoing troubleshooting + support likely to accompany a quick hack is a good use of dev time with that on the horizon.

Aside: the part of this that halts on the presence of /nix would also directly conflict with the effort to partially restore idempotence in #7603 (though I'm not sure if that effort will succeed).

@DynamicApproach
Copy link
Author

DynamicApproach commented Jul 13, 2023

Aside: the part of this that halts on the presence of /nix would also directly conflict with the effort to partially restore idempotence in #7603 (though I'm not sure if that effort will succeed).

I can either remove this part or, if it's used, it could be removed as part of #7603 itself?

Just my opinion but while the incremental work is underway, it's not a bad idea to have some form of basic working script and documentation.

@abathur Is there a way I can contribute to the work being done on it, if so, can you point me in the right direction?

@abathur
Copy link
Member

abathur commented Jul 13, 2023

while it does have an uninstall action, this also requires the user to install it? Seems like it could be confusing for new users to install something, just to uninstall two things.

You don't, strictly speaking, have to install it to run the uninstall action. But yes, you do currently have to have a receipt from a successful run of the detsys installer for it to do anything. From an end-user perspective it'd be ideal to support uninstalls of previous vintages (as far as is feasible/reasonable) with a single mechanism.

(I think I remember them saying they intend to look at this, but I might be misremembering. It'll come up soon regardless.)

@abathur
Copy link
Member

abathur commented Jul 13, 2023

Is there a way I can contribute... ?

Message me on Matrix?

@fricklerhandwerk
Copy link
Contributor

it's not a bad idea to have some form of basic working script and documentation.

Just making correct and complete documentation would be the best quick hack we can have. No need to be fancy, just diligently collect everything that was touched and what to do about it. I'd gladly review such PRs.

@DynamicApproach
Copy link
Author

it's not a bad idea to have some form of basic working script and documentation.

Just making correct and complete documentation would be the best quick hack we can have. No need to be fancy, just diligently collect everything that was touched and what to do about it. I'd gladly review such PRs.

@fricklerhandwerk Is that how I should proceed?

I can change it to be complete manual documentation if that's preferred

@fricklerhandwerk
Copy link
Contributor

@DynamicApproach for this PR it would make sense. For fixing the installer, see previous comments.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-07-10-nix-team-meeting-minutes-70/30471/1

@fricklerhandwerk fricklerhandwerk self-assigned this Jul 21, 2023
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

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

@DynamicApproach sorry for the long delay with my review. There are some technical nitpics, but my primary change request would be that you make sure the manual matches the code as close as possible. I haven't checked this, but I'm almost certain there must be more files: at least another shell (IIRC fish), the XDG files, possibly more. It's fine if you don't do all aspects of it because that's additional work, but for the stuff you decide to cover (such as the two shells) it should ideally be complete.

Comment on lines +3 to +12
## Sections
- [Single User](#single-user)
- [Manual Method](#manual-method)
- [Step 1: Restore Modified Files (if applicable)](#step-2-restore-modified-files-if-applicable)
- [Step 2: Delete Nix Files](#step-4-delete-nix-files)
- [Multi User](#multi-user)
- [Linux](#linux)
- [macOS](#macos)


Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why we'd want that, but let's not create maintenance burden and opportunities for human error for the future. I'd rather review one PR adding automation for that than deal with broken links or missing TOC entries repeatedly.

Suggested change
## Sections
- [Single User](#single-user)
- [Manual Method](#manual-method)
- [Step 1: Restore Modified Files (if applicable)](#step-2-restore-modified-files-if-applicable)
- [Step 2: Delete Nix Files](#step-4-delete-nix-files)
- [Multi User](#multi-user)
- [Linux](#linux)
- [macOS](#macos)

@@ -96,6 +96,7 @@ is_os_darwin() {
fi
}


Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

sudo cp ~/.bash_profile.backup-before-nix ~/.bash_profile
```

Note: After restoring the file, you may need to close and reopen any bash terminal sessions to ensure they are using the restored configurations.
Copy link
Contributor

Choose a reason for hiding this comment

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

You sure have to. Same for the other note.

Suggested change
Note: After restoring the file, you may need to close and reopen any bash terminal sessions to ensure they are using the restored configurations.
> **Note**
>
> After restoring the file, you have to close and reopen any Bash sessions to ensure they are using the restored configurations.

We use this type of highlighting for admonitions everywhere

Comment on lines +24 to +25
sudo cp /etc/bash.bashrc.backup-before-nix /etc/bash.bashrc
sudo cp ~/.bash_profile.backup-before-nix ~/.bash_profile
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not

Suggested change
sudo cp /etc/bash.bashrc.backup-before-nix /etc/bash.bashrc
sudo cp ~/.bash_profile.backup-before-nix ~/.bash_profile
sudo mv /etc/bash.bashrc.backup-before-nix /etc/bash.bashrc
sudo mv ~/.bash_profile.backup-before-nix ~/.bash_profile

Because if those files exist we're done; otherwise error showing they don't


If you have a [single-user installation](./installing-binary.md#single-user-installation) of Nix, uninstall it by running:
#### Step 1: Restore Modified Files (if applicable)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#### Step 1: Restore Modified Files (if applicable)
### Step 1: Restore Modified Files

If they don't exist it's a no-op anyway


1. Nix may have modified certain files on your system. If you have created backup copies of these files before installing Nix, you can restore them. Here are the instructions for bash and zsh files:

- Bash: Nix may have modified the `/etc/bash.bashrc` or `~/.bash_profile` file. If you have created a backup of this file before installing Nix, you can restore it using the following commands:
Copy link
Contributor

Choose a reason for hiding this comment

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

Dedent. I don't think Nix may have done it, pretty sure it just did. I doubt this introductory blurb is even needed.

Note: After restoring the file, you may need to close and reopen any zsh terminal sessions to ensure they are using the restored configurations.


#### Step 2: Delete Nix Files
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#### Step 2: Delete Nix Files
### Step 2: Delete Nix Files


#### Step 2: Delete Nix Files

1. Run the following command in a terminal to delete the files that Nix added to your system:
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't have to be a list, dedent.

sudo rm -rf "/etc/nix" "$HOME/.nix-profile" "$HOME/.nix-defexpr" "$HOME/.nix-channels" "$HOME/.cache/nix" "$NIX_ROOT" "/nix"
```

You have successfully uninstalled Nix from your system. Remember to double-check the commands before executing them and ensure that you have the necessary permissions to perform the uninstallation steps.
Copy link
Contributor

Choose a reason for hiding this comment

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

That's a bit too much promised. The reminder should be a warning admonition before the commands.


You have successfully uninstalled Nix from your system. Remember to double-check the commands before executing them and ensure that you have the necessary permissions to perform the uninstallation steps.

If you ever need to use Nix again in the future, you can reinstall it following the installation instructions provided by the Nix documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

Link to those.

@DynamicApproach
Copy link
Author

@fricklerhandwerk Thanks for taking a look; I'll get on it soon and make edits as needed to update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🏁 Review
Development

Successfully merging this pull request may close these issues.

4 participants