From 651bbe8a0d5fbff261a98314b56f57314ea394cd Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 2 Sep 2019 15:31:31 -0400 Subject: [PATCH 1/3] Practracker: only consider files under "src/" --- changes/ticket31578 | 6 ++++++ scripts/maint/practracker/practracker.py | 5 ++++- scripts/maint/practracker/test_practracker.sh | 1 + scripts/maint/practracker/util.py | 12 +++++++++--- 4 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 changes/ticket31578 diff --git a/changes/ticket31578 b/changes/ticket31578 new file mode 100644 index 0000000000..220efffa63 --- /dev/null +++ b/changes/ticket31578 @@ -0,0 +1,6 @@ + o Minor bugfixes (practracker): + - When running check-best-practices, only consider files in the + src subdirectory. Previously we had recursively considered + all subdirectories, which made us get confused by the + temporary directories made by "make distcheck". Fixes bug + 31578; bugfix on 0.4.1.1-alpha. diff --git a/scripts/maint/practracker/practracker.py b/scripts/maint/practracker/practracker.py index 6483b88da1..537f755c8b 100755 --- a/scripts/maint/practracker/practracker.py +++ b/scripts/maint/practracker/practracker.py @@ -195,6 +195,9 @@ def main(argv): help="Maximum lines per function") parser.add_argument("--max-dependency-violations", default=MAX_DEP_VIOLATIONS, help="Maximum number of dependency violations to allow") + parser.add_argument("--include-dir", action="append", + default=["src"], + help="A directory (under topdir) to search for source") parser.add_argument("topdir", default=".", nargs="?", help="Top-level directory for the tor source") args = parser.parse_args(argv[1:]) @@ -216,7 +219,7 @@ def main(argv): filt.addThreshold(problem.DependencyViolationItem("*", int(args.max_dependency_violations))) # 1) Get all the .c files we care about - files_list = util.get_tor_c_files(TOR_TOPDIR) + files_list = util.get_tor_c_files(TOR_TOPDIR, args.include_dir) # 2) Initialize problem vault and load an optional exceptions file so that # we don't warn about the past diff --git a/scripts/maint/practracker/test_practracker.sh b/scripts/maint/practracker/test_practracker.sh index c878ca5580..78e96ba471 100755 --- a/scripts/maint/practracker/test_practracker.sh +++ b/scripts/maint/practracker/test_practracker.sh @@ -25,6 +25,7 @@ DATA="${PRACTRACKER_DIR}/testdata" run_practracker() { "${PYTHON:-python}" "${PRACTRACKER_DIR}/practracker.py" \ + --include-dir "" \ --max-include-count=0 --max-file-size=0 --max-function-size=0 --terse \ "${DATA}/" "$@"; } diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py index 695668f561..854d369f74 100644 --- a/scripts/maint/practracker/util.py +++ b/scripts/maint/practracker/util.py @@ -3,22 +3,28 @@ import os # 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/rust/", - "src/ext/", ".git/"} + "src/ext/" } EXCLUDE_FILES = {"orconfig.h"} def _norm(p): return os.path.normcase(os.path.normpath(p)) -def get_tor_c_files(tor_topdir): +def get_tor_c_files(tor_topdir, include_dirs=None): """ Return a list with the .c and .h filenames we want to get metrics of. """ files_list = [] exclude_dirs = { _norm(os.path.join(tor_topdir, p)) for p in EXCLUDE_SOURCE_DIRS } + if include_dirs is None: + topdirs = [ tor_topdir ] + else: + topdirs = [ os.path.join(tor_topdir, inc) for inc in include_dirs ] - for root, directories, filenames in os.walk(tor_topdir): + # TO THE REVIEWER: I will fix this indentation shortly. -nm + for topdir in topdirs: + for root, directories, filenames in os.walk(topdir): # Remove all the directories that are excluded. directories[:] = [ d for d in directories if _norm(os.path.join(root,d)) not in exclude_dirs ] From 3a2964577e397319fe89cbba5393818beab665bd Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 2 Sep 2019 15:40:03 -0400 Subject: [PATCH 2/3] practracker: Fix indentation in util.py This commit only changes whitespace and removes a temporary comment. --- scripts/maint/practracker/util.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/scripts/maint/practracker/util.py b/scripts/maint/practracker/util.py index 854d369f74..4b42565528 100644 --- a/scripts/maint/practracker/util.py +++ b/scripts/maint/practracker/util.py @@ -22,23 +22,22 @@ def get_tor_c_files(tor_topdir, include_dirs=None): else: topdirs = [ os.path.join(tor_topdir, inc) for inc in include_dirs ] - # TO THE REVIEWER: I will fix this indentation shortly. -nm for topdir in topdirs: - for root, directories, filenames in os.walk(topdir): - # Remove all the directories that are excluded. - directories[:] = [ d for d in directories - if _norm(os.path.join(root,d)) not in exclude_dirs ] - directories.sort() - filenames.sort() - for filename in filenames: - # We only care about .c and .h files - if not (filename.endswith(".c") or filename.endswith(".h")): - continue - if filename in EXCLUDE_FILES: - continue + for root, directories, filenames in os.walk(topdir): + # Remove all the directories that are excluded. + directories[:] = [ d for d in directories + if _norm(os.path.join(root,d)) not in exclude_dirs ] + directories.sort() + filenames.sort() + for filename in filenames: + # We only care about .c and .h files + if not (filename.endswith(".c") or filename.endswith(".h")): + continue + if filename in EXCLUDE_FILES: + continue - full_path = os.path.join(root,filename) + full_path = os.path.join(root,filename) - files_list.append(full_path) + files_list.append(full_path) return files_list From 03040903e677172ce46319b83f9d00bafb33fad7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 2 Sep 2019 15:33:59 -0400 Subject: [PATCH 3/3] practracker: handle empty files. Previously practracker would fail on a file with no lines. --- scripts/maint/practracker/metrics.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/maint/practracker/metrics.py b/scripts/maint/practracker/metrics.py index 9f69b2ac1f..4c62bc2425 100644 --- a/scripts/maint/practracker/metrics.py +++ b/scripts/maint/practracker/metrics.py @@ -8,6 +8,7 @@ import re def get_file_len(f): """Get file length of file""" + i = -1 for i, l in enumerate(f): pass return i + 1