commit 58de565988160edec1fc5f0fbc2a794a765bce6f Author: George Kadianakis desnacked@riseup.net Date: Thu Feb 28 12:09:10 2019 +0200
Call practracker as part of check-local.
- Introduce 'make check-best-practices'. - Fix up Tor topdir etc to work with the way 'make check-local' gets called. - Make practracker less likely to print useless stuff. --- Makefile.am | 11 +++++-- scripts/maint/practracker/practracker.py | 54 ++++++++++++++++++-------------- scripts/maint/practracker/problem.py | 2 +- scripts/maint/practracker/util.py | 8 +++-- 4 files changed, 47 insertions(+), 28 deletions(-)
diff --git a/Makefile.am b/Makefile.am index b40b2e51b..415c2f2b4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -161,7 +161,9 @@ EXTRA_DIST+= \ README \ ReleaseNotes \ scripts/maint/checkIncludes.py \ - scripts/maint/checkSpace.pl + scripts/maint/checkSpace.pl \ + scripts/maint/practracker +
## This tells etags how to find mockable function definitions. AM_ETAGSFLAGS=--regex='{c}/MOCK_IMPL([^,]+,\W*([a-zA-Z0-9_]+)\W*,/\1/s' @@ -224,7 +226,7 @@ shellcheck: fi; \ fi
-check-local: check-spaces check-changes check-includes shellcheck +check-local: check-spaces check-changes check-includes check-best-practices shellcheck
need-chutney-path: @if test ! -d "$$CHUTNEY_PATH"; then \ @@ -345,6 +347,11 @@ if USEPYTHON $(PYTHON) $(top_srcdir)/scripts/maint/checkIncludes.py endif
+check-best-practices: +if USEPYTHON + $(PYTHON) $(top_srcdir)/scripts/maint/practracker/practracker.py $(top_srcdir) +endif + check-docs: all $(PERL) $(top_builddir)/scripts/maint/checkOptionDocs.pl
diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 3c22abd44..541c5cbfb 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -1,21 +1,21 @@ #!/usr/bin/python3
""" -Tor code best-practices tracker +Best-practices tracker for Tor source code.
Go through the various .c files and collect metrics about them. If the metrics violate some of our best practices and they are not found in the optional -exceptions file ("./exceptions.txt"), then log a problem about them. - -The exceptions file is meant to be initialized with the current state of the -source code as follows: ./practracker.py > ./exceptions.txt +exceptions file, then log a problem about them.
We currently do metrics about file size, function size and number of includes.
-TODO: - - How is this tool supposed to be used? How should the exception file work? - How should the UI work? Does it need special exit codes? - - Fix the function_length function so that practracker_tests.py passes. +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 """
import os, sys @@ -24,14 +24,8 @@ import metrics import util import problem
-# We don't want to run metrics for unittests, automatically-generated C files, -# external libraries or git leftovers. -EXCLUDE_SOURCE_DIRS = ["/src/test/", "/src/trunnel/", "/src/ext/", "/.git/"] - -# Where the Tor source code is -TOR_TOPDIR = "../../../" -# An optional exceptions_file -EXCEPTIONS_FILE = "./exceptions.txt" +# The filename of the exceptions file (it should be placed in the practracker directory) +EXCEPTIONS_FNAME = "./exceptions.txt"
# Recommended file size MAX_FILE_SIZE = 3000 # lines @@ -42,8 +36,12 @@ MAX_INCLUDE_COUNT = 50
#######################################################
+# ProblemVault singleton ProblemVault = None
+# The Tor source code topdir +TOR_TOPDIR = None + #######################################################
def consider_file_size(fname, f): @@ -114,21 +112,31 @@ def consider_metrics_for_file(fname, f): return found_new_issues
def main(): + if (len(sys.argv) != 2): + print("Usage:\n\t$ practracker.py <tor topdir>\n\t(e.g. $ practracker.py ~/tor/)") + return + + global TOR_TOPDIR + TOR_TOPDIR = sys.argv[1] + 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, EXCLUDE_SOURCE_DIRS) + files_list = util.get_tor_c_files(TOR_TOPDIR)
# 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) + 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 found_new_issues: - sys.exit(1) - else: - 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): + new_issues_str = "practracker FAILED as indicated by the problem lines above. Please use the exceptions file ({}) to find any previous state of these problems. If you are unable to fix the underlying best-practices issue right now then you need to either update the relevant exception line or add a new one.".format(exceptions_file) + print(new_issues_str) + + sys.exit(found_new_issues)
if __name__ == '__main__': main() diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index e23d3bbf0..9f5771678 100644 --- a/scripts/maint/practracker/problem.py +++ b/scripts/maint/practracker/problem.py @@ -96,6 +96,6 @@ def get_old_problem_from_exception_str(exception_str): elif problem_type == "function-size": return FunctionSizeProblem(problem_location, metric_value) else: - print("Unknown exception line {}".format(exception_str)) +# print("Unknown exception line '{}'".format(exception_str)) return None
diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py index 8e9d95ece..905cb8a5e 100644 --- a/scripts/maint/practracker/util.py +++ b/scripts/maint/practracker/util.py @@ -1,6 +1,10 @@ import os
-def get_tor_c_files(tor_topdir, exclude_dirs): +# We don't want to run metrics for unittests, automatically-generated C files, +# external libraries or git leftovers. +EXCLUDE_SOURCE_DIRS = ["/src/test/", "/src/trunnel/", "/src/ext/", "/.git/"] + +def get_tor_c_files(tor_topdir): """ Return a list with the .c filenames we want to get metrics of. """ @@ -14,7 +18,7 @@ def get_tor_c_files(tor_topdir, exclude_dirs):
# Exclude the excluded paths full_path = os.path.join(root,filename) - if any(exclude_dir in full_path for exclude_dir in exclude_dirs): + if any(exclude_dir in full_path for exclude_dir in EXCLUDE_SOURCE_DIRS): continue
files_list.append(full_path)