From 301e3f22eff5021b0855a70cf98a1ccee69a19a8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 25 Mar 2019 15:51:48 -0400 Subject: [PATCH 1/5] Practracker: add a string explaining the excptions file. --- scripts/maint/practracker/practracker.py | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index a6e6d0b607..aec7c8b295 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -113,6 +113,38 @@ def consider_metrics_for_file(fname, f): return found_new_issues +HEADER="""\ +# 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 -- ideally one that was grandfathered in from before practracker.py +# existed. +# +# There are three kinds of problems that we recognize right now: +# function-size -- a function of more than {MAX_FUNCTION_SIZE} lines. +# file-size -- a file of more than {MAX_FILE_SIZE} lines. +# include-count -- a file with more than {MAX_INCLUDE_COUNT} #includes. +# +# 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 +# {MAX_FILE_SIZE} lines. +# +# You can either edit this file by hand, or regenerate it completely by +# running `make practracker-regen`. + +""".format(**globals()) + def main(): if (len(sys.argv) != 2): print("Usage:\n\t$ practracker.py \n\t(e.g. $ practracker.py ~/tor/)") From 0260e0f6fc46cd196c59d3d2c4551be2f15a7547 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 25 Mar 2019 15:52:43 -0400 Subject: [PATCH 2/5] practracker: pass sys.argv to main() as an argument --- scripts/maint/practracker/practracker.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index aec7c8b295..9f87415218 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -145,8 +145,8 @@ HEADER="""\ """.format(**globals()) -def main(): - if (len(sys.argv) != 2): +def main(argv): + if (len(argv) != 2): print("Usage:\n\t$ practracker.py \n\t(e.g. $ practracker.py ~/tor/)") return @@ -180,4 +180,4 @@ See doc/HACKING/HelpfulTools.md for more information on using practracker.\ sys.exit(found_new_issues) if __name__ == '__main__': - main() + main(sys.argv) From c2643842a9a93f131f7b1244cce253be657acc8f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 25 Mar 2019 16:06:26 -0400 Subject: [PATCH 3/5] practracker: add ability to regenerate exceptions file. Also add a useful argument parser. --- scripts/maint/practracker/practracker.py | 44 +++++++++++++++++++----- scripts/maint/practracker/problem.py | 5 ++- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 9f87415218..4be1a31b51 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -117,8 +117,8 @@ HEADER="""\ # 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 -- ideally one that was grandfathered in from before practracker.py -# existed. +# 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 {MAX_FUNCTION_SIZE} lines. @@ -142,17 +142,30 @@ HEADER="""\ # # 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! """.format(**globals()) def main(argv): - if (len(argv) != 2): - print("Usage:\n\t$ practracker.py \n\t(e.g. $ practracker.py ~/tor/)") - return + import argparse + + progname = argv[0] + parser = argparse.ArgumentParser(prog=progname) + parser.add_argument("--regen", action="store_true", + help="Regenerate the exceptions file") + parser.add_argument("--exceptions", + help="Override the location for the exceptions file") + parser.add_argument("topdir", default=".", nargs="?", + help="Top-level directory for the tor source") + args = parser.parse_args(argv[1:]) global TOR_TOPDIR - TOR_TOPDIR = sys.argv[1] - exceptions_file = os.path.join(TOR_TOPDIR, "scripts/maint/practracker", EXCEPTIONS_FNAME) + TOR_TOPDIR = args.topdir + if args.exceptions: + exceptions_file = args.exceptions + else: + exceptions_file = os.path.join(TOR_TOPDIR, "scripts/maint/practracker", EXCEPTIONS_FNAME) # 1) Get all the .c files we care about files_list = util.get_tor_c_files(TOR_TOPDIR) @@ -160,13 +173,26 @@ def main(argv): # 2) Initialize problem vault and load an optional exceptions file so that # we don't warn about the past global ProblemVault - ProblemVault = problem.ProblemVault(exceptions_file) + + if args.regen: + tmpname = exceptions_file + ".tmp" + tmpfile = open(tmpname, "w") + sys.stdout = tmpfile + sys.stdout.write(HEADER) + ProblemVault = problem.ProblemVault() + else: + ProblemVault = problem.ProblemVault(exceptions_file) # 3) Go through all the files and report problems if they are not exceptions found_new_issues = consider_all_metrics(files_list) + if args.regen: + 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): + if found_new_issues and not args.regen: new_issues_str = """\ FAILURE: practracker found new problems in the code: see warnings above. diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index d5ebedd17f..c82c5db572 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -19,10 +19,13 @@ class ProblemVault(object): found in the code, and also the old problems we read from the exception file. """ - def __init__(self, exception_fname): + def __init__(self, exception_fname=None): # Exception dictionary: { problem.key() : Problem object } self.exceptions = {} + if exception_fname == None: + return + try: with open(exception_fname, 'r') as exception_f: self.register_exceptions(exception_f) From 2c8af79de52d091f98aa94615d9e6dd69faba0c7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 25 Mar 2019 16:07:58 -0400 Subject: [PATCH 4/5] Add a practracker-regen make target --- Makefile.am | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile.am b/Makefile.am index 415c2f2b4e..eea7c3a12f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -352,6 +352,9 @@ if USEPYTHON $(PYTHON) $(top_srcdir)/scripts/maint/practracker/practracker.py $(top_srcdir) endif +practracker-regen: + $(PYTHON) $(top_srcdir)/scripts/maint/practracker/practracker.py --regen $(top_srcdir) + check-docs: all $(PERL) $(top_builddir)/scripts/maint/checkOptionDocs.pl From 39e449434421c8af5d8d6e89b4b4a1bccd78f284 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 26 Mar 2019 08:42:14 -0400 Subject: [PATCH 5/5] practracker: update usage note in docstring --- scripts/maint/practracker/practracker.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 4be1a31b51..c2efa61f2c 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -13,9 +13,9 @@ practracker.py should be run with its second argument pointing to the Tor top-level source directory like this: $ python3 ./scripts/maint/practracker/practracker.py . -The exceptions file is meant to be initialized once with the current state of -the source code and then get saved in the repository for ever after: - $ python3 ./scripts/maint/practracker/practracker.py . > ./scripts/maint/practracker/exceptions.txt +To regenerate the exceptions file so that it allows all current +problems in the Tor source, use the --regen flag: + $ python3 --regen ./scripts/maint/practracker/practracker.py . """ from __future__ import print_function