From 0c1147e4e1033844932d07241e7e0337be71711c Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 24 Mar 2018 06:38:21 +0000 Subject: [PATCH 01/10] test: Fix typo Signed-off-by: Niels Thykier --- tests/test_hint_parser.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_hint_parser.py b/tests/test_hint_parser.py index b503746..0105db0 100644 --- a/tests/test_hint_parser.py +++ b/tests/test_hint_parser.py @@ -5,7 +5,7 @@ from britney2.hints import HintParser, single_hint_taking_list_of_packages from . import MockObject, HINTS_ALL, TEST_HINTER -def new_hint_paser(logger=None): +def new_hint_parser(logger=None): if logger is None: def empty_logger(x, type='I'): pass @@ -23,7 +23,7 @@ class HintParsing(unittest.TestCase): def test_parse_invalid_hints(self): hint_log = [] - hint_parser = new_hint_paser(lambda x, type='I': hint_log.append(x)) + hint_parser = new_hint_parser(lambda x, type='I': hint_log.append(x)) hint_parser.register_hint_type('min-10-arg', parse_should_not_call_this_function, min_args=10) hint_parser.register_hint_type('simple-hint', parse_should_not_call_this_function) @@ -54,7 +54,7 @@ class HintParsing(unittest.TestCase): hint_log.clear() def test_alias(self): - hint_parser = new_hint_paser() + hint_parser = new_hint_parser() hint_parser.register_hint_type('real-name', single_hint_taking_list_of_packages, aliases=['alias1', 'alias2'] @@ -74,5 +74,6 @@ class HintParsing(unittest.TestCase): assert not hints.search(type='alias1', package='foo', version='1.0') assert not hints.search(type='alias2', package='bar', version='2.0') + if __name__ == '__main__': unittest.main() From e5d790f59272f700be2dba83bbd3fa759a9df0d2 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 24 Mar 2018 18:11:06 +0000 Subject: [PATCH 02/10] Typo fix Signed-off-by: Niels Thykier --- doc/hints.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/hints.rst b/doc/hints.rst index 359175c..b4f4ca7 100644 --- a/doc/hints.rst +++ b/doc/hints.rst @@ -237,7 +237,7 @@ solution for a concrete problem with a valid solution. Although, in many cases, britney will generally figure out the solution on its own. *Caveat*: Due to "uninstallability trading", this hint may cause -undesireable changes to the target suite. In practise, this is rather +undesirable changes to the target suite. In practise, this is rather rare but the hinter is letting britney decide what "repairs" the situation. @@ -254,7 +254,7 @@ This hint is generally useful when the provided `` is more desirable than the resulting breakage. *Caveat*: Be sure to test the outcome of these hints. A last minute -change can have long lasting undesireable consequences on the end +change can have long lasting undesirable consequences on the end result. Other hints From 5e825043d3f2d45f8a1a9728841da0155bc6d3d4 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 24 Mar 2018 08:59:33 +0000 Subject: [PATCH 03/10] Replace "print" logging with the "logging" module This commit rewrites the make-shift "log" methods to use the logging framework without requiring changes to the callers. This will be done in a latter commit to keep things reviewable. Signed-off-by: Niels Thykier --- britney.py | 41 +++++++++++++++++++++++++++++++++++-- britney2/policies/policy.py | 11 ++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/britney.py b/britney.py index 65146da..a0ac5df 100755 --- a/britney.py +++ b/britney.py @@ -179,6 +179,7 @@ does for the generation of the update excuses. * The excuses are written in an HTML file. """ +import logging import optparse import os import sys @@ -253,6 +254,32 @@ class Britney(object): the information needed by the other methods of the class. """ + # setup logging - provide the "short level name" (i.e. INFO -> I) that + # we used to use prior to using the logging module. + + old_factory = logging.getLogRecordFactory() + short_level_mapping = { + 'CRITIAL': 'F', + 'INFO': 'I', + 'WARNING': 'W', + 'ERROR': 'E', + 'DEBUG': 'N', + } + + def record_factory(*args, **kwargs): + record = old_factory(*args, **kwargs) + try: + record.shortlevelname = short_level_mapping[record.levelname] + except KeyError: + record.shortlevelname = record.levelname + return record + + logging.setLogRecordFactory(record_factory) + logging.basicConfig(format='{shortlevelname}: [{asctime}] - {message}', style='{', + datefmt="%Y-%m-%dT%H:%M:%S%z") + + self.logger = logging.getLogger() + # parse the command line arguments self.policies = [] self._hint_parser = HintParser(self) @@ -415,6 +442,11 @@ class Britney(object): parser.add_option("", "--no-compute-migrations", action="store_false", dest="compute_migrations", help="Do not compute which packages can migrate.") (self.options, self.args) = parser.parse_args() + + if self.options.verbose: + self.logger.setLevel(logging.INFO) + else: + self.logger.setLevel(logging.WARNING) # integrity checks if self.options.nuninst_cache and self.options.print_uninst: # pragma: no cover @@ -529,8 +561,12 @@ class Britney(object): `Error'. Warnings and errors are always printed, and information is printed only if verbose logging is enabled. """ - if self.options.verbose or type in ("E", "W"): - print("%s: [%s] - %s" % (type, time.asctime(), msg)) + level = { + 'I': logging.INFO, + 'W': logging.WARNING, + 'E': logging.ERROR, + }.get(type, logging.INFO) + self.logger.log(level, msg) def _load_faux_packages(self, faux_packages_file): """Loads fake packages @@ -2771,6 +2807,7 @@ class Britney(object): self.log('> %s' % stat, type="I") else: self.log('Migration computation skipped as requested.', type='I') + logging.shutdown() if __name__ == '__main__': diff --git a/britney2/policies/policy.py b/britney2/policies/policy.py index adcc938..f4dc7f9 100644 --- a/britney2/policies/policy.py +++ b/britney2/policies/policy.py @@ -1,4 +1,5 @@ import json +import logging import os import time from abc import abstractmethod @@ -30,6 +31,8 @@ class BasePolicy(object): self.suite_info = suite_info self.applicable_suites = applicable_suites self.hints = None + logger_name = ".".join((self.__class__.__module__, self.__class__.__name__)) + self.logger = logging.getLogger(logger_name) # FIXME: use a proper logging framework def log(self, msg, type="I"): @@ -41,8 +44,12 @@ class BasePolicy(object): `Error'. Warnings and errors are always printed, and information is printed only if verbose logging is enabled. """ - if self.options.verbose or type in ("E", "W"): - print("%s: [%s] - %s" % (type, time.asctime(), msg)) + level = { + 'I': logging.INFO, + 'W': logging.WARNING, + 'E': logging.ERROR, + }.get(type, logging.INFO) + self.logger.log(level, msg) def register_hints(self, hint_parser): # pragma: no cover """Register new hints that this policy accepts From ac69b3b5c73db290a712f92255bcdb0d8514a1b5 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 24 Mar 2018 09:11:03 +0000 Subject: [PATCH 04/10] Migrate hint parser to fully use logging Signed-off-by: Niels Thykier --- britney.py | 2 +- britney2/hints.py | 24 ++++++++++++------------ tests/test_hint_parser.py | 25 ++++++++++--------------- tests/test_policy.py | 2 +- 4 files changed, 24 insertions(+), 29 deletions(-) diff --git a/britney.py b/britney.py index a0ac5df..fc27864 100755 --- a/britney.py +++ b/britney.py @@ -282,7 +282,7 @@ class Britney(object): # parse the command line arguments self.policies = [] - self._hint_parser = HintParser(self) + self._hint_parser = HintParser() self.suite_info = {} self.__parse_arguments() MigrationItem.set_architectures(self.options.architectures) diff --git a/britney2/hints.py b/britney2/hints.py index aa9a7e7..5fb39b1 100644 --- a/britney2/hints.py +++ b/britney2/hints.py @@ -12,6 +12,8 @@ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # GNU General Public License for more details. +import logging + from itertools import chain from britney2.migrationitem import MigrationItem @@ -131,8 +133,9 @@ def single_hint_taking_list_of_packages(hints, who, hint_type, *args): class HintParser(object): - def __init__(self, britney): - self._britney = britney + def __init__(self): + logger_name = ".".join((self.__class__.__module__, self.__class__.__name__)) + self.logger = logging.getLogger(logger_name) self.hints = HintCollection() self._hint_table = { 'remark': (0, lambda *x: None), @@ -215,24 +218,21 @@ class HintParser(object): if hint_name == 'finished': break if hint_name not in hint_table: - self.log("Unknown hint found in %s (line %d): '%s'" % (filename, line_no, line), type="W") + self.logger.warning("Unknown hint found in %s (line %d): '%s'" % (filename, line_no, line)) continue if hint_name not in permitted_hints and 'ALL' not in permitted_hints: reason = 'The hint is not a part of the permitted hints for ' + who - self.log("Ignoring \"%s\" hint from %s found in %s (line %d): %s" % ( - hint_name, who, filename, line_no, reason), type="I") + self.logger.info("Ignoring \"%s\" hint from %s found in %s (line %d): %s" % ( + hint_name, who, filename, line_no, reason)) continue min_args, hint_parser_impl = hint_table[hint_name] if len(l) - 1 < min_args: - self.log("Malformed hint found in %s (line %d): Needs at least %d argument(s), got %d" % ( - filename, line_no, min_args, len(l) - 1), type="W") + self.logger.warning("Malformed hint found in %s (line %d): Needs at least %d argument(s), got %d" % ( + filename, line_no, min_args, len(l) - 1)) continue try: hint_parser_impl(hints, who, *l) except MalformedHintException as e: - self.log("Malformed hint found in %s (line %d): \"%s\"" % ( - filename, line_no, e.args[0]), type="W") + self.logger.warning("Malformed hint found in %s (line %d): \"%s\"" % ( + filename, line_no, e.args[0])) continue - - def log(self, msg, type="I"): - self._britney.log(msg, type=type) diff --git a/tests/test_hint_parser.py b/tests/test_hint_parser.py index 0105db0..834c179 100644 --- a/tests/test_hint_parser.py +++ b/tests/test_hint_parser.py @@ -1,18 +1,13 @@ +import logging import unittest from britney2.hints import HintParser, single_hint_taking_list_of_packages -from . import MockObject, HINTS_ALL, TEST_HINTER +from . import HINTS_ALL, TEST_HINTER -def new_hint_parser(logger=None): - if logger is None: - def empty_logger(x, type='I'): - pass - logger = empty_logger - fake_britney = MockObject(log=logger) - hint_parser = HintParser(fake_britney) - return hint_parser +def new_hint_parser(): + return HintParser() def parse_should_not_call_this_function(*args, **kwargs): @@ -22,8 +17,7 @@ def parse_should_not_call_this_function(*args, **kwargs): class HintParsing(unittest.TestCase): def test_parse_invalid_hints(self): - hint_log = [] - hint_parser = new_hint_parser(lambda x, type='I': hint_log.append(x)) + hint_parser = new_hint_parser() hint_parser.register_hint_type('min-10-arg', parse_should_not_call_this_function, min_args=10) hint_parser.register_hint_type('simple-hint', parse_should_not_call_this_function) @@ -47,11 +41,12 @@ class HintParsing(unittest.TestCase): ] for test in tests: - hint_parser.parse_hints(TEST_HINTER, test['permissions'], 'test-parse-hint', [test['hint_text']]) - assert len(hint_log) == 1 - assert test['error_message_contains'] in hint_log[0] + with self.assertLogs() as cm: + hint_parser.parse_hints(TEST_HINTER, test['permissions'], 'test-parse-hint', [test['hint_text']]) + + assert len(cm.output) == 1 + assert test['error_message_contains'] in cm.output[0] assert hint_parser.hints.is_empty - hint_log.clear() def test_alias(self): hint_parser = new_hint_parser() diff --git a/tests/test_policy.py b/tests/test_policy.py index c85f4fa..efc6b04 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -24,7 +24,7 @@ def initialize_policy(test_name, policy_class, *args, **kwargs): } policy = policy_class(options, suite_info, *args) fake_britney = MockObject(log=lambda x, y='I': None) - hint_parser = HintParser(fake_britney) + hint_parser = HintParser() policy.initialise(fake_britney) policy.register_hints(hint_parser) hint_parser.parse_hints(TEST_HINTER, HINTS_ALL, 'test-%s' % test_name, hints) From 8bf73610eed8810a7cbb9211291e56388a1f52f9 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 24 Mar 2018 09:31:22 +0000 Subject: [PATCH 05/10] policies: Use logger instead of "log" Signed-off-by: Niels Thykier --- britney2/policies/policy.py | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/britney2/policies/policy.py b/britney2/policies/policy.py index f4dc7f9..c31504b 100644 --- a/britney2/policies/policy.py +++ b/britney2/policies/policy.py @@ -34,23 +34,6 @@ class BasePolicy(object): logger_name = ".".join((self.__class__.__module__, self.__class__.__name__)) self.logger = logging.getLogger(logger_name) - # FIXME: use a proper logging framework - def log(self, msg, type="I"): - """Print info messages according to verbosity level - - An easy-and-simple log method which prints messages to the standard - output. The type parameter controls the urgency of the message, and - can be equal to `I' for `Information', `W' for `Warning' and `E' for - `Error'. Warnings and errors are always printed, and information is - printed only if verbose logging is enabled. - """ - level = { - 'I': logging.INFO, - 'W': logging.WARNING, - 'E': logging.ERROR, - }.get(type, logging.INFO) - self.logger.log(level, msg) - def register_hints(self, hint_parser): # pragma: no cover """Register new hints that this policy accepts @@ -310,7 +293,7 @@ class AgePolicy(BasePolicy): if not using_new_name: # If we using the legacy name, then just give up raise - self.log("%s does not appear to exist. Creating it" % filename) + self.logger.info("%s does not appear to exist. Creating it" % filename) with open(filename, mode='x', encoding='utf-8'): pass @@ -372,7 +355,7 @@ class AgePolicy(BasePolicy): fd.write("%s %s %d\n" % (pkg, version, date)) os.rename(filename_tmp, filename) if old_file is not None and os.path.exists(old_file): - self.log("Removing old age-policy-dates file %s" % old_file) + self.logger.info("Removing old age-policy-dates file %s" % old_file) os.unlink(old_file) @@ -457,7 +440,7 @@ class RCBugPolicy(BasePolicy): # Only handle one hint for now if 'ignored-bugs' in rcbugs_info: - self.log("Ignoring ignore-rc-bugs hint from %s on %s due to another hint from %s" % ( + self.logger.info("Ignoring ignore-rc-bugs hint from %s on %s due to another hint from %s" % ( ignore_hint.user, source_name, rcbugs_info['ignored-bugs']['issued-by'] )) continue @@ -470,7 +453,7 @@ class RCBugPolicy(BasePolicy): } success_verdict = PolicyVerdict.PASS_HINTED else: - self.log("Ignoring ignore-rc-bugs hint from %s on %s as none of %s affect the package" % ( + self.logger.info("Ignoring ignore-rc-bugs hint from %s on %s as none of %s affect the package" % ( ignore_hint.user, source_name, str(ignored_bugs) )) @@ -508,11 +491,11 @@ class RCBugPolicy(BasePolicy): name and the value is the list of open RC bugs for it. """ bugs = {} - self.log("Loading RC bugs data from %s" % filename) + self.logger.info("Loading RC bugs data from %s" % filename) for line in open(filename, encoding='ascii'): l = line.split() if len(l) != 2: - self.log("Malformed line found in line %s" % (line), type='W') + self.logger.warning("Malformed line found in line %s" % (line)) continue pkg = l[0] if pkg not in bugs: @@ -599,7 +582,7 @@ class PiupartsPolicy(BasePolicy): def _read_piuparts_summary(self, filename, keep_url=True): summary = {} - self.log("Loading piuparts report from {0}".format(filename)) + self.logger.info("Loading piuparts report from {0}".format(filename)) with open(filename) as fd: if os.fstat(fd.fileno()).st_size < 1: return summary From b23777c852ecc7b1deb6b763cced316c290791a3 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 24 Mar 2018 09:42:06 +0000 Subject: [PATCH 06/10] britney: Replace all calls to "log" with logger Signed-off-by: Niels Thykier --- britney.py | 165 ++++++++++++++++++++++++----------------------------- 1 file changed, 74 insertions(+), 91 deletions(-) diff --git a/britney.py b/britney.py index fc27864..762e3ff 100755 --- a/britney.py +++ b/britney.py @@ -300,7 +300,7 @@ class Britney(object): self.read_hints(os.path.join(self.suite_info['unstable'].path, 'Hints')) if self.options.nuninst_cache: - self.log("Not building the list of non-installable packages, as requested", type="I") + self.logger.info("Not building the list of non-installable packages, as requested") if self.options.print_uninst: nuninst = self.get_nuninst(build=False) print('* summary') @@ -340,34 +340,34 @@ class Britney(object): constraints_file = os.path.join(self.options.static_input_dir, 'constraints') faux_packages = os.path.join(self.options.static_input_dir, 'faux-packages') except AttributeError: - self.log("The static_input_dir option is not set", type='I') + self.logger.info("The static_input_dir option is not set") constraints_file = None faux_packages = None if faux_packages is not None and os.path.exists(faux_packages): - self.log("Loading faux packages from %s" % faux_packages, type='I') + self.logger.info("Loading faux packages from %s" % faux_packages) self._load_faux_packages(faux_packages) elif faux_packages is not None: - self.log("No Faux packages as %s does not exist" % faux_packages, type='I') + self.logger.info("No Faux packages as %s does not exist" % faux_packages) if constraints_file is not None and os.path.exists(constraints_file): - self.log("Loading constraints from %s" % constraints_file, type='I') + self.logger.info("Loading constraints from %s" % constraints_file) self.constraints = self._load_constraints(constraints_file) else: if constraints_file is not None: - self.log("No constraints as %s does not exist" % constraints_file, type='I') + self.logger.info("No constraints as %s does not exist" % constraints_file) self.constraints = { 'keep-installable': [], } - self.log("Compiling Installability tester", type="I") + self.logger.info("Compiling Installability tester") self._inst_tester = build_installability_tester(self.binaries, self.options.architectures) if not self.options.nuninst_cache: - self.log("Building the list of non-installable packages for the full archive", type="I") + self.logger.info("Building the list of non-installable packages for the full archive") self._inst_tester.compute_testing_installability() nuninst = self.get_nuninst(build=True) for arch in self.options.architectures: - self.log("> Found %d non-installable packages" % len(nuninst[arch]), type="I") + self.logger.info("> Found %d non-installable packages" % len(nuninst[arch])) if self.options.print_uninst: self.nuninst_arch_report(nuninst, arch) @@ -379,12 +379,12 @@ class Britney(object): write_nuninst(self.options.noninst_status, nuninst) stats = self._inst_tester.compute_stats() - self.log("> Installability tester statistics (per architecture)", type="I") + self.logger.info("> Installability tester statistics (per architecture)") for arch in self.options.architectures: arch_stat = stats[arch] - self.log("> %s" % arch, type="I") + self.logger.info("> %s" % arch) for stat in arch_stat.stat_summary(): - self.log("> - %s" % stat, type="I") + self.logger.info("> - %s" % stat) for policy in self.policies: policy.hints = self.hints @@ -398,10 +398,10 @@ class Britney(object): bad.append((f, pkg_entry1[f], pkg_entry2[f])) if bad: # pragma: no cover - self.log("Mismatch found %s %s %s differs" % ( - package, pkg_entry1.version, parch), type="E") + self.logger.error("Mismatch found %s %s %s differs" % ( + package, pkg_entry1.version, parch)) for f, v1, v2 in bad: - self.log(" ... %s %s != %s" % (check_field_name[f], v1, v2)) + self.logger.info(" ... %s %s != %s" % (check_field_name[f], v1, v2)) raise ValueError("Invalid data set") # Merge ESSENTIAL if necessary @@ -450,11 +450,11 @@ class Britney(object): # integrity checks if self.options.nuninst_cache and self.options.print_uninst: # pragma: no cover - self.log("nuninst_cache and print_uninst are mutually exclusive!", type="E") + self.logger.error("nuninst_cache and print_uninst are mutually exclusive!") sys.exit(1) # if the configuration file exists, then read it and set the additional options elif not os.path.isfile(self.options.config): # pragma: no cover - self.log("Unable to read the configuration file (%s), exiting!" % self.options.config, type="E") + self.logger.error("Unable to read the configuration file (%s), exiting!" % self.options.config) sys.exit(1) # minimum days for unstable-testing transition and the list of hints @@ -484,29 +484,29 @@ class Britney(object): self.suite_info[suite] = SuiteInfo(name=suite, path=suite_path, excuses_suffix=suffix) else: if suite in {'testing', 'unstable'}: # pragma: no cover - self.log("Mandatory configuration %s is not set in the config" % suite.upper(), type='E') + self.logger.error("Mandatory configuration %s is not set in the config" % suite.upper()) sys.exit(1) - self.log("Optional suite %s is not defined (config option: %s) " % (suite, suite.upper())) + self.logger.info("Optional suite %s is not defined (config option: %s) " % (suite, suite.upper())) try: release_file = read_release_file(self.suite_info['testing'].path) - self.log("Found a Release file in testing - using that for defaults") + self.logger.info("Found a Release file in testing - using that for defaults") except FileNotFoundError: - self.log("Testing does not have a Release file.") + self.logger.info("Testing does not have a Release file.") release_file = None if getattr(self.options, "components", None): self.options.components = [s.strip() for s in self.options.components.split(",")] elif release_file and not self.options.control_files: self.options.components = release_file['Components'].split() - self.log("Using components listed in Release file: %s" % ' '.join(self.options.components)) + self.logger.info("Using components listed in Release file: %s" % ' '.join(self.options.components)) else: self.options.components = None if self.options.control_files and self.options.components: # pragma: no cover # We cannot regenerate the control files correctly when reading from an # actual mirror (we don't which package goes in what component etc.). - self.log("Cannot use --control-files with mirror-layout (components)!", type="E") + self.logger.error("Cannot use --control-files with mirror-layout (components)!") sys.exit(1) if not hasattr(self.options, "heidi_delta_output"): @@ -522,12 +522,12 @@ class Britney(object): allarches = sorted(self.options.architectures.split()) else: if not release_file: # pragma: no cover - self.log("No configured architectures and there is no release file for testing", type="E") - self.log("Please check if there is a \"Release\" file in %s" % self.suite_info['testing'].path, type="E") - self.log("or if the config file contains a non-empty \"ARCHITECTURES\" field", type="E") + self.logger.error("No configured architectures and there is no release file for testing") + self.logger.error("Please check if there is a \"Release\" file in %s" % self.suite_info['testing'].path) + self.logger.error("or if the config file contains a non-empty \"ARCHITECTURES\" field") sys.exit(1) allarches = sorted(release_file['Architectures'].split()) - self.log("Using architectures listed in Release file: %s" % ' '.join(allarches)) + self.logger.info("Using architectures listed in Release file: %s" % ' '.join(allarches)) arches = [x for x in allarches if x in self.options.nobreakall_arches] arches += [x for x in allarches if x not in arches and x not in self.options.outofsync_arches] arches += [x for x in allarches if x not in arches and x not in self.options.break_arches] @@ -552,22 +552,6 @@ class Britney(object): def hints(self): return self._hint_parser.hints - def log(self, msg, type="I"): - """Print info messages according to verbosity level - - An easy-and-simple log method which prints messages to the standard - output. The type parameter controls the urgency of the message, and - can be equal to `I' for `Information', `W' for `Warning' and `E' for - `Error'. Warnings and errors are always printed, and information is - printed only if verbose logging is enabled. - """ - level = { - 'I': logging.INFO, - 'W': logging.WARNING, - 'E': logging.ERROR, - }.get(type, logging.INFO) - self.logger.log(level, msg) - def _load_faux_packages(self, faux_packages_file): """Loads fake packages @@ -672,7 +656,7 @@ class Britney(object): if constraint not in {'present-and-installable'}: # pragma: no cover raise ValueError("Unsupported constraint %s for %s (file %s)" % (constraint, pkg_name, constraints_file)) - self.log(" - constraint %s" % pkg_name, type='I') + self.logger.info(" - constraint %s" % pkg_name) pkg_list = [x.strip() for x in mandatory_field('Package-List').split("\n") if x.strip() != '' and not x.strip().startswith("#")] src_data = SourcePackage(faux_version, @@ -746,11 +730,11 @@ class Britney(object): for component in self.options.components: filename = os.path.join(basedir, component, "source", "Sources") filename = possibly_compressed(filename) - self.log("Loading source packages from %s" % filename) + self.logger.info("Loading source packages from %s" % filename) read_sources_file(filename, sources) else: filename = os.path.join(basedir, "Sources") - self.log("Loading source packages from %s" % filename) + self.logger.info("Loading source packages from %s" % filename) sources = read_sources_file(filename) return sources @@ -761,13 +745,13 @@ class Britney(object): for or_clause in parts: if len(or_clause) != 1: # pragma: no cover msg = "Ignoring invalid provides in %s: Alternatives [%s]" % (str(pkg_id), str(or_clause)) - self.log(msg, type='W') + self.logger.warning(msg) continue for part in or_clause: provided, provided_version, op = part if op != '' and op != '=': # pragma: no cover msg = "Ignoring invalid provides in %s: %s (%s %s)" % (str(pkg_id), provided, op, provided_version) - self.log(msg, type='W') + self.logger.warning(msg) continue provided = sys.intern(provided) provided_version = sys.intern(provided_version) @@ -776,7 +760,7 @@ class Britney(object): return nprov def _read_packages_file(self, filename, arch, srcdist, packages=None, intern=sys.intern): - self.log("Loading binary packages from %s" % filename) + self.logger.info("Loading binary packages from %s" % filename) if packages is None: packages = {} @@ -929,7 +913,8 @@ class Britney(object): for arch in architectures: packages = {} if arch not in listed_archs: - self.log("Skipping arch %s for %s: It is not listed in the Release file" % (arch, distribution)) + self.logger.info("Skipping arch %s for %s: It is not listed in the Release file" % ( + arch, distribution)) arch2packages[arch] = ({}, {}) continue for component in self.options.components: @@ -993,9 +978,9 @@ class Britney(object): else: filename = os.path.join(hintsdir, who) if not os.path.isfile(filename): - self.log("Cannot read hints list from %s, no such file!" % filename, type="E") + self.logger.error("Cannot read hints list from %s, no such file!" % filename) continue - self.log("Loading hints list from %s" % filename) + self.logger.info("Loading hints list from %s" % filename) with open(filename, encoding='utf-8') as f: self._hint_parser.parse_hints(who, self.HINTS[who], filename, f) @@ -1011,25 +996,24 @@ class Britney(object): if x in ['unblock', 'unblock-udeb']: if apt_pkg.version_compare(hint2.version, hint.version) < 0: # This hint is for a newer version, so discard the old one - self.log("Overriding %s[%s] = ('%s', '%s') with ('%s', '%s')" % - (x, package, hint2.version, hint2.user, hint.version, hint.user), type="W") + self.logger.warning("Overriding %s[%s] = ('%s', '%s') with ('%s', '%s')" % + (x, package, hint2.version, hint2.user, hint.version, hint.user)) hint2.set_active(False) else: # This hint is for an older version, so ignore it in favour of the new one - self.log("Ignoring %s[%s] = ('%s', '%s'), ('%s', '%s') is higher or equal" % - (x, package, hint.version, hint.user, hint2.version, hint2.user), type="W") + self.logger.warning("Ignoring %s[%s] = ('%s', '%s'), ('%s', '%s') is higher or equal" % + (x, package, hint.version, hint.user, hint2.version, hint2.user)) hint.set_active(False) else: - self.log("Overriding %s[%s] = ('%s', '%s') with ('%s', '%s')" % - (x, package, hint2.user, hint2, hint.user, hint), - type="W") + self.logger.warning("Overriding %s[%s] = ('%s', '%s') with ('%s', '%s')" % + (x, package, hint2.user, hint2, hint.user, hint)) hint2.set_active(False) z[package] = key # Sanity check the hints hash if len(hints["block"]) == 0 and len(hints["block-udeb"]) == 0: - self.log("WARNING: No block hints at all, not even udeb ones!", type="W") + self.logger.warning("WARNING: No block hints at all, not even udeb ones!") # Utility methods for package analysis # ------------------------------------ @@ -1567,7 +1551,7 @@ class Britney(object): of this procedure, please refer to the module docstring. """ - self.log("Update Excuses generation started", type="I") + self.logger.info("Update Excuses generation started") # list of local methods and variables (for better performance) sources = self.sources @@ -1687,16 +1671,16 @@ class Britney(object): # write excuses to the output file if not self.options.dry_run: - self.log("> Writing Excuses to %s" % self.options.excuses_output, type="I") + self.logger.info("> Writing Excuses to %s" % self.options.excuses_output) sorted_excuses = sorted(excuses.values(), key=lambda x: x.sortkey()) write_excuses(sorted_excuses, self.options.excuses_output, output_format="legacy-html") if hasattr(self.options, 'excuses_yaml_output'): - self.log("> Writing YAML Excuses to %s" % self.options.excuses_yaml_output, type="I") + self.logger.info("> Writing YAML Excuses to %s" % self.options.excuses_yaml_output) write_excuses(sorted_excuses, self.options.excuses_yaml_output, output_format="yaml") - self.log("Update Excuses generation completed", type="I") + self.logger.info("Update Excuses generation completed") # Upgrade run # ----------- @@ -2392,14 +2376,14 @@ class Britney(object): self.output_write("\n") def assert_nuninst_is_correct(self): - self.log("> Update complete - Verifying non-installability counters", type="I") + self.logger.info("> Update complete - Verifying non-installability counters") cached_nuninst = self.nuninst_orig self._inst_tester.compute_testing_installability() computed_nuninst = self.get_nuninst(build=True) if cached_nuninst != computed_nuninst: # pragma: no cover only_on_break_archs = True - self.log("==================== NUNINST OUT OF SYNC =========================", type="E") + self.logger.error("==================== NUNINST OUT OF SYNC =========================") for arch in self.options.architectures: expected_nuninst = set(cached_nuninst[arch]) actual_nuninst = set(computed_nuninst[arch]) @@ -2410,18 +2394,17 @@ class Britney(object): if (false_negatives or false_positives) and arch not in self.options.break_arches: only_on_break_archs = False if false_negatives: - self.log(" %s - unnoticed nuninst: %s" % (arch, str(false_negatives)), type="E") + self.logger.error(" %s - unnoticed nuninst: %s" % (arch, str(false_negatives))) if false_positives: - self.log(" %s - invalid nuninst: %s" % (arch, str(false_positives)), type="E") - self.log(" %s - actual nuninst: %s" % (arch, str(actual_nuninst)), type="I") - self.log("==================== NUNINST OUT OF SYNC =========================", type="E") + self.logger.error(" %s - invalid nuninst: %s" % (arch, str(false_positives))) + self.logger.info(" %s - actual nuninst: %s" % (arch, str(actual_nuninst))) + self.logger.error("==================== NUNINST OUT OF SYNC =========================") if not only_on_break_archs: raise AssertionError("NUNINST OUT OF SYNC") else: - self.log("Nuninst is out of sync on some break arches", - type="W") + self.logger.warning("Nuninst is out of sync on some break arches") - self.log("> All non-installability counters are ok", type="I") + self.logger.info("> All non-installability counters are ok") def upgrade_testing(self): @@ -2432,11 +2415,11 @@ class Britney(object): commands. """ - self.log("Starting the upgrade test", type="I") + self.logger.info("Starting the upgrade test") self.output_write("Generated on: %s\n" % (time.strftime("%Y.%m.%d %H:%M:%S %z", time.gmtime(time.time())))) self.output_write("Arch order is: %s\n" % ", ".join(self.options.architectures)) - self.log("> Calculating current uninstallability counters", type="I") + self.logger.info("> Calculating current uninstallability counters") self.nuninst_orig = self.get_nuninst() # nuninst_orig may get updated during the upgrade process self.nuninst_orig_save = self.get_nuninst() @@ -2492,7 +2475,7 @@ class Britney(object): # obsolete source packages # a package is obsolete if none of the binary packages in testing # are built by it - self.log("> Removing obsolete source packages from testing", type="I") + self.logger.info("> Removing obsolete source packages from testing") # local copies for performance sources = self.sources['testing'] binaries = self.binaries['testing'] @@ -2510,15 +2493,15 @@ class Britney(object): # smooth updates removals = old_libraries(self.sources, self.binaries, self.options.outofsync_arches) if self.options.smooth_updates: - self.log("> Removing old packages left in testing from smooth updates", type="I") + self.logger.info("> Removing old packages left in testing from smooth updates") if removals: self.output_write("Removing packages left in testing for smooth updates (%d):\n%s" % \ (len(removals), old_libraries_format(removals))) self.do_all(actions=removals) removals = old_libraries(self.sources, self.binaries, self.options.outofsync_arches) else: - self.log("> Not removing old packages left in testing from smooth updates (smooth-updates disabled)", - type="I") + self.logger.info("> Not removing old packages left in testing from smooth updates" + " (smooth-updates disabled)") self.output_write("List of old libraries in testing (%d):\n%s" % \ (len(removals), old_libraries_format(removals))) @@ -2529,7 +2512,7 @@ class Britney(object): if not self.options.dry_run: # re-write control files if self.options.control_files: - self.log("Writing new testing control files to %s" % + self.logger.info("Writing new testing control files to %s" % self.suite_info['testing'].path) write_controlfiles(self.sources, self.binaries, 'testing', self.suite_info['testing'].path) @@ -2538,21 +2521,21 @@ class Britney(object): policy.save_state(self) # write HeidiResult - self.log("Writing Heidi results to %s" % self.options.heidi_output) + self.logger.info("Writing Heidi results to %s" % self.options.heidi_output) write_heidi(self.options.heidi_output, self.sources["testing"], self.binaries["testing"], outofsync_arches=self.options.outofsync_arches) - self.log("Writing delta to %s" % self.options.heidi_delta_output) + self.logger.info("Writing delta to %s" % self.options.heidi_delta_output) write_heidi_delta(self.options.heidi_delta_output, self.all_selected) self.printuninstchange() - self.log("Test completed!", type="I") + self.logger.info("Test completed!") def printuninstchange(self): - self.log("Checking for newly uninstallable packages", type="I") + self.logger.info("Checking for newly uninstallable packages") text = eval_uninst(self.options.architectures, newly_uninst( self.nuninst_orig_save, self.nuninst_orig)) @@ -2566,7 +2549,7 @@ class Britney(object): This method provides a command line interface for the release team to try hints and evaluate the results. """ - self.log("> Calculating current uninstallability counters", type="I") + self.logger.info("> Calculating current uninstallability counters") self.nuninst_orig = self.get_nuninst() self.nuninst_orig_save = self.get_nuninst() @@ -2613,7 +2596,7 @@ class Britney(object): try: readline.write_history_file(histfile) except IOError as e: - self.log("Could not write %s: %s" % (histfile, e), type="W") + self.logger.warning("Could not write %s: %s" % (histfile, e)) def do_hint(self, hinttype, who, pkgvers): """Process hints @@ -2627,7 +2610,7 @@ class Britney(object): else: _pkgvers = pkgvers - self.log("> Processing '%s' hint from %s" % (hinttype, who), type="I") + self.logger.info("> Processing '%s' hint from %s" % (hinttype, who)) self.output_write("Trying %s from %s: %s\n" % (hinttype, who, " ".join("%s/%s" % (x.uvname, x.version) for x in _pkgvers))) ok = True @@ -2683,7 +2666,7 @@ class Britney(object): excuses relationships. If they build a circular dependency, which we already know as not-working with the standard do_all algorithm, try to `easy` them. """ - self.log("> Processing hints from the auto hinter", type="I") + self.logger.info("> Processing hints from the auto hinter") sources_t = self.sources['testing'] excuses = self.excuses @@ -2802,11 +2785,11 @@ class Britney(object): else: self.upgrade_testing() - self.log('> Stats from the installability tester', type="I") + self.logger.info('> Stats from the installability tester') for stat in self._inst_tester.stats.stats(): - self.log('> %s' % stat, type="I") + self.logger.info('> %s' % stat) else: - self.log('Migration computation skipped as requested.', type='I') + self.logger.info('Migration computation skipped as requested.') logging.shutdown() From 32b2ee326aac4ef61edc82034dc13419881b5ce2 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Fri, 30 Mar 2018 17:38:50 +0000 Subject: [PATCH 07/10] Use lazy evaluation of format arguments in logging Signed-off-by: Niels Thykier --- britney.py | 91 ++++++++++++++++++------------------- britney2/hints.py | 13 +++--- britney2/policies/policy.py | 20 ++++---- 3 files changed, 60 insertions(+), 64 deletions(-) diff --git a/britney.py b/britney.py index 762e3ff..92f15be 100755 --- a/britney.py +++ b/britney.py @@ -344,17 +344,17 @@ class Britney(object): constraints_file = None faux_packages = None if faux_packages is not None and os.path.exists(faux_packages): - self.logger.info("Loading faux packages from %s" % faux_packages) + self.logger.info("Loading faux packages from %s", faux_packages) self._load_faux_packages(faux_packages) elif faux_packages is not None: - self.logger.info("No Faux packages as %s does not exist" % faux_packages) + self.logger.info("No Faux packages as %s does not exist", faux_packages) if constraints_file is not None and os.path.exists(constraints_file): - self.logger.info("Loading constraints from %s" % constraints_file) + self.logger.info("Loading constraints from %s", constraints_file) self.constraints = self._load_constraints(constraints_file) else: if constraints_file is not None: - self.logger.info("No constraints as %s does not exist" % constraints_file) + self.logger.info("No constraints as %s does not exist", constraints_file) self.constraints = { 'keep-installable': [], } @@ -367,7 +367,7 @@ class Britney(object): self._inst_tester.compute_testing_installability() nuninst = self.get_nuninst(build=True) for arch in self.options.architectures: - self.logger.info("> Found %d non-installable packages" % len(nuninst[arch])) + self.logger.info("> Found %d non-installable packages", len(nuninst[arch])) if self.options.print_uninst: self.nuninst_arch_report(nuninst, arch) @@ -382,9 +382,9 @@ class Britney(object): self.logger.info("> Installability tester statistics (per architecture)") for arch in self.options.architectures: arch_stat = stats[arch] - self.logger.info("> %s" % arch) + self.logger.info("> %s", arch) for stat in arch_stat.stat_summary(): - self.logger.info("> - %s" % stat) + self.logger.info("> - %s", stat) for policy in self.policies: policy.hints = self.hints @@ -398,10 +398,9 @@ class Britney(object): bad.append((f, pkg_entry1[f], pkg_entry2[f])) if bad: # pragma: no cover - self.logger.error("Mismatch found %s %s %s differs" % ( - package, pkg_entry1.version, parch)) + self.logger.error("Mismatch found %s %s %s differs", package, pkg_entry1.version, parch) for f, v1, v2 in bad: - self.logger.info(" ... %s %s != %s" % (check_field_name[f], v1, v2)) + self.logger.info(" ... %s %s != %s", check_field_name[f], v1, v2) raise ValueError("Invalid data set") # Merge ESSENTIAL if necessary @@ -454,7 +453,7 @@ class Britney(object): sys.exit(1) # if the configuration file exists, then read it and set the additional options elif not os.path.isfile(self.options.config): # pragma: no cover - self.logger.error("Unable to read the configuration file (%s), exiting!" % self.options.config) + self.logger.error("Unable to read the configuration file (%s), exiting!", self.options.config) sys.exit(1) # minimum days for unstable-testing transition and the list of hints @@ -484,9 +483,9 @@ class Britney(object): self.suite_info[suite] = SuiteInfo(name=suite, path=suite_path, excuses_suffix=suffix) else: if suite in {'testing', 'unstable'}: # pragma: no cover - self.logger.error("Mandatory configuration %s is not set in the config" % suite.upper()) + self.logger.error("Mandatory configuration %s is not set in the config", suite.upper()) sys.exit(1) - self.logger.info("Optional suite %s is not defined (config option: %s) " % (suite, suite.upper())) + self.logger.info("Optional suite %s is not defined (config option: %s) ", suite, suite.upper()) try: release_file = read_release_file(self.suite_info['testing'].path) @@ -499,7 +498,7 @@ class Britney(object): self.options.components = [s.strip() for s in self.options.components.split(",")] elif release_file and not self.options.control_files: self.options.components = release_file['Components'].split() - self.logger.info("Using components listed in Release file: %s" % ' '.join(self.options.components)) + self.logger.info("Using components listed in Release file: %s", ' '.join(self.options.components)) else: self.options.components = None @@ -523,11 +522,11 @@ class Britney(object): else: if not release_file: # pragma: no cover self.logger.error("No configured architectures and there is no release file for testing") - self.logger.error("Please check if there is a \"Release\" file in %s" % self.suite_info['testing'].path) + self.logger.error("Please check if there is a \"Release\" file in %s", self.suite_info['testing'].path) self.logger.error("or if the config file contains a non-empty \"ARCHITECTURES\" field") sys.exit(1) allarches = sorted(release_file['Architectures'].split()) - self.logger.info("Using architectures listed in Release file: %s" % ' '.join(allarches)) + self.logger.info("Using architectures listed in Release file: %s", ' '.join(allarches)) arches = [x for x in allarches if x in self.options.nobreakall_arches] arches += [x for x in allarches if x not in arches and x not in self.options.outofsync_arches] arches += [x for x in allarches if x not in arches and x not in self.options.break_arches] @@ -656,7 +655,7 @@ class Britney(object): if constraint not in {'present-and-installable'}: # pragma: no cover raise ValueError("Unsupported constraint %s for %s (file %s)" % (constraint, pkg_name, constraints_file)) - self.logger.info(" - constraint %s" % pkg_name) + self.logger.info(" - constraint %s", pkg_name) pkg_list = [x.strip() for x in mandatory_field('Package-List').split("\n") if x.strip() != '' and not x.strip().startswith("#")] src_data = SourcePackage(faux_version, @@ -730,11 +729,11 @@ class Britney(object): for component in self.options.components: filename = os.path.join(basedir, component, "source", "Sources") filename = possibly_compressed(filename) - self.logger.info("Loading source packages from %s" % filename) + self.logger.info("Loading source packages from %s", filename) read_sources_file(filename, sources) else: filename = os.path.join(basedir, "Sources") - self.logger.info("Loading source packages from %s" % filename) + self.logger.info("Loading source packages from %s", filename) sources = read_sources_file(filename) return sources @@ -744,14 +743,14 @@ class Britney(object): nprov = [] for or_clause in parts: if len(or_clause) != 1: # pragma: no cover - msg = "Ignoring invalid provides in %s: Alternatives [%s]" % (str(pkg_id), str(or_clause)) - self.logger.warning(msg) + msg = "Ignoring invalid provides in %s: Alternatives [%s]" + self.logger.warning(msg, str(pkg_id), str(or_clause)) continue for part in or_clause: provided, provided_version, op = part if op != '' and op != '=': # pragma: no cover - msg = "Ignoring invalid provides in %s: %s (%s %s)" % (str(pkg_id), provided, op, provided_version) - self.logger.warning(msg) + msg = "Ignoring invalid provides in %s: %s (%s %s)" + self.logger.warning(msg, str(pkg_id), provided, op, provided_version) continue provided = sys.intern(provided) provided_version = sys.intern(provided_version) @@ -760,7 +759,7 @@ class Britney(object): return nprov def _read_packages_file(self, filename, arch, srcdist, packages=None, intern=sys.intern): - self.logger.info("Loading binary packages from %s" % filename) + self.logger.info("Loading binary packages from %s", filename) if packages is None: packages = {} @@ -913,8 +912,8 @@ class Britney(object): for arch in architectures: packages = {} if arch not in listed_archs: - self.logger.info("Skipping arch %s for %s: It is not listed in the Release file" % ( - arch, distribution)) + self.logger.info("Skipping arch %s for %s: It is not listed in the Release file", + arch, distribution) arch2packages[arch] = ({}, {}) continue for component in self.options.components: @@ -978,9 +977,9 @@ class Britney(object): else: filename = os.path.join(hintsdir, who) if not os.path.isfile(filename): - self.logger.error("Cannot read hints list from %s, no such file!" % filename) + self.logger.error("Cannot read hints list from %s, no such file!", filename) continue - self.logger.info("Loading hints list from %s" % filename) + self.logger.info("Loading hints list from %s", filename) with open(filename, encoding='utf-8') as f: self._hint_parser.parse_hints(who, self.HINTS[who], filename, f) @@ -996,17 +995,17 @@ class Britney(object): if x in ['unblock', 'unblock-udeb']: if apt_pkg.version_compare(hint2.version, hint.version) < 0: # This hint is for a newer version, so discard the old one - self.logger.warning("Overriding %s[%s] = ('%s', '%s') with ('%s', '%s')" % - (x, package, hint2.version, hint2.user, hint.version, hint.user)) + self.logger.warning("Overriding %s[%s] = ('%s', '%s') with ('%s', '%s')", + x, package, hint2.version, hint2.user, hint.version, hint.user) hint2.set_active(False) else: # This hint is for an older version, so ignore it in favour of the new one - self.logger.warning("Ignoring %s[%s] = ('%s', '%s'), ('%s', '%s') is higher or equal" % - (x, package, hint.version, hint.user, hint2.version, hint2.user)) + self.logger.warning("Ignoring %s[%s] = ('%s', '%s'), ('%s', '%s') is higher or equal", + x, package, hint.version, hint.user, hint2.version, hint2.user) hint.set_active(False) else: - self.logger.warning("Overriding %s[%s] = ('%s', '%s') with ('%s', '%s')" % - (x, package, hint2.user, hint2, hint.user, hint)) + self.logger.warning("Overriding %s[%s] = ('%s', '%s') with ('%s', '%s')", + x, package, hint2.user, hint2, hint.user, hint) hint2.set_active(False) z[package] = key @@ -1671,12 +1670,12 @@ class Britney(object): # write excuses to the output file if not self.options.dry_run: - self.logger.info("> Writing Excuses to %s" % self.options.excuses_output) + self.logger.info("> Writing Excuses to %s", self.options.excuses_output) sorted_excuses = sorted(excuses.values(), key=lambda x: x.sortkey()) write_excuses(sorted_excuses, self.options.excuses_output, output_format="legacy-html") if hasattr(self.options, 'excuses_yaml_output'): - self.logger.info("> Writing YAML Excuses to %s" % self.options.excuses_yaml_output) + self.logger.info("> Writing YAML Excuses to %s", self.options.excuses_yaml_output) write_excuses(sorted_excuses, self.options.excuses_yaml_output, output_format="yaml") @@ -2394,10 +2393,10 @@ class Britney(object): if (false_negatives or false_positives) and arch not in self.options.break_arches: only_on_break_archs = False if false_negatives: - self.logger.error(" %s - unnoticed nuninst: %s" % (arch, str(false_negatives))) + self.logger.error(" %s - unnoticed nuninst: %s", arch, str(false_negatives)) if false_positives: - self.logger.error(" %s - invalid nuninst: %s" % (arch, str(false_positives))) - self.logger.info(" %s - actual nuninst: %s" % (arch, str(actual_nuninst))) + self.logger.error(" %s - invalid nuninst: %s", arch, str(false_positives)) + self.logger.info(" %s - actual nuninst: %s", arch, str(actual_nuninst)) self.logger.error("==================== NUNINST OUT OF SYNC =========================") if not only_on_break_archs: raise AssertionError("NUNINST OUT OF SYNC") @@ -2512,8 +2511,8 @@ class Britney(object): if not self.options.dry_run: # re-write control files if self.options.control_files: - self.logger.info("Writing new testing control files to %s" % - self.suite_info['testing'].path) + self.logger.info("Writing new testing control files to %s", + self.suite_info['testing'].path) write_controlfiles(self.sources, self.binaries, 'testing', self.suite_info['testing'].path) @@ -2521,12 +2520,12 @@ class Britney(object): policy.save_state(self) # write HeidiResult - self.logger.info("Writing Heidi results to %s" % self.options.heidi_output) + self.logger.info("Writing Heidi results to %s", self.options.heidi_output) write_heidi(self.options.heidi_output, self.sources["testing"], self.binaries["testing"], outofsync_arches=self.options.outofsync_arches) - self.logger.info("Writing delta to %s" % self.options.heidi_delta_output) + self.logger.info("Writing delta to %s", self.options.heidi_delta_output) write_heidi_delta(self.options.heidi_delta_output, self.all_selected) @@ -2596,7 +2595,7 @@ class Britney(object): try: readline.write_history_file(histfile) except IOError as e: - self.logger.warning("Could not write %s: %s" % (histfile, e)) + self.logger.warning("Could not write %s: %s", histfile, e) def do_hint(self, hinttype, who, pkgvers): """Process hints @@ -2610,7 +2609,7 @@ class Britney(object): else: _pkgvers = pkgvers - self.logger.info("> Processing '%s' hint from %s" % (hinttype, who)) + self.logger.info("> Processing '%s' hint from %s", hinttype, who) self.output_write("Trying %s from %s: %s\n" % (hinttype, who, " ".join("%s/%s" % (x.uvname, x.version) for x in _pkgvers))) ok = True @@ -2787,7 +2786,7 @@ class Britney(object): self.logger.info('> Stats from the installability tester') for stat in self._inst_tester.stats.stats(): - self.logger.info('> %s' % stat) + self.logger.info('> %s', stat) else: self.logger.info('Migration computation skipped as requested.') logging.shutdown() diff --git a/britney2/hints.py b/britney2/hints.py index 5fb39b1..0589e78 100644 --- a/britney2/hints.py +++ b/britney2/hints.py @@ -218,21 +218,20 @@ class HintParser(object): if hint_name == 'finished': break if hint_name not in hint_table: - self.logger.warning("Unknown hint found in %s (line %d): '%s'" % (filename, line_no, line)) + self.logger.warning("Unknown hint found in %s (line %d): '%s'", filename, line_no, line) continue if hint_name not in permitted_hints and 'ALL' not in permitted_hints: reason = 'The hint is not a part of the permitted hints for ' + who - self.logger.info("Ignoring \"%s\" hint from %s found in %s (line %d): %s" % ( - hint_name, who, filename, line_no, reason)) + self.logger.info("Ignoring \"%s\" hint from %s found in %s (line %d): %s", + hint_name, who, filename, line_no, reason) continue min_args, hint_parser_impl = hint_table[hint_name] if len(l) - 1 < min_args: - self.logger.warning("Malformed hint found in %s (line %d): Needs at least %d argument(s), got %d" % ( - filename, line_no, min_args, len(l) - 1)) + self.logger.warning("Malformed hint found in %s (line %d): Needs at least %d argument(s), got %d", + filename, line_no, min_args, len(l) - 1) continue try: hint_parser_impl(hints, who, *l) except MalformedHintException as e: - self.logger.warning("Malformed hint found in %s (line %d): \"%s\"" % ( - filename, line_no, e.args[0])) + self.logger.warning("Malformed hint found in %s (line %d): \"%s\"", filename, line_no, e.args[0]) continue diff --git a/britney2/policies/policy.py b/britney2/policies/policy.py index c31504b..c3cd34f 100644 --- a/britney2/policies/policy.py +++ b/britney2/policies/policy.py @@ -293,7 +293,7 @@ class AgePolicy(BasePolicy): if not using_new_name: # If we using the legacy name, then just give up raise - self.logger.info("%s does not appear to exist. Creating it" % filename) + self.logger.info("%s does not appear to exist. Creating it", filename) with open(filename, mode='x', encoding='utf-8'): pass @@ -355,7 +355,7 @@ class AgePolicy(BasePolicy): fd.write("%s %s %d\n" % (pkg, version, date)) os.rename(filename_tmp, filename) if old_file is not None and os.path.exists(old_file): - self.logger.info("Removing old age-policy-dates file %s" % old_file) + self.logger.info("Removing old age-policy-dates file %s", old_file) os.unlink(old_file) @@ -440,9 +440,8 @@ class RCBugPolicy(BasePolicy): # Only handle one hint for now if 'ignored-bugs' in rcbugs_info: - self.logger.info("Ignoring ignore-rc-bugs hint from %s on %s due to another hint from %s" % ( - ignore_hint.user, source_name, rcbugs_info['ignored-bugs']['issued-by'] - )) + self.logger.info("Ignoring ignore-rc-bugs hint from %s on %s due to another hint from %s", + ignore_hint.user, source_name, rcbugs_info['ignored-bugs']['issued-by']) continue if not ignored_bugs.isdisjoint(bugs_u): bugs_u -= ignored_bugs @@ -453,9 +452,8 @@ class RCBugPolicy(BasePolicy): } success_verdict = PolicyVerdict.PASS_HINTED else: - self.logger.info("Ignoring ignore-rc-bugs hint from %s on %s as none of %s affect the package" % ( - ignore_hint.user, source_name, str(ignored_bugs) - )) + self.logger.info("Ignoring ignore-rc-bugs hint from %s on %s as none of %s affect the package", + ignore_hint.user, source_name, str(ignored_bugs)) rcbugs_info['shared-bugs'] = sorted(bugs_u & bugs_t) rcbugs_info['unique-source-bugs'] = sorted(bugs_u - bugs_t) @@ -491,11 +489,11 @@ class RCBugPolicy(BasePolicy): name and the value is the list of open RC bugs for it. """ bugs = {} - self.logger.info("Loading RC bugs data from %s" % filename) + self.logger.info("Loading RC bugs data from %s", filename) for line in open(filename, encoding='ascii'): l = line.split() if len(l) != 2: - self.logger.warning("Malformed line found in line %s" % (line)) + self.logger.warning("Malformed line found in line %s", line) continue pkg = l[0] if pkg not in bugs: @@ -582,7 +580,7 @@ class PiupartsPolicy(BasePolicy): def _read_piuparts_summary(self, filename, keep_url=True): summary = {} - self.logger.info("Loading piuparts report from {0}".format(filename)) + self.logger.info("Loading piuparts report from %s", filename) with open(filename) as fd: if os.fstat(fd.fileno()).st_size < 1: return summary From f25060edf405acba56869cf21cc8fb63de138b46 Mon Sep 17 00:00:00 2001 From: Paul Gevers Date: Sat, 3 Mar 2018 22:26:13 +0100 Subject: [PATCH 08/10] doc: Wording/typos --- doc/short-intro-to-migrations.rst | 6 +++--- doc/solutions-to-common-policy-issues.rst | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/short-intro-to-migrations.rst b/doc/short-intro-to-migrations.rst index 92d3336..a555afc 100644 --- a/doc/short-intro-to-migrations.rst +++ b/doc/short-intro-to-migrations.rst @@ -61,7 +61,7 @@ document will only describe the source migration item. associated binary packages once built. Once a new version of a source package appears in the source suite, -britney will create track it with a source migration item. As the +britney will track it with a source migration item. As the binary packages are built and uploaded, they will be included into the migration item and various QA checks/policies will be applied to the item. @@ -71,8 +71,8 @@ migrate the item (i.e. source with its binaries) to the target suite. -As implied earlier, there are several other migration types. But they -are not covered in this document. They deal with cases like removals, +As implied earlier, there are several other migration types, +not covered in this document. They deal with cases like removals, rebuilds of existing binaries, etc. Migration phase 1: Policies / Excuses diff --git a/doc/solutions-to-common-policy-issues.rst b/doc/solutions-to-common-policy-issues.rst index ec1b43a..b5f0586 100644 --- a/doc/solutions-to-common-policy-issues.rst +++ b/doc/solutions-to-common-policy-issues.rst @@ -9,7 +9,7 @@ Britney complains about a fixed bug in the source suite (bug policy) All decisions about bugs are related to data set extracted from the bug tracker. If britney says that the new version introduces a bug, then it is because the data set from the bug -tracker lists that bug for *a* version in the source suite and +tracker lists that bug for *a* version in the source suite, without it appearing for the version(s) in the target suite. Please note that these data sets do not include versions, so @@ -24,7 +24,7 @@ There is a number of common cases, where this is observed: * The bug is fixed, but the old version is still around in the source suite. In this case, britney will generally - mention a "missing build" or "old binaries". + also mention a "missing build" or "old binaries". If the metadata is wrong, the solution is to fix it in the bug tracker and wait until britney receives a new data set. In @@ -37,9 +37,9 @@ Britney complains about "missing builds" ---------------------------------------- A "missing build" happens when britney detects that the binaries -for a given architecture are missing or is not up to date. This +for a given architecture are missing or not up to date. This is detected by checking the "Packages" files in the archive, so -britney have no knowledge of *why* the build is missing. +britney has no knowledge of *why* the build is missing. Accordingly, this kind of issue is flagged as a "possibly permanent" issue. From e667e4dcaf534838510d9e2a7e7356be4fd55414 Mon Sep 17 00:00:00 2001 From: Paul Gevers Date: Sat, 3 Mar 2018 22:43:48 +0100 Subject: [PATCH 09/10] doc: Initial version for piuparts solutions --- doc/solutions-to-common-policy-issues.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/solutions-to-common-policy-issues.rst b/doc/solutions-to-common-policy-issues.rst index b5f0586..d3ce926 100644 --- a/doc/solutions-to-common-policy-issues.rst +++ b/doc/solutions-to-common-policy-issues.rst @@ -103,3 +103,16 @@ non-obvious issues: then libfoo2 depends on libfoo-data v2, then libfoo1 will become uninstallable as libfoo-data v2 will "shadow" libfoo-data v1. +Britney complains about "Piupart" +--------------------------------- + +Britney can be configured to take the results of piuparts (package +installation, upgrading and removal testing suite) into account. Currently this +policy is only taking into account the piuparts result for installing and +purging the package in the source suite and the target suite (so no upgrade +test). As with the other policies, a regression means that the package passes +in the target suite, but fails in the source suite. Unless this is a bug in +piuparts, the package needs to be fixed first to install and purge cleanly in +the non-interactive debconf state. An URL to the relevant piuparts results is +provided in the excuses. + From e441902d925e1f37bb1ea7f9348e2b98da6712d8 Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Sat, 31 Mar 2018 05:32:51 +0000 Subject: [PATCH 10/10] doc: Fix missing letter in title Signed-off-by: Niels Thykier --- doc/solutions-to-common-policy-issues.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/solutions-to-common-policy-issues.rst b/doc/solutions-to-common-policy-issues.rst index d3ce926..d07130c 100644 --- a/doc/solutions-to-common-policy-issues.rst +++ b/doc/solutions-to-common-policy-issues.rst @@ -103,7 +103,7 @@ non-obvious issues: then libfoo2 depends on libfoo-data v2, then libfoo1 will become uninstallable as libfoo-data v2 will "shadow" libfoo-data v1. -Britney complains about "Piupart" +Britney complains about "Piuparts" --------------------------------- Britney can be configured to take the results of piuparts (package