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

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

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()

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
 

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)