From 02bdf1b3820f6a80d3aa588a3aad4f0acea12741 Mon Sep 17 00:00:00 2001 From: Jeremy Nimmer Date: Tue, 31 May 2022 11:28:29 -0700 Subject: [PATCH] [workspace] Enhance check_lists_consistency docs and failure message (#17293) --- tools/workspace/check_lists_consistency.bzl | 40 ++++++++++--------- .../ignition_math/package.BUILD.bazel | 4 +- .../ignition_utils/package.BUILD.bazel | 4 +- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/tools/workspace/check_lists_consistency.bzl b/tools/workspace/check_lists_consistency.bzl index b945a179db21..e765f35d566a 100644 --- a/tools/workspace/check_lists_consistency.bzl +++ b/tools/workspace/check_lists_consistency.bzl @@ -1,25 +1,29 @@ # -*- python -*- def check_lists_consistency( - glob, - file_list): - """Checks consistency between lists of files and glob expression. + *, + files, + glob_include, + glob_exclude = None): + """Checks that a hard-coded list of files fully covers a glob expression. - If lists of files are hard-coded (e.g. public and private headers), one may - want to verify that all the files that should be listed are indeed listed. - This is especially important when the version of the target is updated, to - verify that the lists of files are also updated if necessary. + When a package.BUILD.bazel file hard-codes a list of files (e.g., a list of + headers of source files), we would like to fail-fast when upstream adds new + files so that we can refresh our list. This is especially important when + the version of the external is updated. Args: - glob (:obj:`str`): expression used to find all the files. The list of - files created thanks to this expression is compared to the given - lists of files. - file_list (:obj:`list` of :obj:`str`): List of file names. This is - typically a list of public headers concatenated with a list of private - headers. + files (:obj:`list` of :obj:`str`): List of expected file names that + will be matched by the glob expressions. + glob_include (:obj:`list` of :obj:`str`): List of glob patterns to + search for, per native.glob(include = ...). + glob_exclude (:obj:`list` of :obj:`str`): List of glob patterns to + exclude from the search, per native.glob(exclude = ...). """ - all_headers = native.glob(glob) - unknown_headers = [x for x in all_headers if x not in file_list] - if len(unknown_headers) != 0: - fail("Inconsistent file lists. Unknown file(s): " + - str(unknown_headers)) + all_files = native.glob(glob_include, exclude = (glob_exclude or [])) + uncovered_files = sorted([x for x in all_files if x not in files]) + if len(uncovered_files) != 0: + fail("The following files matched a glob of upstream sources, but " + + "were not covered by the package.BUILD.bazel file: {}".format( + uncovered_files, + )) diff --git a/tools/workspace/ignition_math/package.BUILD.bazel b/tools/workspace/ignition_math/package.BUILD.bazel index 8669a624b1df..76341dbc9a1f 100644 --- a/tools/workspace/ignition_math/package.BUILD.bazel +++ b/tools/workspace/ignition_math/package.BUILD.bazel @@ -145,8 +145,8 @@ drake_generate_include_header( ) check_lists_consistency( - file_list = private_headers + public_headers_no_gen, - glob = ["include/**/*.hh"], + files = private_headers + public_headers_no_gen, + glob_include = ["include/**/*.hh"], ) public_headers = public_headers_no_gen + [ diff --git a/tools/workspace/ignition_utils/package.BUILD.bazel b/tools/workspace/ignition_utils/package.BUILD.bazel index b3620f8679a6..2ce8f04787d8 100644 --- a/tools/workspace/ignition_utils/package.BUILD.bazel +++ b/tools/workspace/ignition_utils/package.BUILD.bazel @@ -76,8 +76,8 @@ drake_generate_include_header( ) check_lists_consistency( - file_list = private_headers + public_headers_no_gen, - glob = ["include/**/*.hh"], + files = private_headers + public_headers_no_gen, + glob_include = ["include/**/*.hh"], ) public_headers = public_headers_no_gen + [