From 4c64fcc5f0b6b6811247ecf0708a6a40e75101a1 Mon Sep 17 00:00:00 2001 From: Thomas Moschny Date: Dec 01 2019 12:31:04 +0000 Subject: Update to 1.18.12. New version of patch to fix logging with Ansible (#1762693). --- diff --git a/etckeeper-1.18.10-fix-output-for-ansible.patch b/etckeeper-1.18.10-fix-output-for-ansible.patch deleted file mode 100644 index b1d0a65..0000000 --- a/etckeeper-1.18.10-fix-output-for-ansible.patch +++ /dev/null @@ -1,189 +0,0 @@ -From afbdce2d24b46bc91840044b953aca8b68f20fd3 Mon Sep 17 00:00:00 2001 -From: Alan Jenkins -Date: Wed, 10 Apr 2019 00:19:14 +0100 -Subject: [PATCH 1/3] DNF: fix logging, now it will work from Ansible - -The Ansible dnf module uses the python dnf bindings. In contexts like -these, stdout/stderr is owned by the host app (Ansible). dnf should not -mess with stdout/stderr, unless the host app asks it to log there. - -Specifically, it was breaking the JSON output of the Ansible module. -This was only noticeable when the etckeeper message began with a "[" -character. Ansible has a mechanism to try and skip header messages like -login banners. However, "[" is a valid character to start a JSON document. - -https://unix.stackexchange.com/questions/511210/ansible-dnf-module-module-failure/ - -This requires decoding the etckeeper messages into unicode. For gory -details, see the code comment. - -Also enable unicode string literals. This reduces differences between py2 -and py3 semantics; it is universal inside the dnf code. - -Also when I tested the failure case, I noticed the exit code was not -printed correctly. Fix it. ---- - etckeeper-dnf/etckeeper.py | 62 ++++++++++++++++++++++++++++++++------ - 1 file changed, 53 insertions(+), 9 deletions(-) - -diff --git a/etckeeper-dnf/etckeeper.py b/etckeeper-dnf/etckeeper.py -index e8a1a51..d3376ac 100644 ---- a/etckeeper-dnf/etckeeper.py -+++ b/etckeeper-dnf/etckeeper.py -@@ -7,9 +7,11 @@ - # Distutils code below was copied from etckeeper-bzr distributed with v1.15 - # - -+from __future__ import unicode_literals - from dnfpluginscore import logger - --import os -+import subprocess -+import locale - import dnf - - -@@ -17,20 +19,62 @@ class Etckeeper(dnf.Plugin): - - name = 'etckeeper' - -- def _out(self, msg): -+ def _debug(self, msg): - logger.debug('Etckeeper plugin: %s', msg) - -+ def _log_pipe(self, pipe): -+ # etckeeper & git messages should be encoded using the default locale -+ # (or us-ascii, which is a strict subset). -+ # -+ # Normally py2 breaks if you print arbitrary unicode when stdout is -+ # not a tty (UnicodeEncodeError). However the dnf cli has a -+ # workaround; it will survive regardless of what we do. -+ # -+ # Then we have logging.FileHandler. In py3 it will use -+ # locale.getpreferredencoding(False) by default. This should match -+ # the default locale, unless we are on py < 3.2, AND the program -+ # forgot to call setlocale(LC_ALL, ""). dnf already calls -+ # setlocale(LC_ALL, ""), so it will be nice and consistent. -+ # In fact it is the dnf *library* that calls setlocale, this is not -+ # really recommended, but it makes me pretty confident here. -+ # -+ # errors='replace' means that decode errors give us '\ufffd', which -+ # causes UnicodeEncodeError in some character encodings. Let us -+ # simulate a round-trip through errors='replace', by replacing them -+ # with a question mark. -+ # -+ # The story for py2 is more complex. In libdnf 2.6.3, the logfile -+ # is equivalent to hardcoding a utf8 encoding. That is survivable -+ # (and if it changes to match py3, it will also be fine). -+ # -+ encoding = locale.getpreferredencoding(False) -+ for line in pipe: -+ line = line.decode(encoding, 'replace') -+ line.replace('\ufffd', '?') -+ line = line.rstrip('\n') -+ logger.info('%s', line) -+ - def resolved(self): -- self._out('pre transaction commit') -- command = '%s %s' % ('etckeeper', " pre-install") -- ret = os.system(command) -+ self._debug('pre transaction commit') -+ proc = subprocess.Popen(("etckeeper", "pre-install"), -+ stdout=subprocess.PIPE, -+ stderr=subprocess.STDOUT, -+ close_fds=True) -+ self._log_pipe(proc.stdout) -+ ret = proc.wait() - if ret != 0: -- raise dnf.exceptions.Error('etckeeper returned %d' % (ret >> 8)) -+ raise dnf.exceptions.Error('etckeeper returned %d' % ret) - - def transaction(self): -- self._out('post transaction commit') -- command = '%s %s > /dev/null' % ('etckeeper', "post-install") -- os.system(command) -+ self._debug('post transaction commit') -+ proc = subprocess.Popen(("etckeeper", "post-install"), -+ stdout=None, -+ stderr=subprocess.PIPE, -+ close_fds=True) -+ self._log_pipe(proc.stderr) -+ ret = proc.wait() -+ if ret != 0: -+ logger.err('etckeeper returned %d' % ret) - - if __name__ == "__main__": - from distutils.core import setup --- -2.23.0 - - -From dcf94a71d46ff0e9981319b9a7786d565bec9a28 Mon Sep 17 00:00:00 2001 -From: Alan Jenkins -Date: Tue, 23 Apr 2019 20:15:01 +0100 -Subject: [PATCH 2/3] Do not use dnfpluginscore.logger - -dnfpluginscore is not core support for dnf plugins, it is just a -collection of "core plugins" for dnf. There is equally -dnfpluginsextras and dnfpluginsextras.logger. - -A comment in dnf.logger comment says the logger name 'dnf.plugin' is "api". -So we can safely rely on that name. - -Technically you can install etckeeper without the dnfpluginscore package -(at least I managed to run into this for python2, on a Fedora 29 system -which uses python3 for dnf by default). ---- - etckeeper-dnf/etckeeper.py | 4 +++- - 1 file changed, 3 insertions(+), 1 deletion(-) - -diff --git a/etckeeper-dnf/etckeeper.py b/etckeeper-dnf/etckeeper.py -index d3376ac..72f3dc5 100644 ---- a/etckeeper-dnf/etckeeper.py -+++ b/etckeeper-dnf/etckeeper.py -@@ -8,12 +8,14 @@ - # - - from __future__ import unicode_literals --from dnfpluginscore import logger - -+import logging - import subprocess - import locale - import dnf - -+logger = logging.getLogger('dnf.plugin') -+ - - class Etckeeper(dnf.Plugin): - --- -2.23.0 - - -From 6e9e369353457c2cfb303e2be15a14381e2eb53a Mon Sep 17 00:00:00 2001 -From: Thomas Moschny -Date: Tue, 19 Nov 2019 08:27:03 +0100 -Subject: [PATCH 3/3] setup() doesn't accept unicode strings. - ---- - etckeeper-dnf/etckeeper.py | 6 +++--- - 1 file changed, 3 insertions(+), 3 deletions(-) - -diff --git a/etckeeper-dnf/etckeeper.py b/etckeeper-dnf/etckeeper.py -index 72f3dc5..7a9c256 100644 ---- a/etckeeper-dnf/etckeeper.py -+++ b/etckeeper-dnf/etckeeper.py -@@ -80,6 +80,6 @@ class Etckeeper(dnf.Plugin): - - if __name__ == "__main__": - from distutils.core import setup -- setup(name="dnf-etckeeper", -- packages=["dnf-plugins"], -- package_dir={"dnf-plugins":"etckeeper-dnf"}) -+ setup(name=str("dnf-etckeeper"), -+ packages=[str("dnf-plugins")], -+ package_dir={str("dnf-plugins"):str("etckeeper-dnf")}) --- -2.23.0 - diff --git a/etckeeper-1.18.12-fix-output-for-ansible.patch b/etckeeper-1.18.12-fix-output-for-ansible.patch new file mode 100644 index 0000000..7ca4b4f --- /dev/null +++ b/etckeeper-1.18.12-fix-output-for-ansible.patch @@ -0,0 +1,162 @@ +From 8266a4fb45621e08085c58537a531f33fc7eca74 Mon Sep 17 00:00:00 2001 +From: Alan Jenkins +Date: Wed, 10 Apr 2019 00:19:14 +0100 +Subject: [PATCH 1/2] DNF: stderr is not ours to log to (and doing so breaks + Ansible) + +stderr does not belong to etckeeper-dnf +--------------------------------------- + +The Ansible dnf module uses the python dnf bindings. In contexts like +these, stdout/stderr is owned by the host app (Ansible). dnf should not +mess with stdout/stderr, unless the host app asks it to log there. + +Specifically, we were breaking the JSON output of the Ansible dnf module. +This was only noticeable when the etckeeper message began with a "[" +character. Ansible has a mechanism to try and skip header messages like +login banners. However, "[" is a valid character to start a JSON document. + +https://unix.stackexchange.com/questions/511210/ansible-dnf-module-module-failure/ + +Solution +-------- + +Instead, log any non-zero exit status through the dnf logger. + +For the pre-transaction etckeeper run, this message replaces an exception. +So we now avoid logging a traceback, which did not appear to add anything +useful. (In my testing, dnf was continued to install after the exception +was logged). + +I have also added a warning message for the post-transaction etckeeper run. + +I switched from os.system() to subprocess.call(). The latter interface +supports better error reporting. (When I tested os.system(), it returned +an errno number which was indistinguishable from an exit code). + +etckeeper >/dev/null 2>&1 +-------------------------- + +Any specific error messages from etckeeper are now discarded. Because +unfortunately, python conventions about passing text strings have been +somewhat unclear. It is an error to log strings if the encoding settings +of a registered logger cannot handle them. Because a "bad" character +causes the entire string to be discarded, and optionally an exception +printed to stderr. In a previous proposal I showed code that should be +correct in all expected cases. However, the length of comment required to +define "all expected cases" was not very reassuring. + +That was on top of explaining that dnf has a workaround that will avoid +raising exceptions when arbitrary unicode is logged to stdout. My proposal +had to include that explanation, to show that we were not breaking a +system package manager. + +It might sound strange to talk about error messages with characters +we cannot encode. But I have one word for you: "filenames". + +(After the recent python PR #14008[1], logging.basicConfig() defaults +to errors='backslashreplace'. Also, errors= can be manually specified when +creating a lower-level logging "Handler". I think this will resolve the +problem in the future. It means that when programs are written for this +new version of python, it should be the responsibility of the log setup +code to prevent UnicodeEncodeError.) + +[1] https://github.com/python/cpython/pull/14008 +--- + etckeeper-dnf/etckeeper.py | 27 ++++++++++++++++----------- + 1 file changed, 16 insertions(+), 11 deletions(-) + +diff --git a/etckeeper-dnf/etckeeper.py b/etckeeper-dnf/etckeeper.py +index e8a1a51..69edd88 100644 +--- a/etckeeper-dnf/etckeeper.py ++++ b/etckeeper-dnf/etckeeper.py +@@ -9,7 +9,7 @@ + + from dnfpluginscore import logger + +-import os ++import subprocess + import dnf + + +@@ -17,20 +17,25 @@ class Etckeeper(dnf.Plugin): + + name = 'etckeeper' + +- def _out(self, msg): +- logger.debug('Etckeeper plugin: %s', msg) ++ def _run_command(self, command): ++ logger.debug('Etckeeper plugin: %s', command) ++ try: ++ with open("/dev/null", "wb") as devnull: ++ ret = subprocess.call(("etckeeper", command), ++ stdout=devnull, stderr=devnull, ++ close_fds=True) ++ if ret > 0: ++ logger.warning('"etckeeper %s" failed (exit code %d)' % (command, ret)) ++ if ret < 0: ++ logger.warning('"etckeeper %s" died (signal %d)' % (command, -ret)) ++ except OSError as err: ++ logger.warning('Failed to run "etckeeper %s": %s' % (command, err)) + + def resolved(self): +- self._out('pre transaction commit') +- command = '%s %s' % ('etckeeper', " pre-install") +- ret = os.system(command) +- if ret != 0: +- raise dnf.exceptions.Error('etckeeper returned %d' % (ret >> 8)) ++ self._run_command("pre-install") + + def transaction(self): +- self._out('post transaction commit') +- command = '%s %s > /dev/null' % ('etckeeper', "post-install") +- os.system(command) ++ self._run_command("post-install") + + if __name__ == "__main__": + from distutils.core import setup +-- +2.23.0 + + +From 7cda9678b1e60c0495a2a522721b319843d5fae0 Mon Sep 17 00:00:00 2001 +From: Alan Jenkins +Date: Tue, 23 Apr 2019 20:15:01 +0100 +Subject: [PATCH 2/2] Do not use dnfpluginscore.logger + +dnfpluginscore is not core support for dnf plugins, it is just a +collection of "core plugins" for dnf. There is equally +dnfpluginsextras and dnfpluginsextras.logger. + +A comment in dnf.logger comment says the logger name 'dnf.plugin' is "api". +So we can safely rely on that name. + +Technically you can install etckeeper without the dnfpluginscore package +(at least I managed to run into this for python2, on a Fedora 29 system +which uses python3 for dnf by default). +--- + etckeeper-dnf/etckeeper.py | 5 +++-- + 1 file changed, 3 insertions(+), 2 deletions(-) + +diff --git a/etckeeper-dnf/etckeeper.py b/etckeeper-dnf/etckeeper.py +index 69edd88..d9dd2c3 100644 +--- a/etckeeper-dnf/etckeeper.py ++++ b/etckeeper-dnf/etckeeper.py +@@ -7,11 +7,12 @@ + # Distutils code below was copied from etckeeper-bzr distributed with v1.15 + # + +-from dnfpluginscore import logger +- ++import logging + import subprocess + import dnf + ++logger = logging.getLogger('dnf.plugin') ++ + + class Etckeeper(dnf.Plugin): + +-- +2.23.0 + diff --git a/etckeeper.spec b/etckeeper.spec index 652e7f2..f73ccd4 100644 --- a/etckeeper.spec +++ b/etckeeper.spec @@ -32,8 +32,8 @@ %endif Name: etckeeper -Version: 1.18.10 -Release: 6%{?dist} +Version: 1.18.12 +Release: 1%{?dist} Summary: Store /etc in a SCM system (git, mercurial, bzr or darcs) License: GPLv2+ URL: https://etckeeper.branchable.com/ @@ -48,7 +48,7 @@ Patch2: etckeeper-1.18.7-fix-hg-warnings.patch # From https://bugs.launchpad.net/ubuntu/+source/etckeeper/+bug/1826855 Patch3: etckeeper-add-breezy-python3-plugin.patch # see rhbz#1762693 and https://github.com/ansible/ansible/issues/54949 -Patch4: etckeeper-1.18.10-fix-output-for-ansible.patch +Patch4: etckeeper-1.18.12-fix-output-for-ansible.patch BuildArch: noarch BuildRequires: %{__markdown} Requires: git >= 1.6.1 @@ -328,6 +328,10 @@ fi %changelog +* Sun Dec 1 2019 Thomas Moschny - 1.18.12-1 +- Update to 1.18.12. +- New version of patch to fix logging with Ansible (#1762693). + * Tue Nov 19 2019 Thomas Moschny - 1.18.10-6 - Add patch to fix logging with Ansible (#1762693). diff --git a/sources b/sources index 033b121..67c6744 100644 --- a/sources +++ b/sources @@ -1 +1 @@ -SHA512 (etckeeper-1.18.10.tar.gz) = 57eb91823c37f364e7ade6b25d2ab91fc99d1192a606fd09370c2876a0440120a625f30167fc52fb16b53ef9f7a58a1329bfe9567cbc2c91da06a345ebbe8e05 +SHA512 (etckeeper-1.18.12.tar.gz) = 7fb75d89bbf69d5ef29fc93f34f8368f0d93adb6f89a96be4769be4a58faff793682f5a5fc6f6f2bf51bc5a151a28b61319396a4298cbf0aa75ff3c1bbb660d7