Add filtering to ament_clang_tidy

This change adds two types of filtering to which files are tested with clang-tidy.  The motivation behind this is clang-tidy itself can take a really long time to execute and as a result, we'd like to limit the number of files it tests to those changed in a given PR for CI for moveit.

This adds three arguments to ament_clang_tidy:

The first one allows filtering the packages tested to a single repo.  This is useful in CI to limit the tests to the given repo you are executing on.  Also, if it finds a .clang-tidy config file in the root directory of the repo it will use that instead of the default clang-tidy config.
**known issue**: the implementation of this depends on on  `catkin_topological_order`.  Is there an ament/ros2 friendly tool for doing the same thing?

This one uses git to compare the current state of the repo with some previous version and test only the files changed.  This enables us to use clang-tidy in ci with large projects like moveit because it limits the scope of the clang-tidy test to the packages that were built.  This depends on the `filter-repo-path` feature above because the git commands used to figure out what files changed need to be run from the repo directory.

I feel the least strong about this feature.  I added this because I wanted it while debugging what clang-tidy command was actually being executed.  I think this could be a useful argument in general for python scripts that execute external programs.

I'm working to improve the ci used by moveit and moveit2.  I wrote this change to help with the migration away from travis we are going through with moveit (hence the catkin dependency).  Because this script is not easily available in ros1 (asfaik) I have this PR to submit the contents of it with this change into a tools directory in moveit for our own use (moveit/moveit#2470).  I'd appreciate advice if there is a better approach I should use to get ament scripts that are useful on ros1 projects.

Signed-off-by: Tyler Weaver <[email protected]>

Author: tylerjw

ament_clang_tidy/ament_clang_tidy/main.py

@@ -80,21 +80,53 @@ def main(argv=sys.argv[1:]):
     parser.add_argument(
         '--xunit-file',
         help='Generate a xunit compliant XML file')
+
+    parser.add_argument(
+        '--filter-repo-path',
+        help='Used to test only packages from one repo. '
+        'Accepts a path to repo containing ros packages.  If this path contains '
+        'a .clang-tidy config file, the tests will use that config.')
+    parser.add_argument(
+        '--filter-diff',
+        help='Used to only run tests on packages with changes since a specified git commit. '
+        'Accepts branch, tag, or git commit hash to generate diff.  Requires --filter-repo-path.')
+
+    parser.add_argument(
+        '--verbose',
+        action='store_true',
+        help='Show clang-tidy commands as they are run.')
     args = parser.parse_args(argv)
 
+    if args.filter_repo_path:
+        repo_clang_tidy = args.filter_repo_path + '/.clang-tidy'
+        if os.path.exists(repo_clang_tidy):
+            args.config_file = repo_clang_tidy
+
     if not os.path.exists(args.config_file):
         print("Could not find config file '%s'" % args.config_file,
               file=sys.stderr)
         return 1
 
+    print('Using config file %s' % args.config_file)
+
     if args.xunit_file:
         start_time = time.time()
 
     compilation_dbs = get_compilation_db_files(args.paths)
     if not compilation_dbs:
-        print('No compilation database files found', file=sys.stderr)
+        print('No compilation database files found in paths: %s' % (args.paths), file=sys.stderr)
         return 1
 
+    if args.filter_repo_path:
+        compilation_dbs = filter_packages_in_repo(compilation_dbs, args.filter_repo_path)
+
+    if args.filter_diff:
+        if not args.filter_repo_path:
+            print('Diff filter requires --filter-repo-path to be set.', file=sys.stderr)
+            return 1
+        compilation_dbs = filter_diff(compilation_dbs, args.filter_repo_path,
+                                      args.filter_diff)
+
     bin_names = [
         # 'clang-tidy',
         'clang-tidy-6.0',
@@ -157,6 +189,9 @@ def is_unittest_source(package, file_path):
 
             files.append(item['file'])
             full_cmd = cmd + [item['file']]
+
+            if args.verbose:
+                print(' '.join(full_cmd))
             try:
                 output += subprocess.check_output(full_cmd,
                                                   stderr=subprocess.DEVNULL).strip().decode()
@@ -171,7 +206,7 @@ def is_unittest_source(package, file_path):
     for compilation_db in compilation_dbs:
         package_dir = os.path.dirname(compilation_db)
         package_name = os.path.basename(package_dir)
-        print('found compilation database for package "%s"...' % package_name)
+        print('Invoking clang-tidy for package `%s`...' % package_name)
         (source_files, output) = invoke_clang_tidy(compilation_db)
         files += source_files
         outputs.append(output)
@@ -259,6 +294,77 @@ def get_compilation_db_files(paths):
     return [os.path.normpath(f) for f in files]
 
 
+def filter_packages_in_repo(compilation_dbs, repo_path):
+    bin_names = [
+        'catkin_topological_order',
+    ]
+    catkin_topological_order_bin = find_executable(bin_names)
+    if not catkin_topological_order_bin:
+        print('Could not find %s executable' %
+              ' / '.join(["'%s'" % n for n in bin_names]), file=sys.stderr)
+        return 1
+
+    cmd = [catkin_topological_order_bin, repo_path, '--only-names']
+    output = ''
+    try:
+        output = subprocess.check_output(cmd).strip().decode()
+    except subprocess.CalledProcessError as e:
+        print('The invocation of "%s" failed with error code %d: %s' %
+              (os.path.basename(catkin_topological_order_bin), e.returncode, e),
+              file=sys.stderr)
+
+    def test(path):
+        return os.path.basename(os.path.dirname(path)) in output.split()
+
+    filtered_dbs = list(filter(test, compilation_dbs))
+    return filtered_dbs
+
+
+def filter_diff(compilation_dbs, repo_path, diff_head):
+    bin_names = [
+        'git',
+    ]
+    git_bin = find_executable(bin_names)
+    if not git_bin:
+        print('Could not find %s executable' %
+              ' / '.join(["'%s'" % n for n in bin_names]), file=sys.stderr)
+        return 1
+
+    def modified_files_test(db_path):
+        package_dir = os.path.dirname(db_path)
+        package_name = os.path.basename(package_dir)
+        src_path = ''
+        with open(package_dir + '/CMakeCache.txt', 'r') as file:
+            for line in file:
+                if re.match('^CMAKE_HOME_DIRECTORY:INTERNAL=', line):
+                    src_path = line.split('=')[1].strip()
+                    break
+
+        if len(src_path) == 0:
+            print('Source path not found in CMakeCache.txt of package '
+                  '`%s`, skipping.' % package_name, file=sys.stderr)
+            return False
+
+        cmd = ['git', 'diff', '--name-only', '--diff-filter=MA', diff_head + '..HEAD', src_path]
+        output = ''
+        try:
+            output = subprocess.check_output(cmd, cwd=repo_path).strip().decode()
+        except subprocess.CalledProcessError as e:
+            print('The invocation of "%s" failed with error code %d: %s' %
+                  (os.path.basename(git_bin), e.returncode, ' '.join(cmd)),
+                  file=sys.stderr)
+            return False
+        modified_files = output.split()
+        if len(modified_files) == 0:
+            print('No files modified in package `%s`, skipping clang-tidy test.' % package_name)
+            return False
+        else:
+            return True
+
+    filtered_dbs = list(filter(modified_files_test, compilation_dbs))
+    return filtered_dbs
+
+
 def find_error_message(data):
     return data[data.rfind(':') + 2:]