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.

1 files changed, 31 insertions, 23 deletions

diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py
index 3c22abd444..541c5cbfb3 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()