Merge remote-tracking branch 'public/practracker_regen_overbroad_2'

This commit is contained in:
Nick Mathewson 2020-02-06 08:27:12 -05:00
commit b7bbff0c9c
7 changed files with 154 additions and 6 deletions

View File

@ -196,6 +196,8 @@ EXTRA_DIST+= \
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/ex1-regen-expected.txt \
scripts/maint/practracker/testdata/ex1-regen-overbroad-expected.txt \
scripts/maint/practracker/testdata/ex.txt \
scripts/maint/practracker/testdata/header.h \
scripts/maint/practracker/testdata/not_c_file \

4
changes/ticket32372 Normal file
View File

@ -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.

View File

@ -30,6 +30,7 @@ import metrics
import util
import problem
import includes
import shutil
# The filename of the exceptions file (it should be placed in the practracker directory)
EXCEPTIONS_FNAME = "./exceptions.txt"
@ -185,6 +186,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 +231,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 +252,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
@ -269,7 +276,17 @@ def main(argv):
if args.regen:
tmpfile.close()
os.rename(tmpname, exceptions_file)
shutil.move(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()
shutil.move(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.
@ -296,6 +313,7 @@ variable.
else:
print(ex, "->", p.metric_value)
sys.exit(found_new_issues)
if __name__ == '__main__':

View File

@ -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

View File

@ -82,3 +82,15 @@ run_practracker --exceptions "${DATA}/ex1.txt" --list-overbroad \
compare "${TMPDIR}/ex1-overbroad-received.txt" \
"${DATA}/ex1-overbroad-expected.txt"
echo "ex1.regen:"
cp "${DATA}/ex1.txt" "${TMPDIR}/ex1-copy.txt"
run_practracker --exceptions "${TMPDIR}/ex1-copy.txt" --regen >/dev/null 2>&1
compare "${TMPDIR}/ex1-copy.txt" "${DATA}/ex1-regen-expected.txt"
echo "ex1.regen_overbroad:"
cp "${DATA}/ex1.txt" "${TMPDIR}/ex1-copy.txt"
run_practracker --exceptions "${TMPDIR}/ex1-copy.txt" --regen-overbroad >/dev/null 2>&1
compare "${TMPDIR}/ex1-copy.txt" "${DATA}/ex1-regen-overbroad-expected.txt"

View File

@ -0,0 +1,46 @@
# Welcome to the exceptions file for Tor's best-practices tracker!
#
# Each line of this file represents a single violation of Tor's best
# practices -- typically, a violation that we had before practracker.py
# first existed.
#
# There are three kinds of problems that we recognize right now:
# function-size -- a function of more than 100 lines.
# file-size -- a .c file of more than 3000 lines, or a .h
# file with more than 500 lines.
# include-count -- a .c file with more than 50 #includes,
# or a .h file with more than 15 #includes.
# dependency-violation -- a file includes a header that it should
# not, according to an advisory .may_include file.
#
# Each line below represents a single exception that practracker should
# _ignore_. Each line has four parts:
# 1. The word "problem".
# 2. The kind of problem.
# 3. The location of the problem: either a filename, or a
# filename:functionname pair.
# 4. The magnitude of the problem to ignore.
#
# So for example, consider this line:
# problem file-size /src/core/or/connection_or.c 3200
#
# It tells practracker to allow the mentioned file to be up to 3200 lines
# long, even though ordinarily it would warn about any file with more than
# 3000 lines.
#
# You can either edit this file by hand, or regenerate it completely by
# running `make practracker-regen`.
#
# Remember: It is better to fix the problem than to add a new exception!
problem file-size a.c 41
problem include-count a.c 6
problem function-size a.c:i_am_a_function() 9
problem function-size a.c:another_function() 12
problem dependency-violation a.c 4
problem file-size b.c 15
problem function-size b.c:foo() 4
problem function-size b.c:bar() 5
problem file-size header.h 8
problem include-count header.h 4
problem dependency-violation header.h 3

View File

@ -0,0 +1,45 @@
# Welcome to the exceptions file for Tor's best-practices tracker!
#
# Each line of this file represents a single violation of Tor's best
# practices -- typically, a violation that we had before practracker.py
# first existed.
#
# There are three kinds of problems that we recognize right now:
# function-size -- a function of more than 100 lines.
# file-size -- a .c file of more than 3000 lines, or a .h
# file with more than 500 lines.
# include-count -- a .c file with more than 50 #includes,
# or a .h file with more than 15 #includes.
# dependency-violation -- a file includes a header that it should
# not, according to an advisory .may_include file.
#
# Each line below represents a single exception that practracker should
# _ignore_. Each line has four parts:
# 1. The word "problem".
# 2. The kind of problem.
# 3. The location of the problem: either a filename, or a
# filename:functionname pair.
# 4. The magnitude of the problem to ignore.
#
# So for example, consider this line:
# problem file-size /src/core/or/connection_or.c 3200
#
# It tells practracker to allow the mentioned file to be up to 3200 lines
# long, even though ordinarily it would warn about any file with more than
# 3000 lines.
#
# You can either edit this file by hand, or regenerate it completely by
# running `make practracker-regen`.
#
# Remember: It is better to fix the problem than to add a new exception!
problem file-size a.c 41
problem include-count a.c 6
problem function-size a.c:i_am_a_function() 8
problem function-size a.c:another_function() 11
problem file-size b.c 15
problem function-size b.c:bar() 5
problem dependency-violation a.c 4
problem dependency-violation header.h 3
problem file-size header.h 8
problem include-count header.h 4