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