August 2019 - tor-commits - lists.torproject.org

[tor/master] Fix practracker_tests.py and practracker line counting.
by dgoulet＠torproject.org 01 Aug '19

01 Aug '19

commit 4c095329964c47c7a37c812fdaf58b9238cc9ff5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 17 14:00:28 2019 +0200 Fix practracker_tests.py and practracker line counting. The practracker_tests.py unit test file called a function by its old name. Also, practracker counted functions as starting one line after the function name, and ending with the closing brace. Now they start with the open brace and end with the closing brace. --- scripts/maint/practracker/metrics.py | 8 ++++++-- scripts/maint/practracker/practracker_tests.py | 8 +++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/scripts/maint/practracker/metrics.py b/scripts/maint/practracker/metrics.py index 5fa305a86..b35029b51 100644 --- a/scripts/maint/practracker/metrics.py +++ b/scripts/maint/practracker/metrics.py @@ -31,6 +31,7 @@ def get_function_lines(f): "DISABLE_GCC_WARNING", "DISABLE_GCC_WARNINGS"} in_function = False + found_openbrace = False for lineno, line in enumerate(f): if not in_function: # find the start of a function @@ -41,10 +42,13 @@ def get_function_lines(f): continue func_start = lineno in_function = True - + elif not found_openbrace and line.startswith("{"): + found_openbrace = True + func_start = lineno else: # Find the end of a function if line.startswith("}"): - n_lines = lineno - func_start + n_lines = lineno - func_start + 1 in_function = False + found_openbrace = False yield (func_name, n_lines) diff --git a/scripts/maint/practracker/practracker_tests.py b/scripts/maint/practracker/practracker_tests.py index cdbab2908..f11c55a3b 100755 --- a/scripts/maint/practracker/practracker_tests.py +++ b/scripts/maint/practracker/practracker_tests.py @@ -1,3 +1,5 @@ +#!/usr/bin/python + """Some simple tests for practracker metrics""" import unittest @@ -38,13 +40,13 @@ class TestFunctionLength(unittest.TestCase): def test_function_length(self): funcs = StringIO.StringIO(function_file) # All functions should have length 2 - for name, lines in metrics.function_lines(funcs): + for name, lines in metrics.get_function_lines(funcs): self.assertEqual(name, "fun") funcs.seek(0) - for name, lines in metrics.function_lines(funcs): - self.assertEqual(lines, 2) + for name, lines in metrics.get_function_lines(funcs): + self.assertEqual(lines, 4) if __name__ == '__main__': unittest.main()

1 0

[tor/master] Practracker: add a --list-overstrict option
by dgoulet＠torproject.org 01 Aug '19

01 Aug '19

commit a5e1fa3a036b0200e049625c427080881b276730 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 17 15:06:34 2019 +0200 Practracker: add a --list-overstrict option This option lists every exception that is stricter than it needs to be. Part of 30752 --- scripts/maint/practracker/practracker.py | 12 ++++++++++++ scripts/maint/practracker/problem.py | 17 +++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 0dc63f765..1b6502fe5 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -56,6 +56,7 @@ else: def consider_file_size(fname, f): """Consider file size issues for 'f' and return the number of new issues was found""" file_size = metrics.get_file_len(f) + if file_size > MAX_FILE_SIZE: p = problem.FileSizeProblem(fname, file_size) if ProblemVault.register_problem(p): @@ -164,6 +165,8 @@ def main(argv): parser = argparse.ArgumentParser(prog=progname) parser.add_argument("--regen", action="store_true", help="Regenerate the exceptions file") + parser.add_argument("--list-overstrict", action="store_true", + help="List over-strict exceptions") parser.add_argument("--exceptions", help="Override the location for the exceptions file") parser.add_argument("topdir", default=".", nargs="?", @@ -213,6 +216,15 @@ See doc/HACKING/HelpfulTools.md for more information on using practracker.\ """.format(found_new_issues, exceptions_file) print(new_issues_str) + if args.list_overstrict: + def k_fn(tup): + return tup[0].key() + for (ex,p) in sorted(ProblemVault.list_overstrict_exceptions(), key=k_fn): + if p is None: + print(ex, "->", 0) + 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 c82c5db57..751ceb910 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -22,6 +22,9 @@ class ProblemVault(object): def __init__(self, exception_fname=None): # Exception dictionary: { problem.key() : Problem object } self.exceptions = {} + # Exception dictionary: maps key to the problem it was used to + # suppress. + self.used_exception_for = {} if exception_fname == None: return @@ -71,9 +74,23 @@ class ProblemVault(object): if problem.is_worse_than(self.exceptions[problem.key()]): print(problem) return True + else: + self.used_exception_for[problem.key()] = problem return False + def list_overstrict_exceptions(self): + """Return an iterator of tuples containing (ex,prob) where ex is an + exceptions in this vault that are stricter than it needs to be, and + prob is the worst problem (if any) that it covered. + """ + for k in self.exceptions: + e = self.exceptions[k] + p = self.used_exception_for.get(k) + if p is None or e.is_worse_than(p): + yield (e, p) + + class Problem(object): """ A generic problem in our source code. See the subclasses below for the

1 0

[tor/master] Practracker: add tolerances for exceptions
by dgoulet＠torproject.org 01 Aug '19

01 Aug '19

commit 6303c9aa2647365fa66de2b6ce1af109417b6603 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 17 15:20:58 2019 +0200 Practracker: add tolerances for exceptions When an exception is present, we can now violate the limit by a little bit and only produce a warning. The strict flag overrides this behavior. I've given file sizes a 2% tolerances and function sizes/include counts a 10% tolerance. Part of 30752 --- scripts/maint/practracker/practracker.py | 14 ++++++++++++++ scripts/maint/practracker/problem.py | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 1b6502fe5..75cd44d22 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -36,6 +36,13 @@ MAX_FUNCTION_SIZE = 100 # lines # Recommended number of #includes MAX_INCLUDE_COUNT = 50 +# Map from problem type to functions that adjust for tolerance +TOLERANCE_FNS = { + 'include-count': lambda n: int(n*1.1), + 'function-size': lambda n: int(n*1.1), + 'file-size': lambda n: int(n*1.02) +} + ####################################################### # ProblemVault singleton @@ -169,6 +176,8 @@ def main(argv): help="List over-strict exceptions") parser.add_argument("--exceptions", help="Override the location for the exceptions file") + parser.add_argument("--strict", action="store_true", + help="Make all warnings into errors") parser.add_argument("topdir", default=".", nargs="?", help="Top-level directory for the tor source") args = parser.parse_args(argv[1:]) @@ -196,6 +205,11 @@ def main(argv): else: ProblemVault = problem.ProblemVault(exceptions_file) + # 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_overstrict or args.strict): + ProblemVault.set_tolerances(TOLERANCE_FNS) + # 3) Go through all the files and report problems if they are not exceptions found_new_issues = consider_all_metrics(files_list) diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index 751ceb910..89a8f1234 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -90,6 +90,15 @@ class ProblemVault(object): if p is None or e.is_worse_than(p): yield (e, p) + 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 + function to its new maximum value.""" + for k in self.exceptions: + ex = self.exceptions[k] + fn = fns.get(ex.problem_type) + if fn is not None: + ex.metric_value = fn(ex.metric_value) class Problem(object): """ @@ -99,14 +108,21 @@ class Problem(object): def __init__(self, problem_type, problem_location, metric_value): self.problem_location = problem_location self.metric_value = int(metric_value) + self.warning_threshold = self.metric_value self.problem_type = problem_type def is_worse_than(self, other_problem): """Return True if this is a worse problem than other_problem""" if self.metric_value > other_problem.metric_value: return True + elif self.metric_value > other_problem.warning_threshold: + self.warn() return False + def warn(self): + """Warn about this problem on stderr only.""" + print("(warning) {}".format(self), file=sys.stderr) + def key(self): """Generate a unique key that describes this problem that can be used as a dictionary key""" # Problem location is a filesystem path, so we need to normalize this

1 0

[tor/master] practracker: Rename "Problem" to "Item".
by dgoulet＠torproject.org 01 Aug '19

01 Aug '19

commit ec13a727b0120485d5a90dcd053b3503b3080b58 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 30 09:03:55 2019 -0400 practracker: Rename "Problem" to "Item". I'm about to refactor the code into a set of iterators that yield *all* the metrics for the code, and then add a filter on top of that to return the problems. --- scripts/maint/practracker/practracker.py | 6 +++--- scripts/maint/practracker/problem.py | 26 +++++++++++++------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 70035a0ab..bcced93c5 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -65,7 +65,7 @@ def consider_file_size(fname, f): file_size = metrics.get_file_len(f) if file_size > MAX_FILE_SIZE: - p = problem.FileSizeProblem(fname, file_size) + p = problem.FileSizeItem(fname, file_size) if ProblemVault.register_problem(p): return 1 return 0 @@ -75,7 +75,7 @@ def consider_includes(fname, f): include_count = metrics.get_include_count(f) if include_count > MAX_INCLUDE_COUNT: - p = problem.IncludeCountProblem(fname, include_count) + p = problem.IncludeCountItem(fname, include_count) if ProblemVault.register_problem(p): return 1 return 0 @@ -91,7 +91,7 @@ def consider_function_size(fname, f): # That's a big function! Issue a problem! canonical_function_name = "%s:%s()" % (fname, name) - p = problem.FunctionSizeProblem(canonical_function_name, lines) + p = problem.FunctionSizeItem(canonical_function_name, lines) if ProblemVault.register_problem(p): found_new_issues += 1 diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index 89a8f1234..317e2a4a5 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -100,10 +100,10 @@ class ProblemVault(object): if fn is not None: ex.metric_value = fn(ex.metric_value) -class Problem(object): +class Item(object): """ - A generic problem in our source code. See the subclasses below for the - specific problems we are trying to tackle. + A generic measurement about some aspect of our source code. See + the subclasses below for the specific problems we are trying to tackle. """ def __init__(self, problem_type, problem_location, metric_value): self.problem_location = problem_location @@ -125,7 +125,7 @@ class Problem(object): def key(self): """Generate a unique key that describes this problem that can be used as a dictionary key""" - # Problem location is a filesystem path, so we need to normalize this + # Item location is a filesystem path, so we need to normalize this # across platforms otherwise same paths are not gonna match. canonical_location = os.path.normcase(self.problem_location) return "%s:%s" % (canonical_location, self.problem_type) @@ -133,7 +133,7 @@ class Problem(object): def __str__(self): return "problem %s %s %s" % (self.problem_type, self.problem_location, self.metric_value) -class FileSizeProblem(Problem): +class FileSizeItem(Item): """ Denotes a problem with the size of a .c file. @@ -141,9 +141,9 @@ class FileSizeProblem(Problem): 'metric_value' is the number of lines in the .c file. """ def __init__(self, problem_location, metric_value): - super(FileSizeProblem, self).__init__("file-size", problem_location, metric_value) + super(FileSizeItem, self).__init__("file-size", problem_location, metric_value) -class IncludeCountProblem(Problem): +class IncludeCountItem(Item): """ Denotes a problem with the number of #includes in a .c file. @@ -151,9 +151,9 @@ class IncludeCountProblem(Problem): 'metric_value' is the number of #includes in the .c file. """ def __init__(self, problem_location, metric_value): - super(IncludeCountProblem, self).__init__("include-count", problem_location, metric_value) + super(IncludeCountItem, self).__init__("include-count", problem_location, metric_value) -class FunctionSizeProblem(Problem): +class FunctionSizeItem(Item): """ Denotes a problem with a size of a function in a .c file. @@ -164,7 +164,7 @@ class FunctionSizeProblem(Problem): The 'metric_value' is the size of the offending function in lines. """ def __init__(self, problem_location, metric_value): - super(FunctionSizeProblem, self).__init__("function-size", problem_location, metric_value) + super(FunctionSizeItem, self).__init__("function-size", problem_location, metric_value) comment_re = re.compile(r'#.*$') @@ -182,10 +182,10 @@ def get_old_problem_from_exception_str(exception_str): raise ValueError("Misformatted line {!r}".format(orig_str)) if problem_type == "file-size": - return FileSizeProblem(problem_location, metric_value) + return FileSizeItem(problem_location, metric_value) elif problem_type == "include-count": - return IncludeCountProblem(problem_location, metric_value) + return IncludeCountItem(problem_location, metric_value) elif problem_type == "function-size": - return FunctionSizeProblem(problem_location, metric_value) + return FunctionSizeItem(problem_location, metric_value) else: raise ValueError("Unknown exception type {!r}".format(orig_str))

1 0

[tor/master] changes file for 30752
by dgoulet＠torproject.org 01 Aug '19

01 Aug '19

commit 3efe5cc57af040f8adb9a9e24a6ad1e77998a7a1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Jul 18 09:27:58 2019 -0400 changes file for 30752 --- changes/ticket30752 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/changes/ticket30752 b/changes/ticket30752 new file mode 100644 index 000000000..754a9003d --- /dev/null +++ b/changes/ticket30752 @@ -0,0 +1,6 @@ + o Minor features (best practices tracker): + - Give a warning rather than an error when a practracker exception is + violated by a small amount; add a --list-overstrict option to + practracker that lists exceptions that are stricter than they need to + be, and provide an environment variable for disabling + practracker. Closes ticekt 30752.

1 0

[tor/master] Add a TOR_DISABLE_PRACTRACKER envvar for use by folks who don't care
by dgoulet＠torproject.org 01 Aug '19

01 Aug '19

commit d6a3636cdcb2556e99744dc3685db2010e151291 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 17 15:28:48 2019 +0200 Add a TOR_DISABLE_PRACTRACKER envvar for use by folks who don't care Fixes part of bug 30752 --- Makefile.am | 4 +++- scripts/maint/practracker/practracker.py | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index e823f9e10..d6ea72c17 100644 --- a/Makefile.am +++ b/Makefile.am @@ -369,7 +369,9 @@ endif check-best-practices: if USEPYTHON - $(PYTHON) $(top_srcdir)/scripts/maint/practracker/practracker.py $(top_srcdir) + @if test "$$TOR_DISABLE_PRACTRACKER" = ""; then \ + $(PYTHON) $(top_srcdir)/scripts/maint/practracker/practracker.py $(top_srcdir); \ + fi endif practracker-regen: diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 75cd44d22..70035a0ab 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -227,6 +227,9 @@ Please fix the problems if you can, and update the exceptions file ({}) if you can't. See doc/HACKING/HelpfulTools.md for more information on using practracker.\ + +You can disable this message by setting the TOR_DISABLE_PRACTRACKER environment +variable. """.format(found_new_issues, exceptions_file) print(new_issues_str)

1 0

[tor/master] practracker: Refactor flow to use generators
by dgoulet＠torproject.org 01 Aug '19

01 Aug '19

commit bcef6a580254234c7396761f300cca79c10d21e5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 30 09:20:08 2019 -0400 practracker: Refactor flow to use generators Instead of having "consider" functions that have to call a global ProblemVault, we can now generate all the metrics for the code separately from the decision about what to do for them. --- scripts/maint/practracker/practracker.py | 72 +++++++++++++++----------------- scripts/maint/practracker/problem.py | 21 ++++++++++ 2 files changed, 54 insertions(+), 39 deletions(-) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index bcced93c5..92de021e0 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -36,6 +36,7 @@ MAX_FUNCTION_SIZE = 100 # lines # Recommended number of #includes MAX_INCLUDE_COUNT = 50 + # Map from problem type to functions that adjust for tolerance TOLERANCE_FNS = { 'include-count': lambda n: int(n*1.1), @@ -51,6 +52,12 @@ ProblemVault = None # The Tor source code topdir TOR_TOPDIR = None +# ProblemFilter singleton. +FILTER = problem.ProblemFilter() +FILTER.addThreshold(problem.FileSizeItem("*", MAX_FILE_SIZE)) +FILTER.addThreshold(problem.IncludeCountItem("*", MAX_INCLUDE_COUNT)) +FILTER.addThreshold(problem.FunctionSizeItem("*", MAX_FUNCTION_SIZE)) + ####################################################### if sys.version_info[0] <= 2: @@ -61,75 +68,59 @@ else: return open(fname, 'r', encoding='utf-8') def consider_file_size(fname, f): - """Consider file size issues for 'f' and return the number of new issues was found""" + """Consider the size of 'f' and yield an FileSizeItem for it. + """ file_size = metrics.get_file_len(f) - - if file_size > MAX_FILE_SIZE: - p = problem.FileSizeItem(fname, file_size) - if ProblemVault.register_problem(p): - return 1 - return 0 + yield problem.FileSizeItem(fname, file_size) def consider_includes(fname, f): - """Consider #include issues for 'f' and return the number of new issues found""" + """Consider the #include count in for 'f' and yield an IncludeCountItem + for it. + """ include_count = metrics.get_include_count(f) - if include_count > MAX_INCLUDE_COUNT: - p = problem.IncludeCountItem(fname, include_count) - if ProblemVault.register_problem(p): - return 1 - return 0 + yield problem.IncludeCountItem(fname, include_count) def consider_function_size(fname, f): - """Consider the function sizes for 'f' and return the number of new issues found.""" - found_new_issues = 0 + """yield a FunctionSizeItem for every function in f. + """ for name, lines in metrics.get_function_lines(f): - # Don't worry about functions within our limits - if lines <= MAX_FUNCTION_SIZE: - continue - - # That's a big function! Issue a problem! canonical_function_name = "%s:%s()" % (fname, name) - p = problem.FunctionSizeItem(canonical_function_name, lines) - if ProblemVault.register_problem(p): - found_new_issues += 1 - - return found_new_issues + yield problem.FunctionSizeItem(canonical_function_name, lines) ####################################################### def consider_all_metrics(files_list): - """Consider metrics for all files, and return the number of new issues found.""" - found_new_issues = 0 + """Consider metrics for all files, and yield a sequence of problem.Item + object for those issues.""" for fname in files_list: with open_file(fname) as f: - found_new_issues += consider_metrics_for_file(fname, f) - return found_new_issues + for item in consider_metrics_for_file(fname, f): + yield item def consider_metrics_for_file(fname, f): """ - Consider the various metrics for file with filename 'fname' and file descriptor 'f'. - Return the number of new issues found. + Yield a sequence of problem.Item objects for all of the metrics in + 'f'. """ # Strip the useless part of the path if fname.startswith(TOR_TOPDIR): fname = fname[len(TOR_TOPDIR):] - found_new_issues = 0 - # Get file length - found_new_issues += consider_file_size(fname, f) + for item in consider_file_size(fname, f): + yield item # Consider number of #includes f.seek(0) - found_new_issues += consider_includes(fname, f) + for item in consider_includes(fname, f): + yield item # Get function length f.seek(0) - found_new_issues += consider_function_size(fname, f) - - return found_new_issues + for item in consider_function_size(fname, f): + yield item HEADER="""\ # Welcome to the exceptions file for Tor's best-practices tracker! @@ -211,7 +202,10 @@ def main(argv): ProblemVault.set_tolerances(TOLERANCE_FNS) # 3) Go through all the files and report problems if they are not exceptions - found_new_issues = consider_all_metrics(files_list) + found_new_issues = 0 + for item in FILTER.filter(consider_all_metrics(files_list)): + if ProblemVault.register_problem(item): + found_new_issues += 1 if args.regen: tmpfile.close() diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index 317e2a4a5..cf804ad43 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -100,6 +100,24 @@ class ProblemVault(object): if fn is not None: ex.metric_value = fn(ex.metric_value) +class ProblemFilter(object): + def __init__(self): + self.thresholds = dict() + + def addThreshold(self, item): + self.thresholds[item.get_type()] = item + + def matches(self, item): + filt = self.thresholds.get(item.get_type(), None) + if filt is None: + return False + return item.is_worse_than(filt) + + def filter(self, sequence): + for item in iter(sequence): + if self.matches(item): + yield item + class Item(object): """ A generic measurement about some aspect of our source code. See @@ -133,6 +151,9 @@ class Item(object): def __str__(self): return "problem %s %s %s" % (self.problem_type, self.problem_location, self.metric_value) + def get_type(self): + return self.problem_type + class FileSizeItem(Item): """ Denotes a problem with the size of a .c file.

1 0

[tor/master] Practracker: new flags to control output.
by dgoulet＠torproject.org 01 Aug '19

01 Aug '19

commit 3f303c102a3adea24d8a3f513c8528eb417d5283 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 30 11:49:50 2019 -0400 Practracker: new flags to control output. These flags let you suppress the message about the number of problems and warnings, and let you control the thresholds above which something counts as a problem. I need this for testing. --- scripts/maint/practracker/practracker.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 3bf16dcde..fb6649bdc 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -36,7 +36,6 @@ MAX_FUNCTION_SIZE = 100 # lines # Recommended number of #includes MAX_INCLUDE_COUNT = 50 - # Map from problem type to functions that adjust for tolerance TOLERANCE_FNS = { 'include-count': lambda n: int(n*1.1), @@ -49,12 +48,6 @@ TOLERANCE_FNS = { # The Tor source code topdir TOR_TOPDIR = None -# ProblemFilter singleton. -FILTER = problem.ProblemFilter() -FILTER.addThreshold(problem.FileSizeItem("*", MAX_FILE_SIZE)) -FILTER.addThreshold(problem.IncludeCountItem("*", MAX_INCLUDE_COUNT)) -FILTER.addThreshold(problem.FunctionSizeItem("*", MAX_FUNCTION_SIZE)) - ####################################################### if sys.version_info[0] <= 2: @@ -166,6 +159,14 @@ def main(argv): help="Override the location for the exceptions file") parser.add_argument("--strict", action="store_true", help="Make all warnings into errors") + parser.add_argument("--terse", action="store_true", + help="Do not emit helpful instructions.") + parser.add_argument("--max-file-size", default=MAX_FILE_SIZE, + help="Maximum lines per C file size") + parser.add_argument("--max-include-count", default=MAX_INCLUDE_COUNT, + help="Maximum includes per C file") + parser.add_argument("--max-function-size", default=MAX_FUNCTION_SIZE, + help="Maximum lines per function") parser.add_argument("topdir", default=".", nargs="?", help="Top-level directory for the tor source") args = parser.parse_args(argv[1:]) @@ -177,6 +178,12 @@ def main(argv): else: exceptions_file = os.path.join(TOR_TOPDIR, "scripts/maint/practracker", EXCEPTIONS_FNAME) + # 0) Configure our thresholds of "what is a problem actually" + filt = problem.ProblemFilter() + filt.addThreshold(problem.FileSizeItem("*", int(args.max_file_size))) + filt.addThreshold(problem.IncludeCountItem("*", int(args.max_include_count))) + filt.addThreshold(problem.FunctionSizeItem("*", int(args.max_function_size))) + # 1) Get all the .c files we care about files_list = util.get_tor_c_files(TOR_TOPDIR) @@ -198,7 +205,7 @@ def main(argv): # 3) Go through all the files and report problems if they are not exceptions found_new_issues = 0 - for item in FILTER.filter(consider_all_metrics(files_list)): + for item in filt.filter(consider_all_metrics(files_list)): status = ProblemVault.register_problem(item) if status == problem.STATUS_ERR: print(item) @@ -212,7 +219,7 @@ def main(argv): 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: + if found_new_issues and not args.regen and not args.terse: new_issues_str = """\ FAILURE: practracker found {} new problem(s) in the code: see warnings above.

1 0

[tor/master] practracker: Remove problemvault global.
by dgoulet＠torproject.org 01 Aug '19

01 Aug '19

commit 31a0b818542ea459db996daa0dd364f32b310474 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 30 10:17:19 2019 -0400 practracker: Remove problemvault global. --- scripts/maint/practracker/practracker.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 0e6490aab..3bf16dcde 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -46,9 +46,6 @@ TOLERANCE_FNS = { ####################################################### -# ProblemVault singleton -ProblemVault = None - # The Tor source code topdir TOR_TOPDIR = None @@ -185,8 +182,6 @@ def main(argv): # 2) Initialize problem vault and load an optional exceptions file so that # we don't warn about the past - global ProblemVault - if args.regen: tmpname = exceptions_file + ".tmp" tmpfile = open(tmpname, "w")

1 0

[tor/master] practracker: Move the warning/error distinction to a higher level.
by dgoulet＠torproject.org 01 Aug '19

01 Aug '19

commit 65cb4fead50903e7df40f593f12e56902b63458f Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 30 10:15:11 2019 -0400 practracker: Move the warning/error distinction to a higher level. Previously warnings were generated by magic inside ProblemVault; now they're printed on demand. --- scripts/maint/practracker/practracker.py | 6 +++++- scripts/maint/practracker/problem.py | 34 ++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 92de021e0..0e6490aab 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -204,8 +204,12 @@ def main(argv): # 3) Go through all the files and report problems if they are not exceptions found_new_issues = 0 for item in FILTER.filter(consider_all_metrics(files_list)): - if ProblemVault.register_problem(item): + status = ProblemVault.register_problem(item) + if status == problem.STATUS_ERR: + print(item) found_new_issues += 1 + elif status == problem.STATUS_WARN: + item.warn() if args.regen: tmpfile.close() diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index cf804ad43..d162e19ef 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -13,6 +13,10 @@ import os.path import re import sys +STATUS_ERR = 2 +STATUS_WARN = 1 +STATUS_OK = 0 + class ProblemVault(object): """ Singleton where we store the various new problems we @@ -60,24 +64,23 @@ class ProblemVault(object): def register_problem(self, problem): """ - Register this problem to the problem value. Return True if it was a new - problem or it worsens an already existing problem. + Register this problem to the problem value. Return true if it was a new + problem or it worsens an already existing problem. A true + value may be STATUS_ERR to indicate a hard violation, or STATUS_WARN + to indicate a warning. """ # This is a new problem, print it if problem.key() not in self.exceptions: - print(problem) - return True + return STATUS_ERR # If it's an old problem, we don't warn if the situation got better # (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). - if problem.is_worse_than(self.exceptions[problem.key()]): - print(problem) - return True - else: + status = problem.is_worse_than(self.exceptions[problem.key()]) + if status == STATUS_OK: self.used_exception_for[problem.key()] = problem - return False + return status def list_overstrict_exceptions(self): """Return an iterator of tuples containing (ex,prob) where ex is an @@ -130,12 +133,17 @@ class Item(object): self.problem_type = problem_type def is_worse_than(self, other_problem): - """Return True if this is a worse problem than other_problem""" + """Return STATUS_ERR if this is a worse problem than other_problem. + Return STATUS_WARN if it is a little worse, but falls within the + warning threshold. Return STATUS_OK if this problem is not + at all worse than other_problem. + """ if self.metric_value > other_problem.metric_value: - return True + return STATUS_ERR elif self.metric_value > other_problem.warning_threshold: - self.warn() - return False + return STATUS_WARN + else: + return STATUS_OK def warn(self): """Warn about this problem on stderr only."""

1 0