commit 2542a24b63f9d5c7dfd05932fc3b2f05cd720024 Author: Nick Mathewson <nickm@torproject.org> Date: Mon Feb 3 13:02:12 2020 -0500 practracker: add a --regen-overbroad option to remove overbroad exceptions. Closes ticket 32372. --- changes/ticket32372 | 4 ++++ scripts/maint/practracker/practracker.py | 27 ++++++++++++++++++++++----- scripts/maint/practracker/problem.py | 21 +++++++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/changes/ticket32372 b/changes/ticket32372 new file mode 100644 index 000000000..ed2d38b40 --- /dev/null +++ b/changes/ticket32372 @@ -0,0 +1,4 @@ + o Minor features (best practices tracker): + - Practracker now supports a --regen-overbroad option to regenerate + the exceptions file, but only to revise exceptions to be _less_ + tolerant of best-practices violations. Closes ticket 32372. diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index e806aef3b..3f40f63c4 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -185,6 +185,9 @@ def main(argv): help="Regenerate the exceptions file") parser.add_argument("--list-overbroad", action="store_true", help="List over-broad exceptions") + parser.add_argument("--regen-overbroad", action="store_true", + help="Regenerate the exceptions file, " + "removing over-broad exceptions.") parser.add_argument("--exceptions", help="Override the location for the exceptions file") parser.add_argument("--strict", action="store_true", @@ -227,8 +230,9 @@ def main(argv): filt.addThreshold(problem.DependencyViolationItem("*.c", int(args.max_dependency_violations))) filt.addThreshold(problem.DependencyViolationItem("*.h", int(args.max_dependency_violations))) - if args.list_overbroad and args.regen: - print("Cannot use --regen with --list-overbroad", + if args.list_overbroad + args.regen + args.regen_overbroad > 1: + print("Cannot use more than one of --regen, --list-overbroad, and " + "--regen-overbroad.", file=sys.stderr) sys.exit(1) @@ -247,13 +251,15 @@ def main(argv): ProblemVault = problem.ProblemVault(exceptions_file) problem_file = sys.stdout - if args.list_overbroad: - # If we're listing overbroad exceptions, don't list problems. + if args.list_overbroad or args.regen_overbroad: + # If we're looking for overbroad exceptions, don't list problems + # immediately to the problem file. problem_file = util.NullFile() # 2.1) Adjust the exceptions so that we warn only about small problems, # and produce errors on big ones. - if not (args.regen or args.list_overbroad or args.strict): + if not (args.regen or args.list_overbroad or args.regen_overbroad or + args.strict): ProblemVault.set_tolerances(TOLERANCE_FNS) # 3) Go through all the files and report problems if they are not exceptions @@ -272,6 +278,16 @@ def main(argv): os.rename(tmpname, exceptions_file) sys.exit(0) + if args.regen_overbroad: + tmpname = exceptions_file + ".tmp" + tmpfile = open(tmpname, "w") + tmpfile.write(HEADER) + for item in ProblemVault.list_exceptions_without_overbroad(): + print(item, file=tmpfile) + tmpfile.close() + os.rename(tmpname, exceptions_file) + sys.exit(0) + # If new issues were found, try to give out some advice to the developer on how to resolve it. if found_new_issues and not args.regen and not args.terse: new_issues_str = """\ @@ -296,6 +312,7 @@ variable. else: print(ex, "->", p.metric_value) + sys.exit(found_new_issues) if __name__ == '__main__': diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index bee5eeb90..a3255dcc8 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -29,6 +29,8 @@ class ProblemVault(object): def __init__(self, exception_fname=None): # Exception dictionary: { problem.key() : Problem object } self.exceptions = {} + # Exception list: list of Problem objects, in the order added. + self.exception_list = [] # Exception dictionary: maps key to the problem it was used to # suppress. self.used_exception_for = {} @@ -63,6 +65,7 @@ class ProblemVault(object): sys.exit(1) self.exceptions[problem.key()] = problem + self.exception_list.append(problem) #print "Registering exception: %s" % problem def register_problem(self, problem): @@ -98,6 +101,24 @@ class ProblemVault(object): if p is None or e.is_worse_than(p): yield (e, p) + def list_exceptions_without_overbroad(self): + """Return an iterator of new problems, such that overbroad + exceptions are replaced with minimally broad versions, or removed. + """ + for e in self.exception_list: + p = self.used_exception_for.get(e.key()) + if p is None: + # This exception wasn't needed at all. + continue + if e.is_worse_than(p): + # The exception is worse than the problem we found. + # Yield the problem as the new exception value. + yield p + else: + # The problem is as bad as the exception, or worse. + # Yield the exception. + yield e + def set_tolerances(self, fns): """Adjust the tolerances for the exceptions in this vault. Takes a map of problem type to a function that adjusts the permitted