Skip to content

Commit

Permalink
[tools] Add cmake_configure_files (plural) macro (#19952)
Browse files Browse the repository at this point in the history
This is important when configuring multiple files with the same
bag of definitions, so `strict = True` mode still makes sense.

Improve the diagnostic output to show *all* missing definitions, not
just the first one. This helps a lot during Local development.
  • Loading branch information
jwnimmer-tri authored Aug 7, 2023
1 parent 31bebf4 commit 92e830d
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 35 deletions.
61 changes: 44 additions & 17 deletions tools/workspace/cmake_configure_file.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ load(

# Defines the implementation actions to cmake_configure_file.
def _cmake_configure_file_impl(ctx):
arguments = [
"--input",
ctx.file.src.path,
"--output",
ctx.outputs.out.path,
]
if len(ctx.files.srcs) == 0:
fail("There must be at least one srcs")
if len(ctx.files.srcs) != len(ctx.outputs.outs):
fail("The number of srcs and outs must be congruent")
arguments = []
for src in ctx.files.srcs:
arguments += ["--input", src.path]
for out in ctx.outputs.outs:
arguments += ["--output", out.path]
for item in ctx.attr.defines:
arguments += ["-D" + item]
for item in ctx.attr.undefines:
Expand All @@ -22,8 +25,8 @@ def _cmake_configure_file_impl(ctx):
if ctx.attr.strict:
arguments += ["--strict"]
ctx.actions.run(
inputs = [ctx.file.src] + ctx.files.cmakelists,
outputs = [ctx.outputs.out],
inputs = ctx.files.srcs + ctx.files.cmakelists,
outputs = ctx.outputs.outs,
arguments = arguments,
env = ctx.attr.env,
executable = ctx.executable.cmake_configure_file_py,
Expand All @@ -33,11 +36,8 @@ def _cmake_configure_file_impl(ctx):
# Defines the rule to cmake_configure_file.
_cmake_configure_file_gen = rule(
attrs = {
"src": attr.label(
allow_single_file = True,
mandatory = True,
),
"out": attr.output(mandatory = True),
"srcs": attr.label_list(allow_files = True, mandatory = True),
"outs": attr.output_list(mandatory = True),
"defines": attr.string_list(),
"undefines": attr.string_list(),
"cmakelists": attr.label_list(allow_files = True),
Expand Down Expand Up @@ -93,8 +93,35 @@ def cmake_configure_file(
"""
_cmake_configure_file_gen(
name = name,
src = src,
out = out,
srcs = [src],
outs = [out],
defines = defines,
undefines = undefines,
cmakelists = cmakelists,
strict = strict,
env = hermetic_python_env(),
**kwargs
)

def cmake_configure_files(
name,
srcs = None,
outs = None,
defines = None,
undefines = None,
cmakelists = None,
strict = None,
**kwargs):
"""Like cmake_configure_file(), but with itemwise pairs of srcs => outs,
instead of just one pair of src => out.
When in strict mode, the defines / undefines must be used by *at least one*
of the srcs; only a definition that is unused by all srcs is an error.
"""
_cmake_configure_file_gen(
name = name,
srcs = srcs,
outs = outs,
defines = defines,
undefines = undefines,
cmakelists = cmakelists,
Expand Down Expand Up @@ -127,8 +154,8 @@ def autoconf_configure_file(
"""
_cmake_configure_file_gen(
name = name,
src = src,
out = out,
srcs = [src],
outs = [out],
defines = defines,
undefines = undefines,
strict = strict,
Expand Down
47 changes: 29 additions & 18 deletions tools/workspace/cmake_configure_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ def _transform_cmake(*, line, definitions, strict):
if var not in definitions:
defined = False
if strict:
raise KeyError(f"Missing define or undefine decision for {var}"
" when running in strict=True mode")
raise KeyError(var)
else:
defined = definitions[var] is not None
used_vars.add(var)
Expand Down Expand Up @@ -85,7 +84,7 @@ def _transform_cmake(*, line, definitions, strict):
assert len(var) > 0

if var not in definitions:
raise KeyError('Missing definition for ' + var)
raise KeyError(var)
used_vars.add(var)
value = definitions.get(var)
if value is None:
Expand Down Expand Up @@ -173,8 +172,10 @@ def _setup_definitions(args):

def main():
parser = argparse.ArgumentParser()
parser.add_argument('--input', metavar='FILE')
parser.add_argument('--output', metavar='FILE')
parser.add_argument(
'--input', metavar='FILE', action='append', default=[])
parser.add_argument(
'--output', metavar='FILE', action='append', default=[])
parser.add_argument(
'-D', metavar='NAME', dest='defines', action='append', default=[])
parser.add_argument(
Expand All @@ -187,28 +188,38 @@ def main():
parser.add_argument(
'--strict', action='store_true')
args = parser.parse_args()
if args.input is None or args.output is None:
parser.print_usage()
sys.exit(1)
if len(args.input) == 0:
parser.error("There must be at least one --input")
if len(args.input) != len(args.output):
parser.error("The number of --input and --output must be congruent")
definitions, cmakelist_keys = _setup_definitions(args)

transformer = _transform_autoconf if args.autoconf else _transform_cmake
total_used_vars = set()
with open(args.input, 'r') as input_file:
with open(args.output + '.tmp', 'w') as output_file:
for input_line in input_file.readlines():
output_line, used_vars = transformer(
line=input_line,
definitions=definitions,
strict=args.strict)
output_file.write(output_line)
total_used_vars |= used_vars
missing_vars = set()
for input_path, output_path in zip(args.input, args.output):
with open(input_path, 'r') as input_file:
with open(output_path + '.tmp', 'w') as output_file:
for input_line in input_file.readlines():
try:
output_line, used_vars = transformer(
line=input_line,
definitions=definitions,
strict=args.strict)
output_file.write(output_line)
total_used_vars |= used_vars
except KeyError as e:
missing_vars.add(e.args[0])
if missing_vars:
raise RuntimeError(f"The definitions of {sorted(missing_vars)} were"
" required, but missing.")
unused_vars = definitions.keys() - cmakelist_keys - total_used_vars
if unused_vars:
raise RuntimeError(f"The definitions of {sorted(unused_vars)} were"
" ignored and therefore seem like dead code;"
" remove them from defines= or undefines=.")
os.rename(args.output + '.tmp', args.output)
for output_path in args.output:
os.rename(output_path + '.tmp', output_path)


if __name__ == '__main__':
Expand Down

0 comments on commit 92e830d

Please sign in to comment.