From 25d55fa01ebc8d9bc8cd52672605636a9261ddf5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 08:49:57 -0400 Subject: [PATCH 1/4] Practracker: do not list problems when told to --list-overbroad. --- scripts/maint/practracker/practracker.py | 9 +++++++++ scripts/maint/practracker/util.py | 7 +++++++ 2 files changed, 16 insertions(+) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index ce9c5f5d82..6f1a771efc 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -224,6 +224,11 @@ 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", + file=sys.stderr) + sys.exit(1) + # 1) Get all the .c files we care about files_list = util.get_tor_c_files(TOR_TOPDIR, args.include_dir) @@ -239,6 +244,10 @@ 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. + 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): diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py index 4b42565528..df629110c2 100644 --- a/scripts/maint/practracker/util.py +++ b/scripts/maint/practracker/util.py @@ -41,3 +41,10 @@ def get_tor_c_files(tor_topdir, include_dirs=None): files_list.append(full_path) return files_list + +class NullFile: + """A file-like object that we can us to suppress output.""" + def __init__(self): + pass + def write(self, s): + pass From d1ffac3f8abbdd5b7d8e5955615821bff3ee3729 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 08:52:01 -0400 Subject: [PATCH 2/4] practracker: An exception is "used" even when it is violated. The purpose of tracking whether an exception is used is so that we can tell whether it is overbroad. This means that an _underbroad_ exception is still a used one. Fixes bug 31338. --- scripts/maint/practracker/problem.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index 88e1044fb7..d21840a213 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -77,8 +77,10 @@ class ProblemVault(object): # (e.g. we went from 4k LoC to 3k LoC), but we do warn if the # situation worsened (e.g. we went from 60 includes to 80). status = problem.is_worse_than(self.exceptions[problem.key()]) - if status == STATUS_OK: - self.used_exception_for[problem.key()] = problem + + # Remember that we used this exception, so that we can later + # determine whether the exception was overbroad. + self.used_exception_for[problem.key()] = problem return status From 1c67d261d528767bc3a24f738dc316635f000f7e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 08:53:17 -0400 Subject: [PATCH 3/4] practracker: Integration test for --list-overbroad Make sure that we list overbroad exceptions, and nothing else --- Makefile.am | 1 + scripts/maint/practracker/test_practracker.sh | 6 ++++++ .../maint/practracker/testdata/ex1-overbroad-expected.txt | 2 ++ scripts/maint/practracker/testdata/ex1.txt | 5 ++++- 4 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 scripts/maint/practracker/testdata/ex1-overbroad-expected.txt diff --git a/Makefile.am b/Makefile.am index 491b4c8f9f..845440b56f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -183,6 +183,7 @@ EXTRA_DIST+= \ scripts/maint/practracker/testdata/ex0.txt \ scripts/maint/practracker/testdata/ex1-expected.txt \ scripts/maint/practracker/testdata/ex1.txt \ + scripts/maint/practracker/testdata/ex1-overbroad-expected.txt \ scripts/maint/practracker/testdata/ex.txt \ scripts/maint/practracker/testdata/header.h \ scripts/maint/practracker/testdata/not_c_file \ diff --git a/scripts/maint/practracker/test_practracker.sh b/scripts/maint/practracker/test_practracker.sh index bfbd0c6560..9b107e071d 100755 --- a/scripts/maint/practracker/test_practracker.sh +++ b/scripts/maint/practracker/test_practracker.sh @@ -61,3 +61,9 @@ echo "ex1:" run_practracker --exceptions "${DATA}/ex1.txt" > "${TMPDIR}/ex1-received.txt" compare "${TMPDIR}/ex1-received.txt" "${DATA}/ex1-expected.txt" + +echo "ex1.overbroad:" + +run_practracker --exceptions "${DATA}/ex1.txt" --list-overbroad > "${TMPDIR}/ex1-overbroad-received.txt" + +compare "${TMPDIR}/ex1-overbroad-received.txt" "${DATA}/ex1-overbroad-expected.txt" diff --git a/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt b/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt new file mode 100644 index 0000000000..f69c608f40 --- /dev/null +++ b/scripts/maint/practracker/testdata/ex1-overbroad-expected.txt @@ -0,0 +1,2 @@ +problem file-size a.c 40 -> 38 +problem file-size z.c 100 -> 0 diff --git a/scripts/maint/practracker/testdata/ex1.txt b/scripts/maint/practracker/testdata/ex1.txt index f619e33b22..c698005d07 100644 --- a/scripts/maint/practracker/testdata/ex1.txt +++ b/scripts/maint/practracker/testdata/ex1.txt @@ -1,5 +1,5 @@ -problem file-size a.c 38 +problem file-size a.c 40 problem include-count a.c 4 # this problem will produce an error problem function-size a.c:i_am_a_function() 8 @@ -8,6 +8,9 @@ problem function-size a.c:another_function() 11 problem file-size b.c 15 # This is removed, and so will produce an error. # problem function-size b.c:foo() 4 +# This exception isn't used. +problem file-size z.c 100 + problem function-size b.c:bar() 5 problem dependency-violation a.c 3 problem dependency-violation header.h 3 From 82ad63ef404c4634ea98352febe70296918df28d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 18 Sep 2019 08:56:47 -0400 Subject: [PATCH 4/4] changes file for 31338 --- changes/ticket31338 | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changes/ticket31338 diff --git a/changes/ticket31338 b/changes/ticket31338 new file mode 100644 index 0000000000..b76add635d --- /dev/null +++ b/changes/ticket31338 @@ -0,0 +1,4 @@ + o Minor bugfixes (best practices tracker): + - When listing overbroad exceptions, do not also list problems, + and do not list insufficiently broad exceptions. Fixes bug 31338; + bugfix on 0.4.2.1-alpha.