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 back hardcoded self-protection of dnf #1621

Conversation

evan-goode
Copy link
Member

Now that obsoletion of protected packages is allowed (#1610), it's best to hardcode the protection of dnf here instead of using the file in /etc/dnf/protected.d. This way, DNF 4 cannot uninstall itself, but DNF 5 can uninstall DNF 4, and likewise (given a similar change in DNF 5), DNF 5 cannot uninstall itself, but DNF 4 can uninstall DNF 5.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=2221905
Related: https://bugzilla.redhat.com/show_bug.cgi?id=2221907

@evan-goode
Copy link
Member Author

@inknos
Copy link
Contributor

inknos commented Sep 21, 2023

just one small note. could you edit the commit message following the contribution format?
it is usually hard to describe changes like this one in the changelog when you are not the author. Other than this comment, it looks good. Thanks

Now that obsoletion of protected packages is allowed
(rpm-software-management#1610), it's best
to hardcode the protection of dnf here instead of using the file in
/etc/dnf/protected.d. This way, DNF 4 cannot uninstall itself, but DNF 5
can uninstall DNF 4, and likewise (given a similar change in DNF 5), DNF
5 cannot uninstall itself, but DNF 4 can uninstall DNF 5.

Related: https://bugzilla.redhat.com/show_bug.cgi?id=2221905
Related: https://bugzilla.redhat.com/show_bug.cgi?id=2221907

= changelog =
msg: Allow dnf to be removed by DNF 5
type: enhancement
@evan-goode evan-goode force-pushed the evan-goode/self-protection branch from fb562f7 to a2804d5 Compare September 21, 2023 17:54
@evan-goode
Copy link
Member Author

Sure, I updated the commit message and bumped the version to 0.71.1. Pinging @jan-kolarik since he is currently doing the releases.

evan-goode added a commit to evan-goode/dnf that referenced this pull request Sep 21, 2023
Require new version of libdnf so that dnf remains a protected package,
see rpm-software-management/libdnf#1621.
@jan-kolarik
Copy link
Member

jan-kolarik commented Sep 22, 2023

Thanks for the ping, the changes look OK to me, just the thing that in the PR, dnf hardcoded protection is added, but it's labeled as "allowing dnf to be removed by dnf5". That's a bit confusing to me, is it really allowed by just these PR changes?

@evan-goode
Copy link
Member Author

Thanks for the ping, the changes look OK to me, just the thing that in the PR, dnf hardcoded protection is added, but it's labeled as "allowing dnf to be removed by dnf5". That's a bit confusing to me, is it really allowed by just these PR changes?

Well, the change in dnf is also needed for that. But I wrote that in the changelog since that is the main effect to the user of this change, and the motivating reason for it. The changelog could say "Added back hardcoded of protection of dnf", but whether the protection is done using /etc/protected.d/dnf.conf or in code is an implementation detail; IMO the changelog should rather communicate the effect and purpose of a change.

@jan-kolarik
Copy link
Member

Thanks for the ping, the changes look OK to me, just the thing that in the PR, dnf hardcoded protection is added, but it's labeled as "allowing dnf to be removed by dnf5". That's a bit confusing to me, is it really allowed by just these PR changes?

Well, the change in dnf is also needed for that. But I wrote that in the changelog since that is the main effect to the user of this change, and the motivating reason for it. The changelog could say "Added back hardcoded of protection of dnf", but whether the protection is done using /etc/protected.d/dnf.conf or in code is an implementation detail; IMO the changelog should rather communicate the effect and purpose of a change.

Yeah, you're right. When I was looking at this first, I was quite narrow-minded.

@inknos inknos merged commit 0814e79 into rpm-software-management:dnf-4-master Sep 25, 2023
3 checks passed
inknos pushed a commit to rpm-software-management/dnf that referenced this pull request Sep 25, 2023
Require new version of libdnf so that dnf remains a protected package,
see rpm-software-management/libdnf#1621.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants