-
Notifications
You must be signed in to change notification settings - Fork 416
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 detection for ostree-based systems and warn users about losing changes #2053
Conversation
I have two proposals regarding the code I've reviewed. Firstly, as you suggested, I'd go with displaying the warning text to the user during the transaction confirmation, which involves moving the logic before this specific line. This approach eliminates the need to handle individual commands, as it addresses any transactional operation. Secondly, in |
dnf/cli/commands/alias.py
Outdated
@@ -157,6 +158,8 @@ def list_alias(self, cmd): | |||
print(_("Alias %s='%s'") % (cmd, " ".join(args))) | |||
|
|||
def run(self): | |||
dnf.util.is_container() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to spread the message to all commands including commands from plugins (dnf-plugins-core, extras, third party), or do we want to be selective?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have gone back and forth on this and I think it's mostly wasted effort to be selective right now. I think it's a better idea to do a blanket warning and then shift the focus to incorporating rpm-ostree functionality directly in to native dnf (or a dnf core plugin) where appropriate and then eventually we can just drop this change. I view this change as a temporary thing just to make it nicer for users.
I like these suggestions, thanks! I will get the PR updated. I've also got an additional method for reliably detecting an ostree system, so I'm going to try to rework is_container() a bit too. |
…anges On ostree-based systems, users can use dnf to customize the environment but those changes will be lost at the next ostree-based image update. If you want to retain changes between ostree-updates you need to make use of rpm-ostree right now. Signed-off-by: David Cantrell <dcantrell@redhat.com>
LGTM, thanks! |
5c050ba
into
rpm-software-management:master
xref https://issues.redhat.com/browse/SWM-1313 related to this |
This should test for |
On ostree-based systems, users can use dnf to customize the environment but those changes will be lost at the next ostree-based image update. If you want to retain changes between ostree-updates you need to make use of rpm-ostree right now.
All this patch does is add a function to detect systems managed this way and present warnings before dnf operations that modify the system. I display the warning at the beginning of the output, but maybe that should be after any other output is on the display.