diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 4fbca615c5..3c22abd444 100755 --- a/scripts/maint/practracker/practracker.py +++ b/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) + canonical_function_name = "%s:%s()" % (fname, name) + 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() diff --git a/scripts/maint/practracker/problem.py b/scripts/maint/practracker/problem.py index fe6b30915b..e23d3bbf0e 100644 --- a/scripts/maint/practracker/problem.py +++ b/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