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

Add front-end implementation for conda-standalone uninstall subcommand #897

Merged
merged 26 commits into from
Dec 5, 2024

Conversation

marcoesters
Copy link
Contributor

@marcoesters marcoesters commented Nov 13, 2024

Description

Uninstalling distributions for conda currently only has incomplete solutions with files and broken auto-run/initializer scripts left behind (see #642, #588, and #572). A clean uninstallation is currently not available without manually running a series of commands that are still not 100% safe - e.g., conda init --reverse --all will remove initialization of conda, no matter what installation it is pointed to.

conda-standalone is implementing an uninstallation subcommand (see conda/conda-standalone#112) to alleviate these issues. This PR implements the front-end implementation of the uninstallation options for the Windows uninstaller (see screenshot).

The conda-standalone feature needs to be merged and uploaded first before merging this PR.

Screenshot 2024-11-12 at 11 28 26 AM

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Nov 13, 2024
@marcoesters marcoesters changed the title Uninstall standalone Add front-end implementation for conda-standalone uninstall subcommand Nov 13, 2024
@marcoesters marcoesters marked this pull request as ready for review November 25, 2024 22:03
@marcoesters marcoesters requested a review from a team as a code owner November 25, 2024 22:03
@marcoesters
Copy link
Contributor Author

I think this can be reviewed already, even though we have to wait for a new conda-standalone before we can fully test it.

constructor/winexe.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

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

The Windows changes LGTM. It's a lot of NSIS code so I'm assuming this thing runs correctly and if there are edge cases, I guess we'll hear about it.

I'm curious about the uninstallation story for Linux and macOS, though. In principle, users can just run conda.exe constructor uninstall --prefix <install path, right? Is it really that simple? I think we should document this in any case, maybe a new page under the How-to section? WDYT?

@marcoesters
Copy link
Contributor Author

The Windows changes LGTM. It's a lot of NSIS code so I'm assuming this thing runs correctly and if there are edge cases, I guess we'll hear about it.

I will re-run the CI when the new conda-standalone is published. Those tests have helped find issues in both the constructor and conda-standalone implementation. Edge cases are bound to come up. which is why I made this an opt-in feature via uninstall_with_conda_exe.

I'm curious about the uninstallation story for Linux and macOS, though. In principle, users can just run conda.exe constructor uninstall --prefix <install path, right? Is it really that simple? I think we should document this in any case, maybe a new page under the How-to section? WDYT?

It really is that simple. A new page in the how-to section sounds interesting. We could use this to set a standard on how these uninstallers ought to be implemented. We are implicitly doing this with the Windows front-end already.

@jaimergp
Copy link
Contributor

It really is that simple. A new page in the how-to section sounds interesting. We could use this to set a standard on how these uninstallers ought to be implemented. We are implicitly doing this with the Windows front-end already.

Let's write this page here and then I'll approve!

jaimergp
jaimergp previously approved these changes Dec 5, 2024
@marcoesters
Copy link
Contributor Author

@jaimergp, I had to make a change in the uninstallation behavior. The issue was that there were some .conda_trash files left in a subdirectory when running on the CI and my function didn't account for it.

That gave me two choices: implement a recursive search in NSIS (no) or try and delete the directory the way we do it with the old uninstaller method. I opted for the latter.

@marcoesters marcoesters requested a review from jaimergp December 5, 2024 21:29
@marcoesters marcoesters merged commit e462caf into conda:main Dec 5, 2024
18 checks passed
@marcoesters marcoesters deleted the uninstall-standalone branch December 5, 2024 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Status: 🏁 Done
Development

Successfully merging this pull request may close these issues.

3 participants