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

RFC: Feat: add autoremove #578

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Lunarequest
Copy link

@Lunarequest Lunarequest commented Nov 19, 2024

This is a request for comment on a zypper autoremove command. As a system in updated during its life cycle packages can get left behind. While zypper has rm --cleap-deps and dup --remove-orphaned both have short comings. rm only handles the package passed to it and removing orphans can remove packages a user explicitly built and installed themselves.

To bring zypper inline with the wider linux package manager system(dnf, apt/apt-get) I added a zypper autoremove command. This is code does require reworking of some parts but as it stands this will clean up unneeded and orphaned packages based on flags passed.

Thanks @janvhs who helped debug some issues with the code

@Lunarequest Lunarequest force-pushed the autoremove branch 2 times, most recently from 45b6150 to fc190a3 Compare November 25, 2024 10:09
Co-authored-by: janvhs <[email protected]>
@afaerber
Copy link
Member

afaerber commented Dec 3, 2024

It looks like RCF in the title meant RFC? If you don't want it to be merged yet, you should mark it as draft, which prevents that. Otherwise just remove RFC from title and description, in case maintainers don't require changes and would like to merge it.

@Lunarequest Lunarequest changed the title RCF: Feat: add autoremove RFC: Feat: add autoremove Dec 3, 2024
@Lunarequest
Copy link
Author

It looks like RCF in the title meant RFC? If you don't want it to be merged yet, you should mark it as draft, which prevents that. Otherwise just remove RFC from title and description, in case maintainers don't require changes and would like to merge it.

ah thanks I didn't realize that I typoed RFC, I'll move it to a draft for now, I believe it should be fine to just merge it, in the current state.

@Lunarequest Lunarequest marked this pull request as draft December 3, 2024 15:52
Copy link
Member

@afaerber afaerber left a comment

Choose a reason for hiding this comment

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

Appreciate this Hackweek project!
Some minor nits pointed out inline.

src/commands/autoremove.cc Outdated Show resolved Hide resolved
src/commands/autoremove.cc Outdated Show resolved Hide resolved
src/commands/autoremove.cc Outdated Show resolved Hide resolved
src/Command.cc Outdated Show resolved Hide resolved
src/commands/autoremove.h Outdated Show resolved Hide resolved
src/global-settings.h Outdated Show resolved Hide resolved
src/commands/autoremove.cc Outdated Show resolved Hide resolved
src/commands/autoremove.cc Outdated Show resolved Hide resolved
@mlandres
Copy link
Member

mlandres commented Dec 4, 2024

In fact a rm --cleap-deps without arguments is WIP together with commands to allow manipulating the auto-installed status of packages. The later is needed to be able to tune the clean-deps result because no user-requested packages should be removed here. Also --cleap-deps in install/update commands to clean in the dependency tree of update packages. A dedicated autoremove command should not be needed or become just an alias for rm --cleap-deps.

The kind of downside of your approach is that it is zypper only and that it's not under direct control of the resolver. I.e. you pass explicit remove requests for packages based on a previous computation rather than asking the resolver itself to compute the job. Implemented in libzypp, the job will become available to YAST and PK as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants