Bug 2383751

Summary: etckeeper-dnf5 doesn't stop transaction when /etc is not clean
Product: [Fedora] Fedora Reporter: Martin B. <martin.bukatovic>
Component: etckeeperAssignee: Thomas Moschny <thomas.moschny>
Status: ASSIGNED --- QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 42CC: alberto, thomas.moschny
Target Milestone: ---   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: ---
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Martin B. 2025-07-27 12:34:42 UTC 
The etcekeeper-dnf5 plugin is no longer able to stop dnf5 transaction when it notices that /etc directory is not clean.

Reproducible
============

Always

Version
=======

```
$ rpm -qa | egrep '(etckeeper|dnf5)'
libdnf5-5.2.13.1-1.fc42.x86_64
libdnf5-cli-5.2.13.1-1.fc42.x86_64
dnf5-5.2.13.1-1.fc42.x86_64
dnf5-plugins-5.2.13.1-1.fc42.x86_64
dnf5daemon-server-5.2.13.1-1.fc42.x86_64
python3-libdnf5-5.2.13.1-1.fc42.x86_64
libdnf5-plugin-expired-pgp-keys-5.2.13.1-1.fc42.x86_64
libdnf5-plugin-actions-5.2.13.1-1.fc42.x86_64
etckeeper-dnf-1.18.22-3.fc42.noarch
etckeeper-1.18.22-3.fc42.noarch
etckeeper-dnf5-1.18.22-3.fc42.noarch
```

Steps to Reproduce
==================

1. Install and initialize etckeeper to track /etc via git
2. Configure `AVOID_COMMIT_BEFORE_INSTALL=1` in /etc/etckeeper/etckeeper.conf
3. Make a change in /etc so that git working three there will not be clean
4. Try to install some package, eg `dnf install caddy` assuming you don't have it installed

Actual Results
==============

The etcekeeper-dnf5 plugin notices the problem, but the error is not propagated:

```
# dnf install caddy

** etckeeper detected uncommitted changes in /etc prior to dnf run
** Aborting dnf run. Manually commit and restart.

Updating and loading repositories:
Repositories loaded.
Package                        Arch     Version                        Repository            Size
Installing:
 caddy                         x86_64   2.10.0-1.fc42                  updates           46.1 MiB
Installing dependencies:
 fedora-logos-httpd            noarch   42.0.1-1.fc42                  fedora            12.1 KiB
Installing weak dependencies:
 julietaula-montserrat-fonts   noarch   1:9.000-2.fc42                 updates            5.6 MiB

Transaction Summary:
 Installing:         3 packages

Total size of inbound packages is 14 MiB. Need to download 14 MiB.
After this operation, 52 MiB extra will be used (install 52 MiB, remove 0 B).
Is this ok [y/N]:
```

Expected Results
================

The etcekeeper-dnf5 plugin notices the problem and stops the dnf transaction:

```
# dnf install caddy

** etckeeper detected uncommitted changes in /etc prior to dnf run
** Aborting dnf run. Manually commit and restart.

File "/etc/dnf/libdnf5-plugins/actions.d/etckeeper.actions" on line 3: Exit code: 1
# 
```

Additional Information
======================

It looks like this is caused by adding the `log.INFO=` line in the etckeeper output as was proposed here:

https://bugzilla.redhat.com/show_bug.cgi?id=2326283#c8

When I drop these, the raise error works again.

```
# dnf install caddy

** etckeeper detected uncommitted changes in /etc prior to dnf run
** Aborting dnf run. Manually commit and restart.

File "/etc/dnf/libdnf5-plugins/actions.d/etckeeper.actions" on line 3: Exit code: 1
```

I suspect that when using the log.INFO convention, one can no longer simply raise error via sheer return code, but will need to report it as an error or via other  means.
Comment 1 Martin B. 2025-07-27 13:17:53 UTC 
Let's ask dnf5 developers first before we decide how to handle this properly.

I initiated the discussion upstream: https://github.com/rpm-software-management/dnf5/discussions/2366
Comment 2 Thomas Moschny 2025-09-01 15:41:34 UTC 
Thinking about it again, errors are swallowed not by the fact that we add `log.INFO=` per se, but doing it in a pipe.

Therefore, adding "set -o pipefail" should help.

The proposal for /etc/dnf/libdnf5-plugins/actions.d/etckeeper.actions would therefore be:


# to be placed in /etc/dnf/libdnf5-plugins/actions.d
# requires dnf5 5.2.11.0 or later
pre_transaction:::raise_error=1:/usr/bin/sh -c set\ -o\ pipefail\;\ /usr/bin/etckeeper\ pre-install\ \|\ /usr/bin/sed\ -e\ 's/^/log.INFO=/'
post_transaction::::/usr/bin/sh -c DNF_PID=${pid}\ /usr/bin/etckeeper\ post-install\ \|\ /usr/bin/sed\ -e\ 's/^/log.INFO=/'


(also taking into account changes proposed in bug 2387561 and bug 2383375)
Comment 3 Thomas Moschny 2025-09-01 16:25:29 UTC 
Or, after looking at https://dnf5.readthedocs.io/en/latest/dnf5_workflow.html, the goal_resolved hook should be even better.