Exit with 1 if new issues were found. Also work with python3.

scripts/maint/practracker/practracker.py

@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 
 """
 Tor code best-practices tracker
@@ -47,69 +47,88 @@ ProblemVault = None
 #######################################################
 
 def consider_file_size(fname, f):
-    file_size = metrics.file_len(f)
+    """Consider file size issues for 'f' and return True if a new issue was found"""
+    file_size = metrics.get_file_len(f)
     if file_size > MAX_FILE_SIZE:
-        v = problem.FileSizeProblem(fname, file_size)
-        ProblemVault.register_problem(v)
+        p = problem.FileSizeProblem(fname, file_size)
+        return ProblemVault.register_problem(p)
+    return False
 
 def consider_includes(fname, f):
+    """Consider #include issues for 'f' and return True if a new issue was found"""
     include_count = metrics.get_include_count(f)
 
     if include_count > MAX_INCLUDE_COUNT:
-        v = problem.IncludeCountProblem(fname, include_count)
-        ProblemVault.register_problem(v)
+        p = problem.IncludeCountProblem(fname, include_count)
+        return ProblemVault.register_problem(p)
+    return False
 
 def consider_function_size(fname, f):
-    for name, lines in metrics.function_lines(f):
+    """Consider the function sizes for 'f' and return True if a new issue was found"""
+    found_new_issues = False
+
+    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)
-        v = problem.FunctionSizeProblem(canonical_function_name, lines)
-        ProblemVault.register_problem(v)
+        p = problem.FunctionSizeProblem(canonical_function_name, lines)
+        found_new_issues |= ProblemVault.register_problem(p)
+
+    return found_new_issues
 
 #######################################################
 
 def consider_all_metrics(files_list):
-    """Consider metrics for all files"""
+    """Consider metrics for all files, and return True if new issues were found"""
+    found_new_issues = False
     for fname in files_list:
         with open(fname, 'r') as f:
-            consider_metrics_for_file(fname, f)
+            found_new_issues |= consider_metrics_for_file(fname, f)
+    return found_new_issues
 
 def consider_metrics_for_file(fname, f):
     """
-    Get metrics for file with filename 'fname' and file descriptor 'f'.
+    Consider the various metrics for file with filename 'fname' and file descriptor 'f'.
+    Return True if we found new issues.
     """
+    # Strip the useless part of the path
+    if fname.startswith(TOR_TOPDIR):
+        fname = fname[len(TOR_TOPDIR):]
+
+    found_new_issues = False
+
     # Get file length
-    consider_file_size(fname, f)
+    found_new_issues |= consider_file_size(fname, f)
 
     # Consider number of #includes
     f.seek(0)
-    consider_includes(fname, f)
+    found_new_issues |= consider_includes(fname, f)
 
     # Get function length
     f.seek(0)
-    consider_function_size(fname, f)
+    found_new_issues |= consider_function_size(fname, f)
+
+    return found_new_issues
 
 def main():
-    global ProblemVault
-
     # 1) Get all the .c files we care about
     files_list = util.get_tor_c_files(TOR_TOPDIR, EXCLUDE_SOURCE_DIRS)
 
     # 2) Initialize problem vault and load an optional exceptions file so that
     # we don't warn about the past
-    try:
-        with open(EXCEPTIONS_FILE, 'r') as exception_f:
-            ProblemVault = problem.ProblemVault(exception_f)
-    except IOError:
-        print "No exception file provided"
-        ProblemVault = problem.ProblemVault(None)
+    global ProblemVault
+    ProblemVault = problem.ProblemVault(EXCEPTIONS_FILE)
 
     # 3) Go through all the files and report problems if they are not exceptions
-    consider_all_metrics(files_list)
+    found_new_issues = consider_all_metrics(files_list)
+
+    if found_new_issues:
+        sys.exit(1)
+    else:
+        sys.exit(0)
 
 if __name__ == '__main__':
     main()

scripts/maint/practracker/problem.py

@@ -13,12 +13,15 @@ class ProblemVault(object):
     found in the code, and also the old problems we read from the exception
     file.
     """
-    def __init__(self, exception_file):
+    def __init__(self, exception_fname):
         # Exception dictionary: { problem.key() : Problem object }
         self.exceptions = {}
 
-        if exception_file:
-            self.register_exceptions(exception_file)
+        try:
+            with open(exception_fname, 'r') as exception_f:
+                self.register_exceptions(exception_f)
+        except IOError:
+            print("No exception file provided")
 
     def register_exceptions(self, exception_file):
         # Register exceptions
@@ -27,25 +30,27 @@ class ProblemVault(object):
             if problem is None:
                 continue
 
-            # XXX this might overwrite problems with the same key (e.g. MOCK_IMPL)
             self.exceptions[problem.key()] = problem
             #print "Registering exception: %s" % problem
 
     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.
+        """
         # This is a new problem, print it
         if problem.key() not in self.exceptions:
-            print problem
-            return
+            print(problem)
+            return True
 
         # 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
-#        else:
-#            print "OK %s better than %s" % (problem, self.exceptions[problem.key()])
+            print(problem)
+            return True
 
+        return False
 
 class Problem(object):
     def __init__(self, problem_type, problem_location, metric_value):
@@ -91,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 %s" % exception_str
+        print("Unknown exception line {}".format(exception_str))
         return None