4c64fc
From 8266a4fb45621e08085c58537a531f33fc7eca74 Mon Sep 17 00:00:00 2001
4c64fc
From: Alan Jenkins <alan.christopher.jenkins@gmail.com>
4c64fc
Date: Wed, 10 Apr 2019 00:19:14 +0100
4c64fc
Subject: [PATCH 1/2] DNF: stderr is not ours to log to (and doing so breaks
4c64fc
 Ansible)
4c64fc
4c64fc
stderr does not belong to etckeeper-dnf
4c64fc
---------------------------------------
4c64fc
4c64fc
The Ansible dnf module uses the python dnf bindings.  In contexts like
4c64fc
these, stdout/stderr is owned by the host app (Ansible).  dnf should not
4c64fc
mess with stdout/stderr, unless the host app asks it to log there.
4c64fc
4c64fc
Specifically, we were breaking the JSON output of the Ansible dnf module.
4c64fc
This was only noticeable when the etckeeper message began with a "["
4c64fc
character.  Ansible has a mechanism to try and skip header messages like
4c64fc
login banners.  However, "[" is a valid character to start a JSON document.
4c64fc
4c64fc
https://unix.stackexchange.com/questions/511210/ansible-dnf-module-module-failure/
4c64fc
4c64fc
Solution
4c64fc
--------
4c64fc
4c64fc
Instead, log any non-zero exit status through the dnf logger.
4c64fc
4c64fc
For the pre-transaction etckeeper run, this message replaces an exception.
4c64fc
So we now avoid logging a traceback, which did not appear to add anything
4c64fc
useful.  (In my testing, dnf was continued to install after the exception
4c64fc
was logged).
4c64fc
4c64fc
I have also added a warning message for the post-transaction etckeeper run.
4c64fc
4c64fc
I switched from os.system() to subprocess.call().  The latter interface
4c64fc
supports better error reporting.  (When I tested os.system(), it returned
4c64fc
an errno number which was indistinguishable from an exit code).
4c64fc
4c64fc
etckeeper >/dev/null 2>&1
4c64fc
--------------------------
4c64fc
4c64fc
Any specific error messages from etckeeper are now discarded.  Because
4c64fc
unfortunately, python conventions about passing text strings have been
4c64fc
somewhat unclear.  It is an error to log strings if the encoding settings
4c64fc
of a registered logger cannot handle them.  Because a "bad" character
4c64fc
causes the entire string to be discarded, and optionally an exception
4c64fc
printed to stderr.  In a previous proposal I showed code that should be
4c64fc
correct in all expected cases.  However, the length of comment required to
4c64fc
define "all expected cases" was not very reassuring.
4c64fc
4c64fc
That was on top of explaining that dnf has a workaround that will avoid
4c64fc
raising exceptions when arbitrary unicode is logged to stdout.  My proposal
4c64fc
had to include that explanation, to show that we were not breaking a
4c64fc
system package manager.
4c64fc
4c64fc
It might sound strange to talk about error messages with characters
4c64fc
we cannot encode.  But I have one word for you: "filenames".
4c64fc
4c64fc
(After the recent python PR #14008[1], logging.basicConfig() defaults
4c64fc
to errors='backslashreplace'.  Also, errors= can be manually specified when
4c64fc
creating a lower-level logging "Handler".  I think this will resolve the
4c64fc
problem in the future.  It means that when programs are written for this
4c64fc
new version of python, it should be the responsibility of the log setup
4c64fc
code to prevent UnicodeEncodeError.)
4c64fc
4c64fc
[1] https://github.com/python/cpython/pull/14008
4c64fc
---
4c64fc
 etckeeper-dnf/etckeeper.py | 27 ++++++++++++++++-----------
4c64fc
 1 file changed, 16 insertions(+), 11 deletions(-)
4c64fc
4c64fc
diff --git a/etckeeper-dnf/etckeeper.py b/etckeeper-dnf/etckeeper.py
4c64fc
index e8a1a51..69edd88 100644
4c64fc
--- a/etckeeper-dnf/etckeeper.py
4c64fc
+++ b/etckeeper-dnf/etckeeper.py
4c64fc
@@ -9,7 +9,7 @@
4c64fc
 
4c64fc
 from dnfpluginscore import logger
4c64fc
 
4c64fc
-import os
4c64fc
+import subprocess
4c64fc
 import dnf
4c64fc
 
4c64fc
 
4c64fc
@@ -17,20 +17,25 @@ class Etckeeper(dnf.Plugin):
4c64fc
 
4c64fc
     name = 'etckeeper'
4c64fc
 
4c64fc
-    def _out(self, msg):
4c64fc
-        logger.debug('Etckeeper plugin: %s', msg)
4c64fc
+    def _run_command(self, command):
4c64fc
+        logger.debug('Etckeeper plugin: %s', command)
4c64fc
+        try:
4c64fc
+            with open("/dev/null", "wb") as devnull:
4c64fc
+                ret = subprocess.call(("etckeeper", command),
4c64fc
+                                      stdout=devnull, stderr=devnull,
4c64fc
+                                      close_fds=True)
4c64fc
+                if ret > 0:
4c64fc
+                    logger.warning('"etckeeper %s" failed (exit code %d)' % (command, ret))
4c64fc
+                if ret < 0:
4c64fc
+                    logger.warning('"etckeeper %s" died (signal %d)' % (command, -ret))
4c64fc
+        except OSError as err:
4c64fc
+            logger.warning('Failed to run "etckeeper %s": %s' % (command, err))
4c64fc
 
4c64fc
     def resolved(self):
4c64fc
-        self._out('pre transaction commit')
4c64fc
-        command = '%s %s' % ('etckeeper', " pre-install")
4c64fc
-        ret = os.system(command)
4c64fc
-        if ret != 0:
4c64fc
-            raise dnf.exceptions.Error('etckeeper returned %d' % (ret >> 8))
4c64fc
+        self._run_command("pre-install")
4c64fc
 
4c64fc
     def transaction(self):
4c64fc
-        self._out('post transaction commit')
4c64fc
-        command = '%s %s > /dev/null' % ('etckeeper', "post-install")
4c64fc
-        os.system(command)
4c64fc
+        self._run_command("post-install")
4c64fc
 
4c64fc
 if __name__ == "__main__":
4c64fc
     from distutils.core import setup
4c64fc
-- 
4c64fc
2.23.0
4c64fc
4c64fc
4c64fc
From 7cda9678b1e60c0495a2a522721b319843d5fae0 Mon Sep 17 00:00:00 2001
4c64fc
From: Alan Jenkins <alan.christopher.jenkins@gmail.com>
4c64fc
Date: Tue, 23 Apr 2019 20:15:01 +0100
4c64fc
Subject: [PATCH 2/2] Do not use dnfpluginscore.logger
4c64fc
4c64fc
dnfpluginscore is not core support for dnf plugins, it is just a
4c64fc
collection of "core plugins" for dnf.  There is equally
4c64fc
dnfpluginsextras and dnfpluginsextras.logger.
4c64fc
4c64fc
A comment in dnf.logger comment says the logger name 'dnf.plugin' is "api".
4c64fc
So we can safely rely on that name.
4c64fc
4c64fc
Technically you can install etckeeper without the dnfpluginscore package
4c64fc
(at least I managed to run into this for python2, on a Fedora 29 system
4c64fc
which uses python3 for dnf by default).
4c64fc
---
4c64fc
 etckeeper-dnf/etckeeper.py | 5 +++--
4c64fc
 1 file changed, 3 insertions(+), 2 deletions(-)
4c64fc
4c64fc
diff --git a/etckeeper-dnf/etckeeper.py b/etckeeper-dnf/etckeeper.py
4c64fc
index 69edd88..d9dd2c3 100644
4c64fc
--- a/etckeeper-dnf/etckeeper.py
4c64fc
+++ b/etckeeper-dnf/etckeeper.py
4c64fc
@@ -7,11 +7,12 @@
4c64fc
 # Distutils code below was copied from etckeeper-bzr distributed with v1.15
4c64fc
 #
4c64fc
 
4c64fc
-from dnfpluginscore import logger
4c64fc
-
4c64fc
+import logging
4c64fc
 import subprocess
4c64fc
 import dnf
4c64fc
 
4c64fc
+logger = logging.getLogger('dnf.plugin')
4c64fc
+
4c64fc
 
4c64fc
 class Etckeeper(dnf.Plugin):
4c64fc
 
4c64fc
-- 
4c64fc
2.23.0
4c64fc