From 1e29fdb00afa1c9dd262e3966b59bc21e103c341 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 26 Apr 2026 12:28:20 -0700 Subject: [PATCH 01/15] Add posttreatments option Post treatment is to allow a custom per project treatment to manipulate the resulting BUILD file --- README.md | 16 ++ contrib/posttreatments/README.md | 53 ++++++ .../posttreatments/add_crc32_arm_crc_copts.py | 145 +++++++++++++++++ .../examples/BUILD.bazel.addcrc | 26 +++ parser.py | 41 ++++- test/test_contrib_posttreatments.py | 151 ++++++++++++++++++ test/test_parser_utils.py | 50 +++++- 7 files changed, 478 insertions(+), 4 deletions(-) create mode 100644 contrib/posttreatments/README.md create mode 100644 contrib/posttreatments/add_crc32_arm_crc_copts.py create mode 100644 contrib/posttreatments/examples/BUILD.bazel.addcrc create mode 100644 test/test_contrib_posttreatments.py diff --git a/README.md b/README.md index c1f2ec2..3fb4a2e 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,22 @@ if you want to build things after the bazelfiication to checkt that everything s `ninja2bazel` supports remapping of paths/files and manually generated targets. +### Post-treatments + +Generated `BUILD.bazel` files can be rewritten immediately after generation with +one or more `--post-treatment` scripts. Each script receives the path to the +generated file and can update it in place. + +``` +python3 parser.py -p "." path/to/build.ninja path/to/src \ + --post-treatment contrib/posttreatments/add_crc32_arm_crc_copts.py +``` + +There is a complete AST-based example in `contrib/posttreatments/`. +That folder also contains a second example that injects a `genrule` to render +`pregenerated/flow/include/flow/ProtocolVersion.h` from +`flow/ProtocolVersion.h.cmake` and `flow/ProtocolVersions.cmake`. + ### Remapping paths/files Initially this was developped to deal with symlinks to other folders outside of the what was currently bazelified, for instance your are trying to bazelify your C++ code that is in `cpp` but you have already bazelified your protobuf that is in `proto` and you have a symlink from `cpp/proto` to `../proto`, using `--remap cpp/proto=proto` would allow to use targets that would be defined in the proto folder. diff --git a/contrib/posttreatments/README.md b/contrib/posttreatments/README.md new file mode 100644 index 0000000..7044a23 --- /dev/null +++ b/contrib/posttreatments/README.md @@ -0,0 +1,53 @@ +# Post-treatments + +This directory contains examples of post-treatments that run after `ninja2bazel` +generates a `BUILD.bazel` file. + +The example script in this folder parses the generated BUILD file as a Python AST, +finds the target named `crc32`, and rewrites: + +```python +copts = ["..."] +``` + +into: + +```python +copts = ["..."] + select({ + ":platform_linux_arm64": ["-march=armv8-a+crc"], + "//conditions:default": [], +}) +``` + +## Running the example manually + +```bash +python3 contrib/posttreatments/add_crc32_arm_crc_copts.py \ + contrib/posttreatments/examples/BUILD.bazel.BUILD.bazel.add_crc +``` + +## Running it directly from `parser.py` + +`parser.py` accepts repeated `--post-treatment` flags. Each script receives the +path to the generated `BUILD.bazel` file and is expected to rewrite it in place. + +```bash +python3 parser.py -p "." path/to/build.ninja path/to/src \ + --post-treatment contrib/posttreatments/add_crc32_arm_crc_copts.py +``` + +There is also a second example that injects a `genrule` producing the output label +`//:pregenerated/flow/include/flow/ProtocolVersion.h` from +`flow/ProtocolVersion.h.cmake` and `flow/ProtocolVersions.cmake`: + +```bash +python3 contrib/posttreatments/add_protocol_version_header_genrule.py \ + contrib/posttreatments/examples/protocol_version/BUILD.bazel +``` + +## Notes + +- This example uses `ast.parse()` and `ast.unparse()`, so it normalizes formatting. +- Comments are not preserved by `ast.unparse()`. +- If you want final formatting to be closer to normal Bazel style, run `buildifier` + after the post-treatment. diff --git a/contrib/posttreatments/add_crc32_arm_crc_copts.py b/contrib/posttreatments/add_crc32_arm_crc_copts.py new file mode 100644 index 0000000..674441f --- /dev/null +++ b/contrib/posttreatments/add_crc32_arm_crc_copts.py @@ -0,0 +1,145 @@ +#!/usr/bin/env python3 +import argparse +import ast +from pathlib import Path +from typing import Optional + +TARGET_NAME = "crc32" +PLATFORM_CONDITION = ":platform_linux_arm64" +PLATFORM_COPT = "-march=armv8-a+crc" +DEFAULT_CONDITION = "//conditions:default" + + +def _string_value(node: ast.AST) -> Optional[str]: + if isinstance(node, ast.Constant) and isinstance(node.value, str): + return node.value + return None + + +def _make_platform_select() -> ast.Call: + return ast.Call( + func=ast.Name(id="select", ctx=ast.Load()), + args=[ + ast.Dict( + keys=[ + ast.Constant(value=PLATFORM_CONDITION), + ast.Constant(value=DEFAULT_CONDITION), + ], + values=[ + ast.List( + elts=[ast.Constant(value=PLATFORM_COPT)], + ctx=ast.Load(), + ), + ast.List(elts=[], ctx=ast.Load()), + ], + ) + ], + keywords=[], + ) + + +def _has_platform_select(node: ast.AST) -> bool: + for subnode in ast.walk(node): + if not isinstance(subnode, ast.Call): + continue + if not isinstance(subnode.func, ast.Name) or subnode.func.id != "select": + continue + if len(subnode.args) != 1 or not isinstance(subnode.args[0], ast.Dict): + continue + mapping = subnode.args[0] + for key, value in zip(mapping.keys, mapping.values): + if _string_value(key) != PLATFORM_CONDITION: + continue + if not isinstance(value, ast.List): + continue + if any(_string_value(elt) == PLATFORM_COPT for elt in value.elts): + return True + return False + + +def _target_name(call: ast.Call) -> Optional[str]: + for keyword in call.keywords: + if keyword.arg == "name": + return _string_value(keyword.value) + return None + + +class Crc32CoptsTransformer(ast.NodeTransformer): + def __init__(self) -> None: + self.changed = False + + def visit_Expr(self, node: ast.Expr) -> ast.Expr: + node = self.generic_visit(node) + if not isinstance(node.value, ast.Call): + return node + + call = node.value + if _target_name(call) != TARGET_NAME: + return node + + for keyword in call.keywords: + if keyword.arg != "copts": + continue + if _has_platform_select(keyword.value): + return node + keyword.value = ast.BinOp( + left=keyword.value, + op=ast.Add(), + right=_make_platform_select(), + ) + self.changed = True + return node + + call.keywords.append( + ast.keyword( + arg="copts", + value=ast.BinOp( + left=ast.List(elts=[], ctx=ast.Load()), + op=ast.Add(), + right=_make_platform_select(), + ), + ) + ) + self.changed = True + return node + + +def rewrite_crc32_copts(source: str) -> str: + tree = ast.parse(source) + transformer = Crc32CoptsTransformer() + tree = transformer.visit(tree) + if not transformer.changed: + return source + ast.fix_missing_locations(tree) + return ast.unparse(tree) + "\n" + + +def rewrite_build_file(path: Path) -> bool: + source = path.read_text() + rewritten = rewrite_crc32_copts(source) + if rewritten == source: + return False + path.write_text(rewritten) + return True + + +def main() -> int: + parser = argparse.ArgumentParser( + description="Append an ARM CRC select() to the crc32 target copts" + ) + parser.add_argument( + "build_files", + nargs="+", + help="BUILD or BUILD.bazel files to rewrite", + ) + args = parser.parse_args() + + for build_file in args.build_files: + path = Path(build_file) + if rewrite_build_file(path): + print(f"Updated {path}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/contrib/posttreatments/examples/BUILD.bazel.addcrc b/contrib/posttreatments/examples/BUILD.bazel.addcrc new file mode 100644 index 0000000..4e068cb --- /dev/null +++ b/contrib/posttreatments/examples/BUILD.bazel.addcrc @@ -0,0 +1,26 @@ +config_setting( + name = "platform_linux_arm64", + constraint_values = [ + "@platforms//os:linux", + "@platforms//cpu:arm64", + ], +) + +cc_library( + name = "crc32", + srcs = ["crc32.cc"], + hdrs = ["crc32.h"], + copts = [ + "-Wall", + "-Wextra", + ], +) + +cc_library( + name = "adler32", + srcs = ["adler32.cc"], + hdrs = ["adler32.h"], + copts = [ + "-Wall", + ], +) diff --git a/parser.py b/parser.py index cad9784..6da735f 100755 --- a/parser.py +++ b/parser.py @@ -5,7 +5,7 @@ import subprocess import sys import time -from typing import Dict, List +from typing import Dict, List, Optional from cc_import_parse import parseCCImports from ninjabuild import genBazelBuildFiles, getBuildTargets @@ -32,6 +32,36 @@ def parse_manually_generated(manually_generated: List[str]) -> Dict[str, str]: BUILD_CUSTOMIZATION_DIRECTORY = "bazel/cpp" +def _build_post_treatment_command(script: str, build_file: str) -> List[str]: + if script.endswith(".py"): + return [sys.executable, script, build_file] + return [script, build_file] + + +def run_post_treatments( + build_file: str, post_treatments: Optional[List[str]] +) -> None: + if not post_treatments: + return + + for script in post_treatments: + if not os.path.exists(script): + logging.fatal(f"Post-treatment script {script} does not exist") + sys.exit(-1) + + cmd = _build_post_treatment_command(script, build_file) + result = subprocess.run(cmd, capture_output=True, text=True) + if result.returncode == 0: + continue + + logging.error(f"Post-treatment failed for {build_file} with script {script}") + if result.stdout: + logging.error(result.stdout.rstrip()) + if result.stderr: + logging.error(result.stderr.rstrip()) + sys.exit(result.returncode or -1) + + def main(argv=None): logging.basicConfig( level=logging.INFO, @@ -67,6 +97,11 @@ def main(argv=None): action="append", help="The name of top level target(s) to be generated, if not specified all targets will be generated", ) + parser.add_argument( + "--post-treatment", + action="append", + help="Executable run after each generated BUILD.bazel file; receives the file path to rewrite", + ) args = parser.parse_args(argv) @@ -161,8 +196,10 @@ def main(argv=None): logging.info( f"Wrote {rootdir}{name}{os.path.sep}BUILD.bazel len = {len(content)}" ) - with open(f"{rootdir}{name}{os.path.sep}BUILD.bazel", "w") as f: + build_file = f"{rootdir}{name}{os.path.sep}BUILD.bazel" + with open(build_file, "w") as f: f.write(content) + run_post_treatments(build_file, args.post_treatment) def getCompilerIncludesDir(compiler: str = "clang++") -> List[str]: diff --git a/test/test_contrib_posttreatments.py b/test/test_contrib_posttreatments.py new file mode 100644 index 0000000..e8c351b --- /dev/null +++ b/test/test_contrib_posttreatments.py @@ -0,0 +1,151 @@ +import os +import shutil +import subprocess +import sys +import tempfile +import unittest + + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +SCRIPT = os.path.join( + ROOT, + "contrib", + "posttreatments", + "add_crc32_arm_crc_copts.py", +) +SAMPLECRC = os.path.join( + ROOT, + "contrib", + "posttreatments", + "examples", + "BUILD.bazel.addcrc", +) +PROTOCOL_SCRIPT = os.path.join( + ROOT, + "contrib", + "posttreatments", + "add_protocol_version_header_genrule.py", +) +PROTOCOL_RENDERER = os.path.join( + ROOT, + "contrib", + "posttreatments", + "render_protocol_version_header.py", +) +PROTOCOL_SAMPLE_DIR = os.path.join( + ROOT, + "contrib", + "posttreatments", + "examples", + "protocol_version", +) +PROTOCOL_SAMPLE_BUILD = os.path.join(PROTOCOL_SAMPLE_DIR, "BUILD.bazel") + + +class TestContribPostTreatments(unittest.TestCase): + def test_rewrites_crc32_copts(self): + with tempfile.TemporaryDirectory() as tmpdir: + build_file = os.path.join(tmpdir, "BUILD.bazel") + shutil.copyfile(SAMPLECRC, build_file) + + subprocess.run([sys.executable, SCRIPT, build_file], check=True) + + with open(build_file, "r") as f: + content = f.read() + + self.assertIn("name='crc32'", content) + self.assertIn("name='adler32'", content) + self.assertIn( + "copts=['-Wall', '-Wextra'] + select({':platform_linux_arm64': " + "['-march=armv8-a+crc'], '//conditions:default': []})", + content, + ) + + def test_second_run_is_a_no_op(self): + with tempfile.TemporaryDirectory() as tmpdir: + build_file = os.path.join(tmpdir, "BUILD.bazel") + shutil.copyfile(SAMPLECRC, build_file) + + subprocess.run([sys.executable, SCRIPT, build_file], check=True) + with open(build_file, "r") as f: + first_pass = f.read() + + subprocess.run([sys.executable, SCRIPT, build_file], check=True) + with open(build_file, "r") as f: + second_pass = f.read() + + self.assertEqual(first_pass, second_pass) + self.assertEqual(second_pass.count("-march=armv8-a+crc"), 1) + + def test_adds_protocol_version_genrule(self): + with tempfile.TemporaryDirectory() as tmpdir: + build_file = os.path.join(tmpdir, "BUILD.bazel") + shutil.copyfile(PROTOCOL_SAMPLE_BUILD, build_file) + + subprocess.run([sys.executable, PROTOCOL_SCRIPT, build_file], check=True) + + with open(build_file, "r") as f: + content = f.read() + + self.assertIn("genrule(", content) + self.assertIn("name='generate_protocol_version_header'", content) + self.assertIn( + "outs=['pregenerated/flow/include/flow/ProtocolVersion.h']", + content, + ) + self.assertIn( + "tools=['//contrib/posttreatments:render_protocol_version_header']", + content, + ) + self.assertIn("$(location flow/ProtocolVersion.h.cmake)", content) + self.assertIn("$(location flow/ProtocolVersions.cmake)", content) + + def test_protocol_genrule_second_run_is_a_no_op(self): + with tempfile.TemporaryDirectory() as tmpdir: + build_file = os.path.join(tmpdir, "BUILD.bazel") + shutil.copyfile(PROTOCOL_SAMPLE_BUILD, build_file) + + subprocess.run([sys.executable, PROTOCOL_SCRIPT, build_file], check=True) + with open(build_file, "r") as f: + first_pass = f.read() + + subprocess.run([sys.executable, PROTOCOL_SCRIPT, build_file], check=True) + with open(build_file, "r") as f: + second_pass = f.read() + + self.assertEqual(first_pass, second_pass) + self.assertEqual(second_pass.count("generate_protocol_version_header"), 1) + + def test_renders_protocol_version_header(self): + with tempfile.TemporaryDirectory() as tmpdir: + source_dir = os.path.join(tmpdir, "flow") + shutil.copytree(os.path.join(PROTOCOL_SAMPLE_DIR, "flow"), source_dir) + output = os.path.join( + tmpdir, + "pregenerated", + "flow", + "include", + "flow", + "ProtocolVersion.h", + ) + + subprocess.run( + [ + sys.executable, + PROTOCOL_RENDERER, + os.path.join(source_dir, "ProtocolVersion.h.cmake"), + os.path.join(source_dir, "ProtocolVersions.cmake"), + output, + ], + check=True, + ) + + with open(output, "r") as f: + content = f.read() + + self.assertIn('#define DEFAULT_VERSION "0x0FDB00B073000000LL"', content) + self.assertIn('#define FUTURE_VERSION "0x0FDB00B074000000LL"', content) + self.assertIn( + '#define MIN_COMPATIBLE_VERSION "0x0FDB00B070000000LL"', + content, + ) diff --git a/test/test_parser_utils.py b/test/test_parser_utils.py index 760369c..c43dd52 100644 --- a/test/test_parser_utils.py +++ b/test/test_parser_utils.py @@ -1,5 +1,14 @@ +import sys import unittest -from parser import parse_manually_generated +from types import SimpleNamespace +from unittest import mock + +from parser import ( + _build_post_treatment_command, + parse_manually_generated, + run_post_treatments, +) + class TestParserUtils(unittest.TestCase): def test_parse_manual(self): @@ -10,5 +19,42 @@ def test_bad_format(self): with self.assertRaises(SystemExit): parse_manually_generated(['oops']) -if __name__ == '__main__': + def test_build_post_treatment_command_for_python_script(self): + self.assertEqual( + _build_post_treatment_command("script.py", "foo/BUILD.bazel"), + [sys.executable, "script.py", "foo/BUILD.bazel"], + ) + + def test_build_post_treatment_command_for_executable(self): + self.assertEqual( + _build_post_treatment_command("./script", "foo/BUILD.bazel"), + ["./script", "foo/BUILD.bazel"], + ) + + @mock.patch("parser.subprocess.run") + @mock.patch("parser.os.path.exists", return_value=True) + def test_run_post_treatments_runs_all_scripts(self, _exists, run): + run.return_value = SimpleNamespace(returncode=0, stdout="", stderr="") + + run_post_treatments("out/BUILD.bazel", ["first.py", "./second"]) + + self.assertEqual( + run.call_args_list[0].args[0], + [sys.executable, "first.py", "out/BUILD.bazel"], + ) + self.assertEqual( + run.call_args_list[1].args[0], + ["./second", "out/BUILD.bazel"], + ) + + @mock.patch("parser.subprocess.run") + @mock.patch("parser.os.path.exists", return_value=True) + def test_run_post_treatments_fails_on_non_zero_exit(self, _exists, run): + run.return_value = SimpleNamespace(returncode=4, stdout="", stderr="boom") + + with self.assertRaises(SystemExit): + run_post_treatments("out/BUILD.bazel", ["first.py"]) + + +if __name__ == "__main__": unittest.main() From c3777c84f3ad4f3fbd0581a50d5f055680024c8a Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 26 Apr 2026 14:07:03 -0700 Subject: [PATCH 02/15] Add another post treatment for generating ProtocolVersion from cmakke templates --- contrib/posttreatments/BUILD.bazel | 6 + .../add_protocol_version_header_genrule.py | 153 ++++++++++++++++++ .../examples/protocol_version/BUILD.bazel | 4 + .../flow/ProtocolVersion.h.cmake | 5 + .../flow/ProtocolVersions.cmake | 3 + .../render_protocol_version_header.py | 69 ++++++++ 6 files changed, 240 insertions(+) create mode 100644 contrib/posttreatments/BUILD.bazel create mode 100644 contrib/posttreatments/add_protocol_version_header_genrule.py create mode 100644 contrib/posttreatments/examples/protocol_version/BUILD.bazel create mode 100644 contrib/posttreatments/examples/protocol_version/flow/ProtocolVersion.h.cmake create mode 100644 contrib/posttreatments/examples/protocol_version/flow/ProtocolVersions.cmake create mode 100644 contrib/posttreatments/render_protocol_version_header.py diff --git a/contrib/posttreatments/BUILD.bazel b/contrib/posttreatments/BUILD.bazel new file mode 100644 index 0000000..be7718f --- /dev/null +++ b/contrib/posttreatments/BUILD.bazel @@ -0,0 +1,6 @@ +py_binary( + name = "render_protocol_version_header", + srcs = ["render_protocol_version_header.py"], + main = "render_protocol_version_header.py", + visibility = ["//visibility:public"], +) diff --git a/contrib/posttreatments/add_protocol_version_header_genrule.py b/contrib/posttreatments/add_protocol_version_header_genrule.py new file mode 100644 index 0000000..6cfa006 --- /dev/null +++ b/contrib/posttreatments/add_protocol_version_header_genrule.py @@ -0,0 +1,153 @@ +#!/usr/bin/env python3 +import argparse +import ast +from pathlib import Path +from typing import Optional + +RULE_NAME = "generate_protocol_version_header" +OUTPUT = "pregenerated/flow/include/flow/ProtocolVersion.h" +TEMPLATE = "flow/ProtocolVersion.h.cmake" +VALUES = "flow/ProtocolVersions.cmake" +TOOL = "//contrib/posttreatments:render_protocol_version_header" + + +def _string_value(node: ast.AST) -> Optional[str]: + if isinstance(node, ast.Constant) and isinstance(node.value, str): + return node.value + return None + + +def _call_name(node: ast.AST) -> Optional[str]: + if isinstance(node, ast.Call) and isinstance(node.func, ast.Name): + return node.func.id + return None + + +def _target_name(call: ast.Call) -> Optional[str]: + for keyword in call.keywords: + if keyword.arg == "name": + return _string_value(keyword.value) + return None + + +def _list_string_values(node: ast.AST) -> list[str]: + if not isinstance(node, ast.List): + return [] + values = [] + for element in node.elts: + value = _string_value(element) + if value is not None: + values.append(value) + return values + + +def _target_already_exists(call: ast.Call) -> bool: + if _call_name(call) != "genrule": + return False + if _target_name(call) == RULE_NAME: + return True + + for keyword in call.keywords: + if keyword.arg == "outs" and OUTPUT in _list_string_values(keyword.value): + return True + return False + + +def _make_genrule_expr() -> ast.Expr: + return ast.Expr( + value=ast.Call( + func=ast.Name(id="genrule", ctx=ast.Load()), + args=[], + keywords=[ + ast.keyword(arg="name", value=ast.Constant(value=RULE_NAME)), + ast.keyword( + arg="srcs", + value=ast.List( + elts=[ + ast.Constant(value=TEMPLATE), + ast.Constant(value=VALUES), + ], + ctx=ast.Load(), + ), + ), + ast.keyword( + arg="outs", + value=ast.List( + elts=[ast.Constant(value=OUTPUT)], + ctx=ast.Load(), + ), + ), + ast.keyword( + arg="tools", + value=ast.List( + elts=[ast.Constant(value=TOOL)], + ctx=ast.Load(), + ), + ), + ast.keyword( + arg="cmd", + value=ast.Constant( + value=" ".join( + [ + "$(location //contrib/posttreatments:render_protocol_version_header)", + "$(location flow/ProtocolVersion.h.cmake)", + "$(location flow/ProtocolVersions.cmake)", + "$@", + ] + ) + ), + ), + ast.keyword( + arg="visibility", + value=ast.List( + elts=[ast.Constant(value="//visibility:public")], + ctx=ast.Load(), + ), + ), + ], + ) + ) + + +def rewrite_build_file_contents(source: str) -> str: + tree = ast.parse(source) + + for node in tree.body: + if isinstance(node, ast.Expr) and isinstance(node.value, ast.Call): + if _target_already_exists(node.value): + return source + + tree.body.append(_make_genrule_expr()) + ast.fix_missing_locations(tree) + return ast.unparse(tree) + "\n" + + +def rewrite_build_file(path: Path) -> bool: + source = path.read_text() + rewritten = rewrite_build_file_contents(source) + if rewritten == source: + return False + path.write_text(rewritten) + return True + + +def main() -> int: + parser = argparse.ArgumentParser( + description="Add a genrule that renders pregenerated flow/ProtocolVersion.h" + ) + parser.add_argument( + "build_files", + nargs="+", + help="BUILD or BUILD.bazel files to rewrite", + ) + args = parser.parse_args() + + for build_file in args.build_files: + path = Path(build_file) + if rewrite_build_file(path): + print(f"Updated {path}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/contrib/posttreatments/examples/protocol_version/BUILD.bazel b/contrib/posttreatments/examples/protocol_version/BUILD.bazel new file mode 100644 index 0000000..2fcc873 --- /dev/null +++ b/contrib/posttreatments/examples/protocol_version/BUILD.bazel @@ -0,0 +1,4 @@ +cc_library( + name = "flow_support", + hdrs = ["flow/Foo.h"], +) diff --git a/contrib/posttreatments/examples/protocol_version/flow/ProtocolVersion.h.cmake b/contrib/posttreatments/examples/protocol_version/flow/ProtocolVersion.h.cmake new file mode 100644 index 0000000..ce97082 --- /dev/null +++ b/contrib/posttreatments/examples/protocol_version/flow/ProtocolVersion.h.cmake @@ -0,0 +1,5 @@ +#pragma once + +#define DEFAULT_VERSION "@DEFAULT_VERSION@" +#define FUTURE_VERSION "${FUTURE_VERSION}" +#define MIN_COMPATIBLE_VERSION "@MIN_COMPATIBLE_VERSION@" diff --git a/contrib/posttreatments/examples/protocol_version/flow/ProtocolVersions.cmake b/contrib/posttreatments/examples/protocol_version/flow/ProtocolVersions.cmake new file mode 100644 index 0000000..7a6147e --- /dev/null +++ b/contrib/posttreatments/examples/protocol_version/flow/ProtocolVersions.cmake @@ -0,0 +1,3 @@ +set(DEFAULT_VERSION "0x0FDB00B073000000LL") +set(FUTURE_VERSION 0x0FDB00B074000000LL) +set(MIN_COMPATIBLE_VERSION "0x0FDB00B070000000LL") diff --git a/contrib/posttreatments/render_protocol_version_header.py b/contrib/posttreatments/render_protocol_version_header.py new file mode 100644 index 0000000..1a3bcc3 --- /dev/null +++ b/contrib/posttreatments/render_protocol_version_header.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python3 +import argparse +import re +from pathlib import Path + +SET_RE = re.compile( + r"set\(\s*([A-Za-z_][A-Za-z0-9_]*)\s+(.*?)\s*\)", + re.DOTALL, +) +AT_PLACEHOLDER_RE = re.compile(r"@([A-Za-z_][A-Za-z0-9_]*)@") +DOLLAR_PLACEHOLDER_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") + + +def _normalize_value(raw_value: str) -> str: + value = raw_value.strip() + if "#" in value: + value = value.split("#", 1)[0].rstrip() + if value.startswith('"') and value.endswith('"'): + return value[1:-1] + return value + + +def parse_cmake_definitions(contents: str) -> dict[str, str]: + definitions: dict[str, str] = {} + for match in SET_RE.finditer(contents): + definitions[match.group(1)] = _normalize_value(match.group(2)) + return definitions + + +def render_template(template: str, definitions: dict[str, str]) -> str: + def replace_at(match: re.Match[str]) -> str: + key = match.group(1) + return definitions.get(key, match.group(0)) + + def replace_dollar(match: re.Match[str]) -> str: + key = match.group(1) + return definitions.get(key, match.group(0)) + + rendered = AT_PLACEHOLDER_RE.sub(replace_at, template) + return DOLLAR_PLACEHOLDER_RE.sub(replace_dollar, rendered) + + +def generate_header( + template_path: Path, + values_path: Path, + output_path: Path, +) -> None: + template = template_path.read_text() + definitions = parse_cmake_definitions(values_path.read_text()) + rendered = render_template(template, definitions) + output_path.parent.mkdir(parents=True, exist_ok=True) + output_path.write_text(rendered) + + +def main() -> int: + parser = argparse.ArgumentParser( + description="Render ProtocolVersion.h from CMake template inputs" + ) + parser.add_argument("template") + parser.add_argument("values") + parser.add_argument("output") + args = parser.parse_args() + + generate_header(Path(args.template), Path(args.values), Path(args.output)) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 4d525899c5f6125f9aacaffd97161a9ceee49e1a Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 26 Apr 2026 14:07:17 -0700 Subject: [PATCH 03/15] Generate load instruction for macros that are used related to cc_library, sh_binary ... --- bazel.py | 51 ++++++++++++++++++++++++++++++++++++++++- test/test_bazel.py | 56 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/bazel.py b/bazel.py index 126f9ce..1e2968f 100644 --- a/bazel.py +++ b/bazel.py @@ -27,6 +27,16 @@ CompilationFlags = Dict[str, Union[str, Set[str]]] BASIC_C_STD_RE = re.compile(r"^-std=(?:c|gnu)[0-9A-Za-z]+$") BASIC_CXX_STD_RE = re.compile(r"^-std=(?:c\+\+|gnu\+\+)[0-9A-Za-z]+$") +RULES_CC_BZL = "@rules_cc//cc:defs.bzl" +RULES_CC_SYMBOLS = ( + "cc_binary", + "cc_import", + "cc_library", + "cc_shared_library", + "cc_test", +) +RULES_PYTHON_LOAD = 'load("@rules_python//python:defs.bzl", "py_binary")' +RULES_SHELL_LOAD = 'load("@rules_shell//shell:sh_binary.bzl", "sh_binary")' def _normalize_flag(flag: str) -> str: @@ -50,6 +60,28 @@ def _split_language_opts( return conlyopts, cxxopts, copts +def _format_rules_cc_load(symbols: Iterable[str]) -> str: + ordered_symbols = [symbol for symbol in RULES_CC_SYMBOLS if symbol in symbols] + quoted_symbols = ", ".join([f'"{symbol}"' for symbol in ordered_symbols]) + return f'load("{RULES_CC_BZL}", {quoted_symbols})' + + +def _merge_rules_cc_loads(loads: Iterable[str]) -> List[str]: + symbols: Set[str] = set() + other_loads: List[str] = [] + prefix = f'load("{RULES_CC_BZL}", ' + for load in loads: + if load.startswith(prefix): + for symbol in RULES_CC_SYMBOLS: + if f'"{symbol}"' in load: + symbols.add(symbol) + else: + other_loads.append(load) + if symbols: + other_loads.append(_format_rules_cc_load(symbols)) + return other_loads + + def _getPrefix( d: Union["BaseBazelTarget", "BazelCCImport"], location: str, defaultPrefix: str ) -> str: @@ -229,7 +261,12 @@ def __repr__(self) -> str: return f"cc_import {self.name}" def getGlobalImport(self) -> str: - return "" + if self.alias is not None: + return "" + symbols = {"cc_import"} + if not self.skipWrapping: + symbols.add("cc_library") + return _format_rules_cc_load(symbols) def getAllHeaders(self, deps_only=False): # cc_import have headers but we don't include them in the upper target @@ -534,6 +571,7 @@ def genBazelBuildContent(self) -> Dict[str, str]: for k, v in topContent.items(): topStanza = list(filter(lambda x: x != "", v)) + topStanza = _merge_rules_cc_loads(topStanza) if len(topStanza) > 0: # Force empty line @@ -737,6 +775,11 @@ def __repr__(self) -> str: base += deps return base + def getGlobalImport(self) -> str: + if self.type in RULES_CC_SYMBOLS: + return _format_rules_cc_load({self.type}) + return "" + def asBazel( self, commonFlags: CompilationFlags, defaultPrefix: str = None ) -> BazelTargetStrings: @@ -1164,6 +1207,9 @@ def asBazel( def addSrc(self, target: BaseBazelTarget): self.srcs.add(target) + def getGlobalImport(self) -> str: + return RULES_PYTHON_LOAD + class ShBinaryBazelTarget(BaseBazelTarget): def __init__(self, name: str, location: str): @@ -1190,6 +1236,9 @@ def asBazel( def addSrc(self, target: BaseBazelTarget): self.srcs.add(target) + def getGlobalImport(self) -> str: + return RULES_SHELL_LOAD + bazelcache: Dict[str, Any] = {} diff --git a/test/test_bazel.py b/test/test_bazel.py index 78851cf..4a87a1e 100644 --- a/test/test_bazel.py +++ b/test/test_bazel.py @@ -10,6 +10,8 @@ BazelGRPCCCProtoLibrary, BazelGenRuleTarget, BazelProtoLibrary, + PyBinaryBazelTarget, + ShBinaryBazelTarget, BazelTarget, ExportedFile, _getPrefix, @@ -44,6 +46,26 @@ def test_as_bazel_binary(self): } self.assertEqual(res, expected) + def test_cc_targets_emit_explicit_rules_cc_load(self): + self.assertEqual( + BazelTarget("cc_library", "lib", "src").getGlobalImport(), + 'load("@rules_cc//cc:defs.bzl", "cc_library")', + ) + self.assertEqual( + BazelTarget("cc_binary", "app", "src").getGlobalImport(), + 'load("@rules_cc//cc:defs.bzl", "cc_binary")', + ) + + def test_py_and_sh_targets_emit_explicit_loads(self): + self.assertEqual( + PyBinaryBazelTarget("tool", "src").getGlobalImport(), + 'load("@rules_python//python:defs.bzl", "py_binary")', + ) + self.assertEqual( + ShBinaryBazelTarget("tool", "src").getGlobalImport(), + 'load("@rules_shell//shell:sh_binary.bzl", "sh_binary")', + ) + class TestBazelUtils(unittest.TestCase): def test_get_prefix(self): @@ -89,6 +111,15 @@ def test_cc_import_as_bazel(self): self.assertIn("foo", res) self.assertIn("raw_foo", res) self.assertTrue(any("cc_import(" in line for line in res["raw_foo"])) + self.assertEqual( + imp.getGlobalImport(), + 'load("@rules_cc//cc:defs.bzl", "cc_import", "cc_library")', + ) + imp.setSkipWrapping(True) + self.assertEqual( + imp.getGlobalImport(), + 'load("@rules_cc//cc:defs.bzl", "cc_import")', + ) class TestBazelGen(unittest.TestCase): @@ -167,6 +198,12 @@ def test_gen_bazel_build_content_includes_various_targets(self) -> None: self.assertIn("cc_library(", src_content) self.assertIn("foo_proto_cc_grpc", src_content) self.assertIn("generated.h", src_content) + self.assertIn( + 'load("@rules_cc//cc:defs.bzl", "cc_library")', + src_content, + ) + self.assertNotIn('"cc_binary"', src_content) + self.assertNotIn('"cc_shared_library"', src_content) self.assertIn( 'load("@rules_proto//proto:defs.bzl", "proto_library")', src_content ) @@ -177,3 +214,22 @@ def test_gen_bazel_build_content_includes_various_targets(self) -> None: self.assertNotIn( 'load("//src:helpers.bzl", "add_bazel_out_prefix")', src_content ) + + def test_gen_bazel_build_content_merges_needed_rules_cc_symbols(self) -> None: + build = BazelBuild("src/") + lib = BazelTarget("cc_library", "lib", "src") + app = BazelTarget("cc_binary", "app", "src") + build.bazelTargets.update({lib, app}) + + src_content = build.genBazelBuildContent()["src"] + + self.assertIn( + 'load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")', + src_content, + ) + self.assertNotIn( + 'load("@rules_cc//cc:defs.bzl", "cc_binary")\n' + 'load("@rules_cc//cc:defs.bzl", "cc_library")', + src_content, + ) + self.assertNotIn('"cc_shared_library"', src_content) From 5f35537b2dd6bf9cdba924de3e9ab33c2402b701 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 26 Apr 2026 20:08:43 -0700 Subject: [PATCH 04/15] Add support for generating pregenerated files This might sound counter-intuitive but we might want to generate pre-generated files ie the one that that are generated by cmake instead by the build. Why ? because it is a pain to have to copy the files all the time and keep them in sync as code change. --- README.md | 15 ++ build.py | 113 +++++++++-- configure_file.py | 182 ++++++++++++++++++ contrib/posttreatments/BUILD.bazel | 7 + .../posttreatments/render_configure_file.py | 69 +++++++ ninjabuild.py | 22 ++- parser.py | 17 +- test/test_configure_file.py | 124 ++++++++++++ 8 files changed, 531 insertions(+), 18 deletions(-) create mode 100644 configure_file.py create mode 100644 contrib/posttreatments/render_configure_file.py create mode 100644 test/test_configure_file.py diff --git a/README.md b/README.md index 3fb4a2e..675110f 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,21 @@ Initially this was developped to deal with symlinks to other folders outside of More recently this feature was extended to remap files as well, in this case you don't specify the full path where you want it remap but just the prefix to remap to; for instance if you have a file that is generated during the build you can remap it to a pre-exiting file that you have placed somewhere else, you would use `--remap flow/config.h=bazel/build` will remap the file flow/config.h to bazel/build assuming that there is a file called `flow/config.h` there. The tool will take care of setting the `include` value properly to make things work. +### CMake configure_file pregenerated files + +Pass `--configure_files_list path/to/configure_files.txt` to teach `ninja2bazel` +how CMake-created files under the work directory are generated. The file should +contain `configure_file(...)` lines copied from CMake, for example: + +``` +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ProtocolVersion.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/include/flow/ProtocolVersion.h) +``` + +When a pregenerated include matches one of those outputs, `ninja2bazel` emits a +`genrule` that renders it from the template. Template placeholders using +`@VAR@` or `${VAR}` are resolved by scanning CMake files for `set(VAR ...)`; the +conversion fails if any placeholder cannot be found. + ### Manually generated targets Sometime the build generates files but they are not generated by `ninja` a counter example for that are files generated by `cmake` because it won't work for them because usually the CMake build don't include them in the dependencies they are more often than not just included headers. In that case it's better to use the pregenerated support for that but for instance `RocksDB` build generates a file and add it as dependency to other targets but don't generate the command to get the generate the file itself. In this case you want to use `-m foo/bar.h=bazel/build/bar.h`. Beware that in order for this to work today you need to use a different prefix, this will need to be changed in the future to be more flexible. diff --git a/build.py b/build.py index 840ab81..1d2642d 100644 --- a/build.py +++ b/build.py @@ -13,6 +13,7 @@ BazelCCProtoLibrary, BazelExternalDep, BazelGenRuleTarget, + BazelGenRuleTargetOutput, BazelGRPCCCProtoLibrary, BazelProtoLibrary, BazelTarget, @@ -20,6 +21,7 @@ ShBinaryBazelTarget, getObject, ) +from configure_file import ConfigureFile, find_configure_file from helpers import resolvePath from visitor import VisitorContext @@ -51,6 +53,8 @@ class BazelBuildVisitorContext(VisitorContext): next_current: Optional[BaseBazelTarget] = None currentBuild: Optional["Build"] = None prefix: Optional["str"] = None + configure_files: Optional[Dict[str, ConfigureFile]] = None + configure_binary_dir: Optional[str] = None def __post_init__(self): if self.prefix.endswith(os.path.sep): @@ -66,6 +70,16 @@ def cleanup(self): pass +def _relpath_for_bazel(path: str, rootdir: str) -> str: + if path.startswith(rootdir): + return path[len(rootdir) :].lstrip(os.path.sep) + return path + + +def _configure_file_rule_name(output: str) -> str: + return "configure_" + re.sub(r"[^A-Za-z0-9_]", "_", output).strip("_") + + class BuildFileGroupingStrategy: _instance = None @@ -519,6 +533,58 @@ def _addAllCCimportDeps( if isinstance(d, BazelCCImport): cls._addAllCCimportDeps(d, ctx) + @classmethod + def _genConfigureFileRule( + cls, + ctx: BazelBuildVisitorContext, + configure_file: ConfigureFile, + output: str, + ) -> BaseBazelTarget: + location = ctx.prefix or "." + normalized_output = output.replace("/", "") + if not normalized_output.startswith("pregenerated/"): + normalized_output = f"pregenerated/{normalized_output}" + + genTarget = getObject( + BazelGenRuleTarget, + _configure_file_rule_name(normalized_output), + location, + ) + if len(genTarget.outs) == 0: + genTarget.addOut(normalized_output) + tool = BazelExternalDep("render_configure_file", "contrib/posttreatments") + genTarget.addTool(tool) + + source = _relpath_for_bazel(configure_file.source, ctx.rootdir) + genTarget.addSrc( + cls._genExportedFile( + filename=source, + locationCaller=genTarget.location, + ctx=ctx, + ) + ) + value_files = [ + _relpath_for_bazel(value_file, ctx.rootdir) + for value_file in configure_file.value_files + ] + for value_file in value_files: + genTarget.addSrc( + cls._genExportedFile( + filename=value_file, + locationCaller=genTarget.location, + ctx=ctx, + ) + ) + + args = [f"$(location {source})", "$@"] + args.extend([f"$(location {value_file})" for value_file in value_files]) + genTarget.cmd = ( + "$(location //contrib/posttreatments:render_configure_file) " + + " ".join(args) + ) + ctx.bazelbuild.bazelTargets.add(genTarget) + return next(iter(genTarget.outs)) + @classmethod def handleFileForBazelGen( cls, @@ -703,14 +769,25 @@ def handleFileForBazelGen( pregenerated = False name = f"{el.shortName}" - exported = cls._genExportedFile( - filename=name, - locationCaller=ctx.current.location, - ctx=ctx, - fileLocation=None, - ispregenerated=pregenerated, - ) - ctx.bazelbuild.bazelTargets.add(exported) + configure_file = None + if pregenerated: + configure_file = find_configure_file( + ctx.configure_files or {}, + name, + ctx.configure_binary_dir or "", + ) + if configure_file is not None: + exported = cls._genConfigureFileRule(ctx, configure_file, name) + else: + exported = cls._genExportedFile( + filename=name, + locationCaller=ctx.current.location, + ctx=ctx, + fileLocation=None, + ispregenerated=pregenerated, + ) + if not isinstance(exported, BazelGenRuleTargetOutput): + ctx.bazelbuild.bazelTargets.add(exported) ctx.current.addSrc(exported) if el.includes is None: @@ -779,13 +856,21 @@ def _handleIncludeBazelTarget( # logging.info(f"Adding header {i} using include {includeDir} from {el.name} {generated} to {ctx.current.name}") if includeDir is not None: if pregenerated: - ef = cls._genExportedFile( - filename=i, - locationCaller=ctx.current.location, - ctx=ctx, - fileLocation=None, - ispregenerated=True, + configure_file = find_configure_file( + ctx.configure_files or {}, + i, + ctx.configure_binary_dir or "", ) + if configure_file is not None: + ef = cls._genConfigureFileRule(ctx, configure_file, i) + else: + ef = cls._genExportedFile( + filename=i, + locationCaller=ctx.current.location, + ctx=ctx, + fileLocation=None, + ispregenerated=True, + ) else: ef = cls._genExportedFile( filename=i, locationCaller=ctx.current.location, ctx=ctx diff --git a/configure_file.py b/configure_file.py new file mode 100644 index 0000000..7db919d --- /dev/null +++ b/configure_file.py @@ -0,0 +1,182 @@ +import logging +import os +import re +import shlex +from dataclasses import dataclass +from typing import Dict, Iterable, List, Optional, Set + + +PLACEHOLDER_RE = re.compile(r"@([A-Za-z_][A-Za-z0-9_]*)@|\$\{([A-Za-z_][A-Za-z0-9_]*)\}") +SET_RE_TEMPLATE = r"set\s*\(\s*{name}(?:\s|\))" + + +@dataclass(frozen=True) +class ConfigureFile: + source: str + output: str + value_files: tuple[str, ...] + + +def _normalize_path(path: str) -> str: + return os.path.normpath(path).replace(os.path.sep, "/") + + +def _resolve_cmake_path(path: str, source_dir: str, binary_dir: str) -> str: + replacements = { + "${CMAKE_CURRENT_SOURCE_DIR}": source_dir, + "${CMAKE_SOURCE_DIR}": source_dir, + "${PROJECT_SOURCE_DIR}": source_dir, + "${CMAKE_CURRENT_BINARY_DIR}": binary_dir, + "${CMAKE_BINARY_DIR}": binary_dir, + "${PROJECT_BINARY_DIR}": binary_dir, + } + for key, value in replacements.items(): + path = path.replace(key, value) + if not os.path.isabs(path): + path = os.path.join(source_dir, path) + return os.path.normpath(path) + + +def _path_tail(path: str) -> str: + match = re.search(r"\$\{[A-Za-z_][A-Za-z0-9_]*\}/(.+)", path) + if match: + return match.group(1) + return path + + +def _resolve_existing_source(path: str, source_dir: str, resolved: str) -> str: + if os.path.exists(resolved): + return resolved + + tail = _path_tail(path) + matches = [] + for current, dirs, files in os.walk(source_dir): + dirs[:] = [d for d in dirs if d != ".git"] + for filename in files: + candidate = os.path.join(current, filename) + if _normalize_path(candidate).endswith(_normalize_path(tail)): + matches.append(candidate) + + if len(matches) == 1: + return os.path.normpath(matches[0]) + if len(matches) > 1: + logging.fatal( + f"Could not resolve configure_file source {path}; " + f"found multiple matches: {', '.join(sorted(matches))}" + ) + raise SystemExit(-1) + return resolved + + +def _parse_configure_file_args(line: str) -> Optional[List[str]]: + match = re.search(r"configure_file\s*\((.*)\)", line) + if not match: + return None + lexer = shlex.shlex(match.group(1), posix=True) + lexer.whitespace_split = True + lexer.commenters = "#" + args = list(lexer) + if len(args) < 2: + logging.fatal(f"Invalid configure_file entry: {line.rstrip()}") + raise SystemExit(-1) + return args + + +def _find_placeholders(template_path: str) -> Set[str]: + try: + with open(template_path, "r") as f: + contents = f.read() + except OSError as exc: + logging.fatal(f"Cannot read configure_file template {template_path}: {exc}") + raise SystemExit(-1) + return {match.group(1) or match.group(2) for match in PLACEHOLDER_RE.finditer(contents)} + + +def _iter_candidate_files(rootdir: str) -> Iterable[str]: + ignored_dirs = {".git", "bazel-bin", "bazel-out", "bazel-testlogs"} + for current, dirs, files in os.walk(rootdir): + dirs[:] = [d for d in dirs if d not in ignored_dirs] + for filename in files: + if filename.endswith((".cmake", ".txt", ".in", ".h.cmake")) or filename == "CMakeLists.txt": + yield os.path.join(current, filename) + + +def _find_value_files(rootdir: str, placeholders: Set[str], template_path: str) -> tuple[str, ...]: + found: Dict[str, Set[str]] = {placeholder: set() for placeholder in placeholders} + for path in _iter_candidate_files(rootdir): + if os.path.normpath(path) == os.path.normpath(template_path): + continue + try: + with open(path, "r", errors="ignore") as f: + contents = f.read() + except OSError: + continue + for placeholder in placeholders: + if re.search(SET_RE_TEMPLATE.format(name=re.escape(placeholder)), contents): + found[placeholder].add(path) + + missing = sorted([placeholder for placeholder, paths in found.items() if not paths]) + if missing: + logging.fatal( + "Missing CMake definitions for configure_file placeholders " + f"{', '.join(missing)} in {template_path}" + ) + raise SystemExit(-1) + + files = set() + for paths in found.values(): + files.update(paths) + return tuple(sorted(files)) + + +def parse_configure_files_list( + filename: Optional[str], + source_dir: str, + binary_dir: str, +) -> Dict[str, ConfigureFile]: + if not filename: + return {} + if not os.path.exists(filename): + logging.fatal(f"Configure files list {filename} does not exist") + raise SystemExit(-1) + + ret: Dict[str, ConfigureFile] = {} + with open(filename, "r") as f: + lines = f.readlines() + for line in lines: + args = _parse_configure_file_args(line) + if args is None: + continue + source = _resolve_cmake_path(args[0], source_dir, binary_dir) + source = _resolve_existing_source(args[0], source_dir, source) + output = _resolve_cmake_path(args[1], source_dir, binary_dir) + placeholders = _find_placeholders(source) + value_files = _find_value_files(source_dir, placeholders, source) + entry = ConfigureFile(source=source, output=output, value_files=value_files) + ret[_normalize_path(output)] = entry + ret[_normalize_path(os.path.relpath(output, binary_dir))] = entry + return ret + + +def find_configure_file( + configure_files: Dict[str, ConfigureFile], + output: str, + binary_dir: str, +) -> Optional[ConfigureFile]: + if not configure_files: + return None + candidates = [ + output, + output.replace("/", ""), + output.replace("pregenerated/", "", 1), + ] + if not os.path.isabs(output): + candidates.append(os.path.join(binary_dir, output.replace("pregenerated/", "", 1))) + normalized = [_normalize_path(candidate) for candidate in candidates] + for candidate in normalized: + if candidate in configure_files: + return configure_files[candidate] + for key, entry in configure_files.items(): + if any(key.endswith(candidate) or candidate.endswith(key) for candidate in normalized): + return entry + return None diff --git a/contrib/posttreatments/BUILD.bazel b/contrib/posttreatments/BUILD.bazel index be7718f..bcf275e 100644 --- a/contrib/posttreatments/BUILD.bazel +++ b/contrib/posttreatments/BUILD.bazel @@ -4,3 +4,10 @@ py_binary( main = "render_protocol_version_header.py", visibility = ["//visibility:public"], ) + +py_binary( + name = "render_configure_file", + srcs = ["render_configure_file.py"], + main = "render_configure_file.py", + visibility = ["//visibility:public"], +) diff --git a/contrib/posttreatments/render_configure_file.py b/contrib/posttreatments/render_configure_file.py new file mode 100644 index 0000000..60a0ab3 --- /dev/null +++ b/contrib/posttreatments/render_configure_file.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python3 +import argparse +import re +from pathlib import Path + + +SET_RE = re.compile( + r"set\(\s*([A-Za-z_][A-Za-z0-9_]*)\s+(.*?)\s*\)", + re.DOTALL, +) +AT_PLACEHOLDER_RE = re.compile(r"@([A-Za-z_][A-Za-z0-9_]*)@") +DOLLAR_PLACEHOLDER_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") + + +def _normalize_value(raw_value: str) -> str: + value = raw_value.strip() + if "#" in value: + value = value.split("#", 1)[0].rstrip() + if value.startswith('"') and value.endswith('"'): + return value[1:-1] + return value + + +def parse_cmake_definitions(contents: str) -> dict[str, str]: + definitions: dict[str, str] = {} + for match in SET_RE.finditer(contents): + definitions[match.group(1)] = _normalize_value(match.group(2)) + return definitions + + +def render_template(template: str, definitions: dict[str, str]) -> str: + def replace_at(match: re.Match[str]) -> str: + key = match.group(1) + return definitions.get(key, match.group(0)) + + def replace_dollar(match: re.Match[str]) -> str: + key = match.group(1) + return definitions.get(key, match.group(0)) + + rendered = AT_PLACEHOLDER_RE.sub(replace_at, template) + return DOLLAR_PLACEHOLDER_RE.sub(replace_dollar, rendered) + + +def generate_file( + template_path: Path, + output_path: Path, + values_paths: list[Path], +) -> None: + definitions: dict[str, str] = {} + for values_path in values_paths: + definitions.update(parse_cmake_definitions(values_path.read_text())) + rendered = render_template(template_path.read_text(), definitions) + output_path.parent.mkdir(parents=True, exist_ok=True) + output_path.write_text(rendered) + + +def main() -> int: + parser = argparse.ArgumentParser(description="Render a CMake configure_file template") + parser.add_argument("template") + parser.add_argument("output") + parser.add_argument("values", nargs="*") + args = parser.parse_args() + + generate_file(Path(args.template), Path(args.output), [Path(v) for v in args.values]) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/ninjabuild.py b/ninjabuild.py index 41266c5..cd3ee46 100644 --- a/ninjabuild.py +++ b/ninjabuild.py @@ -12,6 +12,7 @@ from bazel import BazelBuild, BazelCCImport from build import Build, BuildTarget, Rule, TargetType, TopLevelGroupingStrategy from build_visitor import BazelBuildVisitorContext, BuildVisitor, PrintVisitorContext +from configure_file import ConfigureFile from cppfileparser import CPPIncludes, findCPPIncludes, parseIncludes from helpers import resolvePath from protoparser import findProtoIncludes @@ -1123,14 +1124,27 @@ def _printNiceDict(d: dict[str, Any]) -> str: def genBazel( - buildTarget: BuildTarget, bb: BazelBuild, rootdir: str, flagsToIgnore: List[str] + buildTarget: BuildTarget, + bb: BazelBuild, + rootdir: str, + flagsToIgnore: List[str], + configure_files: Optional[Dict[str, ConfigureFile]] = None, + configure_binary_dir: Optional[str] = None, ): if rootdir.endswith("/"): dir = rootdir else: dir = f"{rootdir}/" - ctx = BazelBuildVisitorContext(False, dir, bb, flagsToIgnore, prefix=bb.prefix) + ctx = BazelBuildVisitorContext( + False, + dir, + bb, + flagsToIgnore, + prefix=bb.prefix, + configure_files=configure_files, + configure_binary_dir=configure_binary_dir, + ) visitor = BuildVisitor.getVisitor() @@ -1196,6 +1210,8 @@ def genBazelBuildFiles( rootdir: str, prefix: str, buildCustomizationDirectory: str, + configure_files: Optional[Dict[str, ConfigureFile]] = None, + configure_binary_dir: Optional[str] = None, ) -> Dict[str, str]: bb = BazelBuild(prefix) if buildCustomizationDirectory.startswith("/"): @@ -1224,7 +1240,7 @@ def genBazelBuildFiles( for e in sorted(top_levels): e.markTopLevel() - genBazel(e, bb, rootdir, flagsToIgnore) + genBazel(e, bb, rootdir, flagsToIgnore, configure_files, configure_binary_dir) bb.cleanup() diff --git a/parser.py b/parser.py index 6da735f..ff5ce03 100755 --- a/parser.py +++ b/parser.py @@ -8,6 +8,7 @@ from typing import Dict, List, Optional from cc_import_parse import parseCCImports +from configure_file import parse_configure_files_list from ninjabuild import genBazelBuildFiles, getBuildTargets @@ -102,6 +103,10 @@ def main(argv=None): action="append", help="Executable run after each generated BUILD.bazel file; receives the file path to rewrite", ) + parser.add_argument( + "--configure_files_list", + help="File containing CMake configure_file(...) lines used to generate pregenerated files", + ) args = parser.parse_args(argv) @@ -161,6 +166,11 @@ def main(argv=None): logging.info("Parsing ninja file and buildTargets") if not rootdir.endswith(os.path.sep): rootdir = f"{rootdir}{os.path.sep}" + configure_files = parse_configure_files_list( + args.configure_files_list, + rootdir, + cur_dir, + ) remap = {} if args.remap: for e in args.remap: @@ -186,7 +196,12 @@ def main(argv=None): logging.info(f"There are {len(top_levels_targets)} top level targets") output = genBazelBuildFiles( - top_levels_targets, rootdir, prefix, BUILD_CUSTOMIZATION_DIRECTORY + top_levels_targets, + rootdir, + prefix, + BUILD_CUSTOMIZATION_DIRECTORY, + configure_files, + cur_dir, ) end = time.time() print(f"Time to generate Bazel's BUILD files: {end - start}", file=sys.stdout) diff --git a/test/test_configure_file.py b/test/test_configure_file.py new file mode 100644 index 0000000..c1b7a7b --- /dev/null +++ b/test/test_configure_file.py @@ -0,0 +1,124 @@ +import os +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +from bazel import BazelBuild, BazelGenRuleTarget, BazelTarget +from build import BazelBuildVisitorContext, Build, BuildTarget, TopLevelGroupingStrategy +from configure_file import parse_configure_files_list + + +ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +RENDERER = os.path.join(ROOT, "contrib", "posttreatments", "render_configure_file.py") + + +class TestConfigureFile(unittest.TestCase): + def test_parse_configure_files_list_finds_value_files(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) / "src" + build = Path(td) / "build" + flow = root / "flow" + root.mkdir() + flow.mkdir() + build.mkdir() + (flow / "ProtocolVersion.h.cmake").write_text("#define V @VERSION@\n") + (flow / "ProtocolVersions.cmake").write_text('set(VERSION "1")\n') + list_file = Path(td) / "configure_files.txt" + list_file.write_text( + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ProtocolVersion.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/include/flow/ProtocolVersion.h)\n" + ) + + parsed = parse_configure_files_list(str(list_file), str(root), str(build)) + + self.assertIn("include/flow/ProtocolVersion.h", parsed) + entry = parsed["include/flow/ProtocolVersion.h"] + self.assertEqual(entry.source.replace(os.path.sep, "/").split("/")[-1], "ProtocolVersion.h.cmake") + self.assertIn("/flow/", entry.source.replace(os.path.sep, "/")) + self.assertEqual(len(entry.value_files), 1) + + def test_parse_configure_files_list_fails_for_missing_placeholder(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) / "src" + build = Path(td) / "build" + root.mkdir() + build.mkdir() + (root / "config.h.cmake").write_text("#define V @MISSING@\n") + list_file = Path(td) / "configure_files.txt" + list_file.write_text( + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/config.h)\n" + ) + + with self.assertRaises(SystemExit): + parse_configure_files_list(str(list_file), str(root), str(build)) + + def test_render_configure_file_uses_multiple_value_files(self) -> None: + with tempfile.TemporaryDirectory() as td: + template = Path(td) / "config.h.cmake" + values_a = Path(td) / "a.cmake" + values_b = Path(td) / "b.cmake" + output = Path(td) / "out" / "config.h" + template.write_text("#define A @A@\n#define B ${B}\n") + values_a.write_text("set(A one)\n") + values_b.write_text("set(B two)\n") + + subprocess.run( + [sys.executable, RENDERER, str(template), str(output), str(values_a), str(values_b)], + check=True, + ) + + self.assertEqual(output.read_text(), "#define A one\n#define B two\n") + + def test_pregenerated_include_gets_configure_file_genrule(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) / "src" + build_dir = Path(td) / "build" + pregenerated = build_dir / "pregenerated" / "include" / "flow" + root.mkdir() + build_dir.mkdir() + pregenerated.mkdir(parents=True) + (root / "ProtocolVersion.h.cmake").write_text("#define V @VERSION@\n") + (root / "ProtocolVersions.cmake").write_text("set(VERSION 1)\n") + list_file = Path(td) / "configure_files.txt" + list_file.write_text( + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ProtocolVersion.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/include/flow/ProtocolVersion.h)\n" + ) + configure_files = parse_configure_files_list(str(list_file), str(root), str(build_dir)) + TopLevelGroupingStrategy("") + bb = BazelBuild("") + current = BazelTarget("cc_library", "flow", ".") + ctx = BazelBuildVisitorContext( + False, + f"{root}{os.path.sep}", + bb, + [], + current=current, + prefix=".", + configure_files=configure_files, + configure_binary_dir=str(build_dir), + ) + el = BuildTarget("flow.cc", ("flow.cc", ".")) + el.setIncludedFiles( + [ + ( + "include/flow/ProtocolVersion.h", + f"{build_dir}{os.path.sep}pregenerated{os.path.sep}include{os.path.sep}flow", + ) + ] + ) + + Build._handleIncludeBazelTarget(el, ctx, f"{build_dir}{os.path.sep}") + bb.bazelTargets.add(current) + content = bb.genBazelBuildContent()["."] + + self.assertIn("genrule(", content) + self.assertEqual(content.count("genrule("), 1) + self.assertIn('name = "configure_pregenerated_include_flow_ProtocolVersion_h"', content) + self.assertIn('":pregenerated/include/flow/ProtocolVersion.h"', content) + self.assertIn("//contrib/posttreatments:render_configure_file", content) + self.assertIn(":pregenerated/include/flow/ProtocolVersion.h", content) + self.assertTrue(any(isinstance(target, BazelGenRuleTarget) for target in bb.bazelTargets)) From abb0a39f500124afb9b39ba80049e336a5119146 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 26 Apr 2026 21:08:30 -0700 Subject: [PATCH 05/15] Allow to provide some placeholders on the cli too --- README.md | 6 ++ build.py | 7 +++ configure_file.py | 28 ++++++++- .../posttreatments/render_configure_file.py | 21 ++++++- parser.py | 8 ++- test/test_configure_file.py | 60 ++++++++++++++++++- 6 files changed, 124 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 675110f..c3783c2 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,12 @@ When a pregenerated include matches one of those outputs, `ninja2bazel` emits a `@VAR@` or `${VAR}` are resolved by scanning CMake files for `set(VAR ...)`; the conversion fails if any placeholder cannot be found. +You can also provide values directly on the command line: + +``` +--configure_var=var1=val1 --configure_var=var2=val2 +``` + ### Manually generated targets Sometime the build generates files but they are not generated by `ninja` a counter example for that are files generated by `cmake` because it won't work for them because usually the CMake build don't include them in the dependencies they are more often than not just included headers. In that case it's better to use the pregenerated support for that but for instance `RocksDB` build generates a file and add it as dependency to other targets but don't generate the command to get the generate the file itself. In this case you want to use `-m foo/bar.h=bazel/build/bar.h`. Beware that in order for this to work today you need to use a different prefix, this will need to be changed in the future to be more flexible. diff --git a/build.py b/build.py index 1d2642d..73f3c97 100644 --- a/build.py +++ b/build.py @@ -1,6 +1,7 @@ import logging import os import re +import shlex from dataclasses import dataclass from enum import Enum from functools import total_ordering @@ -577,6 +578,12 @@ def _genConfigureFileRule( ) args = [f"$(location {source})", "$@"] + args.extend( + [ + f"--var {shlex.quote(f'{key}={value}')}" + for key, value in sorted(configure_file.variables.items()) + ] + ) args.extend([f"$(location {value_file})" for value_file in value_files]) genTarget.cmd = ( "$(location //contrib/posttreatments:render_configure_file) " diff --git a/configure_file.py b/configure_file.py index 7db919d..97d03bb 100644 --- a/configure_file.py +++ b/configure_file.py @@ -15,6 +15,23 @@ class ConfigureFile: source: str output: str value_files: tuple[str, ...] + variables: Dict[str, str] + + +def parse_configure_vars(configure_vars: Optional[List[str]]) -> Dict[str, str]: + ret: Dict[str, str] = {} + for configure_var in configure_vars or []: + if "=" not in configure_var: + logging.fatal( + f"Configure variable {configure_var} is not in the form key=value" + ) + raise SystemExit(-1) + key, value = configure_var.split("=", 1) + if not key: + logging.fatal(f"Configure variable {configure_var} has an empty key") + raise SystemExit(-1) + ret[key] = value + return ret def _normalize_path(path: str) -> str: @@ -133,6 +150,7 @@ def parse_configure_files_list( filename: Optional[str], source_dir: str, binary_dir: str, + configure_vars: Optional[Dict[str, str]] = None, ) -> Dict[str, ConfigureFile]: if not filename: return {} @@ -150,9 +168,15 @@ def parse_configure_files_list( source = _resolve_cmake_path(args[0], source_dir, binary_dir) source = _resolve_existing_source(args[0], source_dir, source) output = _resolve_cmake_path(args[1], source_dir, binary_dir) - placeholders = _find_placeholders(source) + variables = configure_vars or {} + placeholders = _find_placeholders(source) - set(variables.keys()) value_files = _find_value_files(source_dir, placeholders, source) - entry = ConfigureFile(source=source, output=output, value_files=value_files) + entry = ConfigureFile( + source=source, + output=output, + value_files=value_files, + variables=variables, + ) ret[_normalize_path(output)] = entry ret[_normalize_path(os.path.relpath(output, binary_dir))] = entry return ret diff --git a/contrib/posttreatments/render_configure_file.py b/contrib/posttreatments/render_configure_file.py index 60a0ab3..a76c942 100644 --- a/contrib/posttreatments/render_configure_file.py +++ b/contrib/posttreatments/render_configure_file.py @@ -45,10 +45,12 @@ def generate_file( template_path: Path, output_path: Path, values_paths: list[Path], + variables: dict[str, str], ) -> None: definitions: dict[str, str] = {} for values_path in values_paths: definitions.update(parse_cmake_definitions(values_path.read_text())) + definitions.update(variables) rendered = render_template(template_path.read_text(), definitions) output_path.parent.mkdir(parents=True, exist_ok=True) output_path.write_text(rendered) @@ -59,9 +61,26 @@ def main() -> int: parser.add_argument("template") parser.add_argument("output") parser.add_argument("values", nargs="*") + parser.add_argument( + "--var", + action="append", + default=[], + help="Template variable in the form key=value", + ) args = parser.parse_args() - generate_file(Path(args.template), Path(args.output), [Path(v) for v in args.values]) + variables = {} + for variable in args.var: + if "=" not in variable: + parser.error(f"--var must be in the form key=value: {variable}") + key, value = variable.split("=", 1) + variables[key] = value + generate_file( + Path(args.template), + Path(args.output), + [Path(v) for v in args.values], + variables, + ) return 0 diff --git a/parser.py b/parser.py index ff5ce03..38af9d0 100755 --- a/parser.py +++ b/parser.py @@ -8,7 +8,7 @@ from typing import Dict, List, Optional from cc_import_parse import parseCCImports -from configure_file import parse_configure_files_list +from configure_file import parse_configure_files_list, parse_configure_vars from ninjabuild import genBazelBuildFiles, getBuildTargets @@ -107,6 +107,11 @@ def main(argv=None): "--configure_files_list", help="File containing CMake configure_file(...) lines used to generate pregenerated files", ) + parser.add_argument( + "--configure_var", + action="append", + help="CMake configure_file variable in the form key=value", + ) args = parser.parse_args(argv) @@ -170,6 +175,7 @@ def main(argv=None): args.configure_files_list, rootdir, cur_dir, + parse_configure_vars(args.configure_var), ) remap = {} if args.remap: diff --git a/test/test_configure_file.py b/test/test_configure_file.py index c1b7a7b..6853484 100644 --- a/test/test_configure_file.py +++ b/test/test_configure_file.py @@ -7,7 +7,7 @@ from bazel import BazelBuild, BazelGenRuleTarget, BazelTarget from build import BazelBuildVisitorContext, Build, BuildTarget, TopLevelGroupingStrategy -from configure_file import parse_configure_files_list +from configure_file import parse_configure_files_list, parse_configure_vars ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) @@ -55,6 +55,30 @@ def test_parse_configure_files_list_fails_for_missing_placeholder(self) -> None: with self.assertRaises(SystemExit): parse_configure_files_list(str(list_file), str(root), str(build)) + def test_parse_configure_files_list_accepts_cli_configure_vars(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) / "src" + build = Path(td) / "build" + root.mkdir() + build.mkdir() + (root / "config.h.cmake").write_text("#define V @FROM_CLI@\n") + list_file = Path(td) / "configure_files.txt" + list_file.write_text( + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/config.h)\n" + ) + + parsed = parse_configure_files_list( + str(list_file), + str(root), + str(build), + parse_configure_vars(["FROM_CLI=yes"]), + ) + + entry = parsed["config.h"] + self.assertEqual(entry.value_files, ()) + self.assertEqual(entry.variables, {"FROM_CLI": "yes"}) + def test_render_configure_file_uses_multiple_value_files(self) -> None: with tempfile.TemporaryDirectory() as td: template = Path(td) / "config.h.cmake" @@ -72,6 +96,32 @@ def test_render_configure_file_uses_multiple_value_files(self) -> None: self.assertEqual(output.read_text(), "#define A one\n#define B two\n") + def test_render_configure_file_uses_cli_vars(self) -> None: + with tempfile.TemporaryDirectory() as td: + template = Path(td) / "config.h.cmake" + values = Path(td) / "values.cmake" + output = Path(td) / "out" / "config.h" + template.write_text("#define A @A@\n#define B @B@\n") + values.write_text("set(A from-file)\nset(B from-file)\n") + + subprocess.run( + [ + sys.executable, + RENDERER, + str(template), + str(output), + "--var", + "A=from-cli", + str(values), + ], + check=True, + ) + + self.assertEqual( + output.read_text(), + "#define A from-cli\n#define B from-file\n", + ) + def test_pregenerated_include_gets_configure_file_genrule(self) -> None: with tempfile.TemporaryDirectory() as td: root = Path(td) / "src" @@ -87,7 +137,12 @@ def test_pregenerated_include_gets_configure_file_genrule(self) -> None: "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ProtocolVersion.h.cmake " "${CMAKE_CURRENT_BINARY_DIR}/include/flow/ProtocolVersion.h)\n" ) - configure_files = parse_configure_files_list(str(list_file), str(root), str(build_dir)) + configure_files = parse_configure_files_list( + str(list_file), + str(root), + str(build_dir), + parse_configure_vars(["CLI_VALUE=abc"]), + ) TopLevelGroupingStrategy("") bb = BazelBuild("") current = BazelTarget("cc_library", "flow", ".") @@ -120,5 +175,6 @@ def test_pregenerated_include_gets_configure_file_genrule(self) -> None: self.assertIn('name = "configure_pregenerated_include_flow_ProtocolVersion_h"', content) self.assertIn('":pregenerated/include/flow/ProtocolVersion.h"', content) self.assertIn("//contrib/posttreatments:render_configure_file", content) + self.assertIn("--var CLI_VALUE=abc", content) self.assertIn(":pregenerated/include/flow/ProtocolVersion.h", content) self.assertTrue(any(isinstance(target, BazelGenRuleTarget) for target in bb.bazelTargets)) From d91d6634843a6ed94ec4f5a7a5b165e7d8c4662e Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Thu, 1 Jan 2026 23:28:41 -0800 Subject: [PATCH 06/15] Add tool to post process bazel file from ninja2bazel to make it more useful --- postprocess | 291 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 291 insertions(+) create mode 100755 postprocess diff --git a/postprocess b/postprocess new file mode 100755 index 0000000..b3b3248 --- /dev/null +++ b/postprocess @@ -0,0 +1,291 @@ +#!/usr/bin/env python3 +from __future__ import annotations + +import argparse +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Callable, Dict, List, Optional + + +@dataclass +class RuleCall: + rule: str + attrs: Dict[str, str|List[str]] + positional: List[Any] + deps: List[RuleCall] = field(default_factory=list) + usedBy: List[RuleCall] = field(default_factory=list) + hasHiddenSymbols: bool = False + location: str = "" + + +class BuildRecorder: + """ + Collects top-level rule calls like cc_library(name=..., deps=...). + """ + + def __init__(self) -> None: + self.calls: List[RuleCall] = [] + + def rule_fn(self, rule_name: str): + # This callable will be invoked from Starlark. + def _impl(*args, **kwargs): + dc = dict(kwargs) + # FIXME filter out the empty args and put + # list attrs to a separate from the str attr, + # put name separately too + self.calls.append( + RuleCall(rule=rule_name, attrs=dc, positional=list(args)) + ) + # BUILD rule calls return None in Bazel; mimic that. + return None + + return _impl + + +# --- Optional helpers: stub out common BUILD functions ------------------------ + + +def glob(include, exclude=None, **kwargs): + # Keep it symbolic unless you want to implement filesystem matching. + return { + "__fn__": "glob", + "include": include, + "exclude": exclude or [], + "kwargs": kwargs, + } + + +def select(mapping, **kwargs): + # Keep it symbolic; real resolution depends on Bazel configuration. + return {"__fn__": "select", "mapping": mapping, "kwargs": kwargs} + + +def parse_build_with_starlark_pyo3( + build_path: str, + *, + rule_names: Optional[List[str]] = None, +) -> List[RuleCall]: + """ + Parse+evaluate a BUILD file, recording calls to known rule functions. + """ + import starlark as sl # pip install starlark-pyo3 + + if rule_names is None: + # Add whatever rules you care about. + rule_names = [ + "cc_library", + "cc_binary", + "cc_test", + "py_library", + "py_binary", + "py_test", + "java_library", + "filegroup", + "genrule", + "alias", + ] + + source = Path(build_path).read_text(encoding="utf-8") + + # Dialect: standard Starlark; BUILD files usually need top-level statements. + dialect = sl.Dialect.standard() + dialect.enable_top_level_stmt = True # supported knob (write-only in docs) :contentReference[oaicite:1]{index=1} + + ast = sl.parse( + str(build_path), source, dialect=dialect + ) # :contentReference[oaicite:2]{index=2} + recorder = BuildRecorder() + module = sl.Module() + glb = sl.Globals.standard() + + # Provide rule functions that record what they were called with. + for r in rule_names: + module.add_callable( + r, recorder.rule_fn(r) + ) # :contentReference[oaicite:3]{index=3} + + # Provide common BUILD helpers (symbolic) + module.add_callable("glob", glob) + module.add_callable("select", select) + + # Evaluate the BUILD file; rule calls will be recorded. + # (If your BUILD uses load(), see note below.) + sl.eval(module, ast, glb) # :contentReference[oaicite:4]{index=4} + + return recorder.calls + + +def grafify_rules(raw_rules: List[RuleCall]) -> List[RuleCall]: + # For the moment assume that all the rules are in the same file or external + ret: List[RuleCall] = [] + missing: Dict[str, List[RuleCall]] = dict() + all_rules: Dict[str, RuleCall] = dict() + for c in raw_rules: + name = c.attrs.get("name") + assert name is not None + assert isinstance(name, str) + all_rules[name] = c + if name in missing: + for r in missing[name]: + r.deps.append(c) + c.usedBy.append(r) + del missing[name] + deps = c.attrs.get("deps") + remaining_deps = [] + if deps is not None: + for d in deps: + if d.startswith(':'): + dep_name = d[1:] + elif d.startswith('@'): + remaining_deps.append(d) + continue + else: + # FIXME + raise Exception("no deps from other file supported yet") + if dep_name in all_rules: + c.deps.append(all_rules[dep_name]) + all_rules[dep_name].usedBy.append(c) + else: + if missing[dep_name] is None: + missing[dep_name] = [] + missing[dep_name].append(c) + if len(remaining_deps) > 0: + c.attrs["deps"] = remaining_deps + elif deps is not None: + del c.attrs["deps"] + copts: List[str] = [] + copts.extend( c.attrs.get("copts") or [] ) + copts.extend( c.attrs.get("cxxopts") or [] ) + if "-fvisibility=hidden" in copts: + c.hasHiddenSymbols = True + + for r in all_rules.values(): + if len(r.usedBy) == 0: + ret.append(r) + + return ret + +def print_rule(e: RuleCall, depth: int = 0): + print(f"{' ' * depth}{e.rule} name={e.attrs.get('name')!r} attrs={list(e.attrs.keys())} hasHiddenSymbols={e.hasHiddenSymbols}, external_deps={e.attrs.get("deps")}") + for d in e.deps: + print_rule(d, depth + 2) + +def debug_graph(graph: List[RuleCall]): + for e in graph: + print_rule(e) + +def print_bazel(rule: RuleCall) -> None: + print(f"{rule.rule}(") + print(f' name = "{rule.attrs.get("name")}",') + for attr,val in rule.attrs.items(): + if attr != "name": + if isinstance(val, list): + if len(val) == 0: + continue + print(f' {attr} = [') + for v in val: + print(f' "{v}",') + + print(' ],') + else: + print(f' {attr} = "{val}",') + + print(")") + print(f"{rule.rule}(") + print(f' name = "{rule.attrs.get("name")}_hdrs",') + for attr,val in rule.attrs.items(): + if attr in ["visibility", "hdrs"]: + if isinstance(val, list): + if len(val) == 0: + continue + print(f' {attr} = [') + for v in val: + print(f' "{v}",') + + print(' ],') + else: + print(f' {attr} = "{val}",') + + print(")") + +def visit_rule(e: RuleCall, shouldCollapse: bool, callback: Callable[[RuleCall], None]): + # Here is the logic, + # When we find a rule that has hidden symbols we start collecting all the srcs, hdrs, copts, ... from the + # deps that also have hidden symbols + hasHidden = e.hasHiddenSymbols + # Let's copy external deps + new_deps: List[RuleCall] = [] + for d in e.deps: + visit_rule(d, (e.rule == "cc_library" or hasHidden) and d.hasHiddenSymbols, callback) + if (e.rule == "cc_library" or hasHidden) and d.hasHiddenSymbols: + for attr in d.attrs: + if attr in ["name", "visibility"]: + continue + current_attr = e.attrs.get(attr, []) + assert isinstance(current_attr, list) + current_attr.extend(d.attrs.get(attr, [])) + else: + new_deps.append(d) + e.deps = new_deps + if not shouldCollapse: + callback(e) + + +DESCRIPTION = """\ +Rewrite a generated BUILD.bazel file by collapsing selected C/C++ dependency +chains. + +The script evaluates a BUILD file with lightweight stub implementations of +common Bazel rules, records each top-level rule call, and builds an in-memory +dependency graph from same-file deps such as ":foo". External deps beginning +with "@" are kept as normal dependency strings. + +Rules whose copts or cxxopts contain -fvisibility=hidden are treated as hidden +symbol rules. During graph traversal, if a cc_library or another hidden-symbol +rule depends on a hidden-symbol rule, the dependency is collapsed into the +parent: list attributes such as srcs, hdrs, copts, cxxopts, and deps are copied +up, while name and visibility are left on the original rule. The collapsed dep +edge is removed. + +The rewritten BUILD content is printed to stdout. For each emitted rule, the +script also prints a companion rule named "_hdrs" containing only hdrs and +visibility. This is useful when generated hidden-symbol libraries need to be +flattened while still preserving a header-only target shape. +""" + + +def parse_args() -> argparse.Namespace: + parser = argparse.ArgumentParser( + description=DESCRIPTION, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + parser.add_argument( + "build_file", + nargs="?", + default="BUILD.bazel", + help="BUILD file to parse and rewrite. Defaults to BUILD.bazel.", + ) + parser.add_argument( + "--debug-graph", + action="store_true", + help="Print the dependency graph instead of rewritten BUILD rules.", + ) + return parser.parse_args() + + +def main() -> None: + args = parse_args() + calls = parse_build_with_starlark_pyo3(args.build_file) + graph = grafify_rules(calls) + + if args.debug_graph: + debug_graph(graph) + return + + # Collapse hidden-symbol deps and print the rewritten BUILD content. + for e in graph: + visit_rule(e, False, lambda e: print_bazel(e)) + + +if __name__ == "__main__": + main() From 5c569821709f77ff8f04f8475d94ad7a8bf89e60 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 26 Apr 2026 21:27:59 -0700 Subject: [PATCH 07/15] Deal with subfolders for configure_file Sometime the file is an subfolder so ${CMAKE_CURRENT_BINARY_DIR} points to the folder in the work dir + the subfolder but we didn't handle that before --- build.py | 13 +++ configure_file.py | 167 +++++++++++++++++++++++++++++++++++- parser.py | 69 +++++++++++++-- test/test_configure_file.py | 129 +++++++++++++++++++++++++++- 4 files changed, 366 insertions(+), 12 deletions(-) diff --git a/build.py b/build.py index 73f3c97..bfa4509 100644 --- a/build.py +++ b/build.py @@ -778,6 +778,12 @@ def handleFileForBazelGen( configure_file = None if pregenerated: + logging.info( + "Handling pregenerated file %s for target %s with workDir=%s", + name, + ctx.current.name, + workDir, + ) configure_file = find_configure_file( ctx.configure_files or {}, name, @@ -863,6 +869,13 @@ def _handleIncludeBazelTarget( # logging.info(f"Adding header {i} using include {includeDir} from {el.name} {generated} to {ctx.current.name}") if includeDir is not None: if pregenerated: + logging.info( + "Handling pregenerated include %s from include dir %s " + "for target %s", + i, + includeDir, + ctx.current.name, + ) configure_file = find_configure_file( ctx.configure_files or {}, i, diff --git a/configure_file.py b/configure_file.py index 97d03bb..f654b1c 100644 --- a/configure_file.py +++ b/configure_file.py @@ -119,6 +119,9 @@ def _iter_candidate_files(rootdir: str) -> Iterable[str]: def _find_value_files(rootdir: str, placeholders: Set[str], template_path: str) -> tuple[str, ...]: + if not placeholders: + return () + found: Dict[str, Set[str]] = {placeholder: set() for placeholder in placeholders} for path in _iter_candidate_files(rootdir): if os.path.normpath(path) == os.path.normpath(template_path): @@ -146,11 +149,85 @@ def _find_value_files(rootdir: str, placeholders: Set[str], template_path: str) return tuple(sorted(files)) +def _current_cmake_dirs(filename: str, source_dir: str, binary_dir: str) -> tuple[str, str]: + source_dir_abs = os.path.abspath(source_dir) + binary_dir_abs = os.path.abspath(binary_dir) + cmake_dir = os.path.dirname(os.path.abspath(filename)) + try: + if os.path.commonpath([source_dir_abs, cmake_dir]) != source_dir_abs: + return source_dir, binary_dir + except ValueError: + return source_dir, binary_dir + + rel_dir = os.path.relpath(cmake_dir, source_dir_abs) + if rel_dir == ".": + return source_dir, binary_dir + return cmake_dir, os.path.normpath(os.path.join(binary_dir_abs, rel_dir)) + + +def _configure_output_keys(output: str, binary_dir: str) -> Set[str]: + keys = {_normalize_path(output)} + rel_output = output + if os.path.isabs(output): + rel_output = os.path.relpath(output, binary_dir) + keys.add(_normalize_path(rel_output)) + + normalized_rel_output = _normalize_path(rel_output) + if normalized_rel_output.startswith("pregenerated/"): + keys.add(normalized_rel_output[len("pregenerated/") :]) + else: + keys.add(f"pregenerated/{normalized_rel_output}") + return keys + + +def _uses_cmake_current_dir(path: str) -> bool: + return ( + "${CMAKE_CURRENT_SOURCE_DIR}" in path + or "${CMAKE_CURRENT_BINARY_DIR}" in path + ) + + +def _infer_current_dirs_from_source( + source_arg: str, + source: str, + source_dir: str, + binary_dir: str, +) -> tuple[str, str]: + source_abs = _normalize_path(os.path.abspath(source)) + tail = _normalize_path(_path_tail(source_arg)) + if tail and source_abs.endswith(f"/{tail}"): + source_parent = os.path.normpath(source_abs[: -len(tail)].rstrip("/")) + else: + source_parent = os.path.dirname(os.path.abspath(source)) + source_dir_abs = os.path.abspath(source_dir) + binary_dir_abs = os.path.abspath(binary_dir) + try: + if os.path.commonpath([source_dir_abs, source_parent]) != source_dir_abs: + return source_dir, binary_dir + except ValueError: + return source_dir, binary_dir + + rel_dir = os.path.relpath(source_parent, source_dir_abs) + if rel_dir == ".": + return source_dir, binary_dir + return source_parent, os.path.normpath(os.path.join(binary_dir_abs, rel_dir)) + + +def _describe_configure_file(key: str, entry: ConfigureFile, binary_dir: str) -> str: + return ( + f"key={key}, output={_normalize_path(entry.output)}, " + f"rel_output={_normalize_path(os.path.relpath(entry.output, binary_dir))}, " + f"source={_normalize_path(entry.source)}, " + f"value_files={[ _normalize_path(path) for path in entry.value_files ]}" + ) + + def parse_configure_files_list( filename: Optional[str], source_dir: str, binary_dir: str, configure_vars: Optional[Dict[str, str]] = None, + needed_outputs: Optional[Set[str]] = None, ) -> Dict[str, ConfigureFile]: if not filename: return {} @@ -159,15 +236,45 @@ def parse_configure_files_list( raise SystemExit(-1) ret: Dict[str, ConfigureFile] = {} + normalized_needed_outputs = ( + {_normalize_path(output) for output in needed_outputs} + if needed_outputs is not None + else None + ) with open(filename, "r") as f: lines = f.readlines() for line in lines: args = _parse_configure_file_args(line) if args is None: continue - source = _resolve_cmake_path(args[0], source_dir, binary_dir) + current_source_dir, current_binary_dir = _current_cmake_dirs( + filename, + source_dir, + binary_dir, + ) + source = _resolve_cmake_path(args[0], current_source_dir, current_binary_dir) source = _resolve_existing_source(args[0], source_dir, source) - output = _resolve_cmake_path(args[1], source_dir, binary_dir) + if _uses_cmake_current_dir(args[0]) and _uses_cmake_current_dir(args[1]): + current_source_dir, current_binary_dir = _infer_current_dirs_from_source( + args[0], + source, + source_dir, + binary_dir, + ) + output = _resolve_cmake_path(args[1], current_source_dir, current_binary_dir) + output_keys = _configure_output_keys(output, binary_dir) + if normalized_needed_outputs is not None and not ( + output_keys & normalized_needed_outputs + ): + logging.info( + "Skipping configure_file entry for output %s because none of its " + "keys %s are needed; needed output count=%d", + _normalize_path(output), + sorted(output_keys), + len(normalized_needed_outputs), + ) + continue + variables = configure_vars or {} placeholders = _find_placeholders(source) - set(variables.keys()) value_files = _find_value_files(source_dir, placeholders, source) @@ -179,15 +286,53 @@ def parse_configure_files_list( ) ret[_normalize_path(output)] = entry ret[_normalize_path(os.path.relpath(output, binary_dir))] = entry + logging.info( + "Registered configure_file output %s from source %s with keys %s", + _normalize_path(output), + _normalize_path(source), + sorted(output_keys), + ) + if filename: + logging.info( + "Configured %d configure_file entries from %s", + len({entry.output for entry in ret.values()}), + filename, + ) return ret +def _log_configure_file_miss( + configure_files: Dict[str, ConfigureFile], + output: str, + binary_dir: str, + normalized: List[str], +) -> None: + logging.info( + "No configure_file matched requested output %s. Tried candidates: %s. " + "binary_dir=%s. configured entries=%d", + _normalize_path(output), + normalized, + _normalize_path(binary_dir), + len({entry.output for entry in configure_files.values()}), + ) + for key, entry in sorted(configure_files.items()): + logging.info( + "Configured configure_file did not match %s: %s", + _normalize_path(output), + _describe_configure_file(key, entry, binary_dir), + ) + + def find_configure_file( configure_files: Dict[str, ConfigureFile], output: str, binary_dir: str, ) -> Optional[ConfigureFile]: if not configure_files: + logging.info( + "No configure_file entries are configured while looking for %s", + _normalize_path(output), + ) return None candidates = [ output, @@ -197,10 +342,28 @@ def find_configure_file( if not os.path.isabs(output): candidates.append(os.path.join(binary_dir, output.replace("pregenerated/", "", 1))) normalized = [_normalize_path(candidate) for candidate in candidates] + logging.info( + "Looking for configure_file match for %s using candidates %s", + _normalize_path(output), + normalized, + ) for candidate in normalized: if candidate in configure_files: + logging.info( + "Matched configure_file for %s by exact candidate %s: %s", + _normalize_path(output), + candidate, + _describe_configure_file(candidate, configure_files[candidate], binary_dir), + ) return configure_files[candidate] for key, entry in configure_files.items(): if any(key.endswith(candidate) or candidate.endswith(key) for candidate in normalized): + logging.info( + "Matched configure_file for %s by suffix key %s: %s", + _normalize_path(output), + key, + _describe_configure_file(key, entry, binary_dir), + ) return entry + _log_configure_file_miss(configure_files, output, binary_dir, normalized) return None diff --git a/parser.py b/parser.py index 38af9d0..1dfb18f 100755 --- a/parser.py +++ b/parser.py @@ -5,7 +5,7 @@ import subprocess import sys import time -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Set from cc_import_parse import parseCCImports from configure_file import parse_configure_files_list, parse_configure_vars @@ -63,6 +63,56 @@ def run_post_treatments( sys.exit(result.returncode or -1) +def _add_needed_configure_output(outputs: Set[str], path: Optional[str], binary_dir: str) -> None: + if not path: + return + + normalized = os.path.normpath(path).replace(os.path.sep, "/") + outputs.add(normalized) + if os.path.isabs(path): + rel = os.path.relpath(path, binary_dir) + normalized = os.path.normpath(rel).replace(os.path.sep, "/") + outputs.add(normalized) + + if normalized.startswith("pregenerated/"): + outputs.add(normalized[len("pregenerated/") :]) + else: + outputs.add(f"pregenerated/{normalized}") + + +def collect_needed_configure_outputs(top_levels: List[object], binary_dir: str) -> Set[str]: + outputs: Set[str] = set() + seen: Set[str] = set() + + def visit(target: object) -> None: + name = getattr(target, "name", None) + if name in seen: + return + if name is not None: + seen.add(name) + _add_needed_configure_output(outputs, name, binary_dir) + _add_needed_configure_output(outputs, getattr(target, "shortName", None), binary_dir) + + for include, include_dir in getattr(target, "includes", set()): + _add_needed_configure_output(outputs, include, binary_dir) + if include_dir is not None: + _add_needed_configure_output(outputs, os.path.join(include_dir, include), binary_dir) + + build = getattr(target, "producedby", None) + if build is None: + return + for dep in build.getInputs(): + visit(dep) + for dep in build.depends: + if dep.depsAreVirtual(): + continue + visit(dep) + + for top_level in top_levels: + visit(top_level) + return outputs + + def main(argv=None): logging.basicConfig( level=logging.INFO, @@ -171,12 +221,6 @@ def main(argv=None): logging.info("Parsing ninja file and buildTargets") if not rootdir.endswith(os.path.sep): rootdir = f"{rootdir}{os.path.sep}" - configure_files = parse_configure_files_list( - args.configure_files_list, - rootdir, - cur_dir, - parse_configure_vars(args.configure_var), - ) remap = {} if args.remap: for e in args.remap: @@ -198,6 +242,17 @@ def main(argv=None): end = time.time() print(f"Time to getBuildTargets: {end - start}", file=sys.stdout) start = time.time() + needed_configure_outputs = collect_needed_configure_outputs(top_levels_targets, cur_dir) + configure_files = parse_configure_files_list( + args.configure_files_list, + rootdir, + cur_dir, + parse_configure_vars(args.configure_var), + needed_configure_outputs, + ) + end = time.time() + print(f"Time to parse configure_files: {end - start}", file=sys.stdout) + start = time.time() logging.info("Generating Bazel BUILD files from buildTargets") logging.info(f"There are {len(top_levels_targets)} top level targets") diff --git a/test/test_configure_file.py b/test/test_configure_file.py index 6853484..33d0055 100644 --- a/test/test_configure_file.py +++ b/test/test_configure_file.py @@ -7,7 +7,7 @@ from bazel import BazelBuild, BazelGenRuleTarget, BazelTarget from build import BazelBuildVisitorContext, Build, BuildTarget, TopLevelGroupingStrategy -from configure_file import parse_configure_files_list, parse_configure_vars +from configure_file import find_configure_file, parse_configure_files_list, parse_configure_vars ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) @@ -33,8 +33,8 @@ def test_parse_configure_files_list_finds_value_files(self) -> None: parsed = parse_configure_files_list(str(list_file), str(root), str(build)) - self.assertIn("include/flow/ProtocolVersion.h", parsed) - entry = parsed["include/flow/ProtocolVersion.h"] + self.assertIn("flow/include/flow/ProtocolVersion.h", parsed) + entry = parsed["flow/include/flow/ProtocolVersion.h"] self.assertEqual(entry.source.replace(os.path.sep, "/").split("/")[-1], "ProtocolVersion.h.cmake") self.assertIn("/flow/", entry.source.replace(os.path.sep, "/")) self.assertEqual(len(entry.value_files), 1) @@ -79,6 +79,129 @@ def test_parse_configure_files_list_accepts_cli_configure_vars(self) -> None: self.assertEqual(entry.value_files, ()) self.assertEqual(entry.variables, {"FROM_CLI": "yes"}) + def test_parse_configure_files_list_skips_unneeded_outputs(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) / "src" + build = Path(td) / "build" + root.mkdir() + build.mkdir() + (root / "needed.h.cmake").write_text("#define V @VERSION@\n") + (root / "unused.h.cmake").write_text("#define V @MISSING@\n") + (root / "values.cmake").write_text("set(VERSION 1)\n") + list_file = Path(td) / "configure_files.txt" + list_file.write_text( + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/needed.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/include/needed.h)\n" + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/unused.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/include/unused.h)\n" + ) + + parsed = parse_configure_files_list( + str(list_file), + str(root), + str(build), + needed_outputs={"include/needed.h"}, + ) + + self.assertIn("include/needed.h", parsed) + self.assertNotIn("include/unused.h", parsed) + + def test_parse_configure_files_list_uses_cmake_file_directory(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) / "src" + build = Path(td) / "build" + flow = root / "flow" + flow.mkdir(parents=True) + build.mkdir() + (flow / "ProtocolVersion.h.cmake").write_text("#define V @VERSION@\n") + (flow / "ProtocolVersions.cmake").write_text("set(VERSION 1)\n") + cmake_file = flow / "ProtocolVersion.cmake" + cmake_file.write_text( + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ProtocolVersion.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/include/flow/ProtocolVersion.h)\n" + ) + + parsed = parse_configure_files_list( + str(cmake_file), + str(root), + str(build), + needed_outputs={"pregenerated/flow/include/flow/ProtocolVersion.h"}, + ) + + self.assertIn("flow/include/flow/ProtocolVersion.h", parsed) + self.assertIn("/flow/", parsed["flow/include/flow/ProtocolVersion.h"].source) + self.assertIsNotNone( + find_configure_file( + parsed, + "pregenerated/flow/include/flow/ProtocolVersion.h", + str(build), + ) + ) + + def test_parse_flat_configure_files_list_infers_directory_from_template(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) / "src" + build = Path(td) / "build" + flow = root / "flow" + flow.mkdir(parents=True) + build.mkdir() + (flow / "ProtocolVersion.h.cmake").write_text("#define V @VERSION@\n") + (flow / "ProtocolVersions.cmake").write_text("set(VERSION 1)\n") + list_file = root / "configure_list" + list_file.write_text( + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/ProtocolVersion.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/include/flow/ProtocolVersion.h)\n" + ) + + parsed = parse_configure_files_list( + str(list_file), + str(root), + str(build), + needed_outputs={"pregenerated/flow/include/flow/ProtocolVersion.h"}, + ) + + self.assertIn("flow/include/flow/ProtocolVersion.h", parsed) + self.assertIsNotNone( + find_configure_file( + parsed, + "pregenerated/flow/include/flow/ProtocolVersion.h", + str(build), + ) + ) + + def test_parse_flat_configure_files_list_infers_current_dir_before_source_suffix(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) / "src" + build = Path(td) / "build" + bindings = root / "bindings" / "c" + foundationdb = bindings / "foundationdb" + foundationdb.mkdir(parents=True) + build.mkdir() + (foundationdb / "fdb_c_apiversion.h.cmake").write_text("#define V @VERSION@\n") + (bindings / "values.cmake").write_text("set(VERSION 1)\n") + list_file = root / "configure_list" + list_file.write_text( + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/foundationdb/fdb_c_apiversion.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/foundationdb/fdb_c_apiversion.g.h)\n" + ) + + parsed = parse_configure_files_list( + str(list_file), + str(root), + str(build), + needed_outputs={"pregenerated/bindings/c/foundationdb/fdb_c_apiversion.g.h"}, + ) + + self.assertIn("bindings/c/foundationdb/fdb_c_apiversion.g.h", parsed) + self.assertNotIn("bindings/c/foundationdb/foundationdb/fdb_c_apiversion.g.h", parsed) + self.assertIsNotNone( + find_configure_file( + parsed, + "pregenerated/bindings/c/foundationdb/fdb_c_apiversion.g.h", + str(build), + ) + ) + def test_render_configure_file_uses_multiple_value_files(self) -> None: with tempfile.TemporaryDirectory() as td: template = Path(td) / "config.h.cmake" From 6e873a2f056c6ac891d5ba9df3ab7f7e444348aa Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 26 Apr 2026 22:08:54 -0700 Subject: [PATCH 08/15] Generate some vars like CMAKE_SOURCE_DIR automatically --- parser.py | 27 ++++++++++++++++++++++++++- test/test_parser_utils.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/parser.py b/parser.py index 1dfb18f..0025f71 100755 --- a/parser.py +++ b/parser.py @@ -39,6 +39,26 @@ def _build_post_treatment_command(script: str, build_file: str) -> List[str]: return [script, build_file] +def _top_level_build_dir(rootdir: str, prefix: str) -> str: + if prefix in ("", "."): + return rootdir + return os.path.join(rootdir, prefix) + + +def _configure_vars_from_cli_paths( + rootdir: str, + binary_dir: str, + prefix: str, + configure_vars: Optional[List[str]], +) -> Dict[str, str]: + ret = { + "CMAKE_SOURCE_DIR": os.path.abspath(_top_level_build_dir(rootdir, prefix)), + "CMAKE_BINARY_DIR": os.path.abspath(binary_dir), + } + ret.update(parse_configure_vars(configure_vars)) + return ret + + def run_post_treatments( build_file: str, post_treatments: Optional[List[str]] ) -> None: @@ -247,7 +267,12 @@ def main(argv=None): args.configure_files_list, rootdir, cur_dir, - parse_configure_vars(args.configure_var), + _configure_vars_from_cli_paths( + rootdir, + cur_dir, + args.prefix, + args.configure_var, + ), needed_configure_outputs, ) end = time.time() diff --git a/test/test_parser_utils.py b/test/test_parser_utils.py index c43dd52..fe27f77 100644 --- a/test/test_parser_utils.py +++ b/test/test_parser_utils.py @@ -5,6 +5,7 @@ from parser import ( _build_post_treatment_command, + _configure_vars_from_cli_paths, parse_manually_generated, run_post_treatments, ) @@ -31,6 +32,39 @@ def test_build_post_treatment_command_for_executable(self): ["./script", "foo/BUILD.bazel"], ) + def test_configure_vars_from_cli_paths_sets_cmake_dirs(self): + values = _configure_vars_from_cli_paths( + "/tmp/project/src", + "/tmp/project/build", + ".", + None, + ) + + self.assertEqual(values["CMAKE_SOURCE_DIR"], "/tmp/project/src") + self.assertEqual(values["CMAKE_BINARY_DIR"], "/tmp/project/build") + + def test_configure_vars_from_cli_paths_uses_prefix_for_top_level_build(self): + values = _configure_vars_from_cli_paths( + "/tmp/project/src", + "/tmp/project/build", + "subdir", + None, + ) + + self.assertEqual(values["CMAKE_SOURCE_DIR"], "/tmp/project/src/subdir") + + def test_configure_vars_from_cli_paths_allows_cli_override(self): + values = _configure_vars_from_cli_paths( + "/tmp/project/src", + "/tmp/project/build", + ".", + ["CMAKE_SOURCE_DIR=/override/src", "CUSTOM=yes"], + ) + + self.assertEqual(values["CMAKE_SOURCE_DIR"], "/override/src") + self.assertEqual(values["CMAKE_BINARY_DIR"], "/tmp/project/build") + self.assertEqual(values["CUSTOM"], "yes") + @mock.patch("parser.subprocess.run") @mock.patch("parser.os.path.exists", return_value=True) def test_run_post_treatments_runs_all_scripts(self, _exists, run): From 0283b2a344c907a3671cf4d7e55e4eab81534804 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 26 Apr 2026 22:22:16 -0700 Subject: [PATCH 09/15] Move renderconfig to the target tree if needed --- build.py | 31 +++---- contrib/posttreatments/BUILD.bazel | 7 -- .../posttreatments/render_configure_file.py | 88 ------------------- parser.py | 28 ++++++ test/test_configure_file.py | 7 +- test/test_parser_utils.py | 11 +++ 6 files changed, 57 insertions(+), 115 deletions(-) delete mode 100644 contrib/posttreatments/render_configure_file.py diff --git a/build.py b/build.py index bfa4509..2860f1b 100644 --- a/build.py +++ b/build.py @@ -7,21 +7,11 @@ from functools import total_ordering from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Union -from bazel import ( - BaseBazelTarget, - BazelBuild, - BazelCCImport, - BazelCCProtoLibrary, - BazelExternalDep, - BazelGenRuleTarget, - BazelGenRuleTargetOutput, - BazelGRPCCCProtoLibrary, - BazelProtoLibrary, - BazelTarget, - ExportedFile, - ShBinaryBazelTarget, - getObject, -) +from bazel import (BaseBazelTarget, BazelBuild, BazelCCImport, + BazelCCProtoLibrary, BazelExternalDep, BazelGenRuleTarget, + BazelGenRuleTargetOutput, BazelGRPCCCProtoLibrary, + BazelProtoLibrary, BazelTarget, ExportedFile, PyBinaryBazelTarget, + ShBinaryBazelTarget, getObject) from configure_file import ConfigureFile, find_configure_file from helpers import resolvePath from visitor import VisitorContext @@ -30,6 +20,8 @@ TargetType = Enum( "TargetType", ["other", "unknown", "known", "external", "manually_generated"] ) +CONFIGURE_FILE_TOOL_PATH = "bazel/tools/render_configure_file.py" +CONFIGURE_FILE_TOOL_TARGET = "render_configure_file" def genShBinaryScript(rootdir: str, command: str) -> str: @@ -553,7 +545,10 @@ def _genConfigureFileRule( ) if len(genTarget.outs) == 0: genTarget.addOut(normalized_output) - tool = BazelExternalDep("render_configure_file", "contrib/posttreatments") + tool = getObject(PyBinaryBazelTarget, CONFIGURE_FILE_TOOL_TARGET, location) + tool.main = CONFIGURE_FILE_TOOL_PATH + tool.addSrc(ExportedFile(CONFIGURE_FILE_TOOL_PATH, location)) + ctx.bazelbuild.bazelTargets.add(tool) genTarget.addTool(tool) source = _relpath_for_bazel(configure_file.source, ctx.rootdir) @@ -578,15 +573,15 @@ def _genConfigureFileRule( ) args = [f"$(location {source})", "$@"] + args.extend([f"$(location {value_file})" for value_file in value_files]) args.extend( [ f"--var {shlex.quote(f'{key}={value}')}" for key, value in sorted(configure_file.variables.items()) ] ) - args.extend([f"$(location {value_file})" for value_file in value_files]) genTarget.cmd = ( - "$(location //contrib/posttreatments:render_configure_file) " + f"$(location :{CONFIGURE_FILE_TOOL_TARGET}) " + " ".join(args) ) ctx.bazelbuild.bazelTargets.add(genTarget) diff --git a/contrib/posttreatments/BUILD.bazel b/contrib/posttreatments/BUILD.bazel index bcf275e..be7718f 100644 --- a/contrib/posttreatments/BUILD.bazel +++ b/contrib/posttreatments/BUILD.bazel @@ -4,10 +4,3 @@ py_binary( main = "render_protocol_version_header.py", visibility = ["//visibility:public"], ) - -py_binary( - name = "render_configure_file", - srcs = ["render_configure_file.py"], - main = "render_configure_file.py", - visibility = ["//visibility:public"], -) diff --git a/contrib/posttreatments/render_configure_file.py b/contrib/posttreatments/render_configure_file.py deleted file mode 100644 index a76c942..0000000 --- a/contrib/posttreatments/render_configure_file.py +++ /dev/null @@ -1,88 +0,0 @@ -#!/usr/bin/env python3 -import argparse -import re -from pathlib import Path - - -SET_RE = re.compile( - r"set\(\s*([A-Za-z_][A-Za-z0-9_]*)\s+(.*?)\s*\)", - re.DOTALL, -) -AT_PLACEHOLDER_RE = re.compile(r"@([A-Za-z_][A-Za-z0-9_]*)@") -DOLLAR_PLACEHOLDER_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") - - -def _normalize_value(raw_value: str) -> str: - value = raw_value.strip() - if "#" in value: - value = value.split("#", 1)[0].rstrip() - if value.startswith('"') and value.endswith('"'): - return value[1:-1] - return value - - -def parse_cmake_definitions(contents: str) -> dict[str, str]: - definitions: dict[str, str] = {} - for match in SET_RE.finditer(contents): - definitions[match.group(1)] = _normalize_value(match.group(2)) - return definitions - - -def render_template(template: str, definitions: dict[str, str]) -> str: - def replace_at(match: re.Match[str]) -> str: - key = match.group(1) - return definitions.get(key, match.group(0)) - - def replace_dollar(match: re.Match[str]) -> str: - key = match.group(1) - return definitions.get(key, match.group(0)) - - rendered = AT_PLACEHOLDER_RE.sub(replace_at, template) - return DOLLAR_PLACEHOLDER_RE.sub(replace_dollar, rendered) - - -def generate_file( - template_path: Path, - output_path: Path, - values_paths: list[Path], - variables: dict[str, str], -) -> None: - definitions: dict[str, str] = {} - for values_path in values_paths: - definitions.update(parse_cmake_definitions(values_path.read_text())) - definitions.update(variables) - rendered = render_template(template_path.read_text(), definitions) - output_path.parent.mkdir(parents=True, exist_ok=True) - output_path.write_text(rendered) - - -def main() -> int: - parser = argparse.ArgumentParser(description="Render a CMake configure_file template") - parser.add_argument("template") - parser.add_argument("output") - parser.add_argument("values", nargs="*") - parser.add_argument( - "--var", - action="append", - default=[], - help="Template variable in the form key=value", - ) - args = parser.parse_args() - - variables = {} - for variable in args.var: - if "=" not in variable: - parser.error(f"--var must be in the form key=value: {variable}") - key, value = variable.split("=", 1) - variables[key] = value - generate_file( - Path(args.template), - Path(args.output), - [Path(v) for v in args.values], - variables, - ) - return 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/parser.py b/parser.py index 0025f71..9edbde3 100755 --- a/parser.py +++ b/parser.py @@ -2,11 +2,13 @@ import argparse import logging import os +import shutil import subprocess import sys import time from typing import Dict, List, Optional, Set +from build import CONFIGURE_FILE_TOOL_PATH from cc_import_parse import parseCCImports from configure_file import parse_configure_files_list, parse_configure_vars from ninjabuild import genBazelBuildFiles, getBuildTargets @@ -31,6 +33,10 @@ def parse_manually_generated(manually_generated: List[str]) -> Dict[str, str]: # FIXME: This should be a parameter # if relative it's relative to the rootdir BUILD_CUSTOMIZATION_DIRECTORY = "bazel/cpp" +TOOL_SOURCE_DIRECTORY = os.path.join( + os.path.dirname(os.path.abspath(__file__)), + "tools", +) def _build_post_treatment_command(script: str, build_file: str) -> List[str]: @@ -59,6 +65,26 @@ def _configure_vars_from_cli_paths( return ret +def _copy_if_different(source: str, destination: str) -> None: + source_abs = os.path.abspath(source) + destination_abs = os.path.abspath(destination) + if source_abs == destination_abs: + return + shutil.copy2(source_abs, destination_abs) + + +def install_configure_file_tool(rootdir: str, prefix: str) -> None: + tool_destination = os.path.join( + _top_level_build_dir(rootdir, prefix), + CONFIGURE_FILE_TOOL_PATH, + ) + os.makedirs(os.path.dirname(tool_destination), exist_ok=True) + _copy_if_different( + os.path.join(TOOL_SOURCE_DIRECTORY, "render_configure_file.py"), + tool_destination, + ) + + def run_post_treatments( build_file: str, post_treatments: Optional[List[str]] ) -> None: @@ -278,6 +304,8 @@ def main(argv=None): end = time.time() print(f"Time to parse configure_files: {end - start}", file=sys.stdout) start = time.time() + if configure_files: + install_configure_file_tool(rootdir, args.prefix) logging.info("Generating Bazel BUILD files from buildTargets") logging.info(f"There are {len(top_levels_targets)} top level targets") diff --git a/test/test_configure_file.py b/test/test_configure_file.py index 33d0055..d6485ca 100644 --- a/test/test_configure_file.py +++ b/test/test_configure_file.py @@ -11,7 +11,7 @@ ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) -RENDERER = os.path.join(ROOT, "contrib", "posttreatments", "render_configure_file.py") +RENDERER = os.path.join(ROOT, "tools", "render_configure_file.py") class TestConfigureFile(unittest.TestCase): @@ -297,7 +297,10 @@ def test_pregenerated_include_gets_configure_file_genrule(self) -> None: self.assertEqual(content.count("genrule("), 1) self.assertIn('name = "configure_pregenerated_include_flow_ProtocolVersion_h"', content) self.assertIn('":pregenerated/include/flow/ProtocolVersion.h"', content) - self.assertIn("//contrib/posttreatments:render_configure_file", content) + self.assertIn('name = "render_configure_file"', content) + self.assertIn('":render_configure_file"', content) + self.assertIn("$(location :render_configure_file)", content) + self.assertIn("$(location ProtocolVersions.cmake) --var", content) self.assertIn("--var CLI_VALUE=abc", content) self.assertIn(":pregenerated/include/flow/ProtocolVersion.h", content) self.assertTrue(any(isinstance(target, BazelGenRuleTarget) for target in bb.bazelTargets)) diff --git a/test/test_parser_utils.py b/test/test_parser_utils.py index fe27f77..3540715 100644 --- a/test/test_parser_utils.py +++ b/test/test_parser_utils.py @@ -1,11 +1,14 @@ import sys +import tempfile import unittest +from pathlib import Path from types import SimpleNamespace from unittest import mock from parser import ( _build_post_treatment_command, _configure_vars_from_cli_paths, + install_configure_file_tool, parse_manually_generated, run_post_treatments, ) @@ -65,6 +68,14 @@ def test_configure_vars_from_cli_paths_allows_cli_override(self): self.assertEqual(values["CMAKE_BINARY_DIR"], "/tmp/project/build") self.assertEqual(values["CUSTOM"], "yes") + def test_install_configure_file_tool_copies_bazel_package(self): + with tempfile.TemporaryDirectory() as td: + install_configure_file_tool(td, ".") + + self.assertTrue( + (Path(td) / "bazel" / "tools" / "render_configure_file.py").exists() + ) + @mock.patch("parser.subprocess.run") @mock.patch("parser.os.path.exists", return_value=True) def test_run_post_treatments_runs_all_scripts(self, _exists, run): From f14eaad87530a045e932d26f3ee4883f6c0998fc Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 26 Apr 2026 22:45:14 -0700 Subject: [PATCH 10/15] Deal with #cmakedefine values --- configure_file.py | 35 ++++++++-- test/test_configure_file.py | 77 +++++++++++++++++++++ tools/render_configure_file.py | 121 +++++++++++++++++++++++++++++++++ 3 files changed, 227 insertions(+), 6 deletions(-) create mode 100644 tools/render_configure_file.py diff --git a/configure_file.py b/configure_file.py index f654b1c..e662976 100644 --- a/configure_file.py +++ b/configure_file.py @@ -7,6 +7,7 @@ PLACEHOLDER_RE = re.compile(r"@([A-Za-z_][A-Za-z0-9_]*)@|\$\{([A-Za-z_][A-Za-z0-9_]*)\}") +CMAKE_DEFINE_RE = re.compile(r"^\s*#\s*cmakedefine(?:01)?\s+([A-Za-z_][A-Za-z0-9_]*)", re.MULTILINE) SET_RE_TEMPLATE = r"set\s*\(\s*{name}(?:\s|\))" @@ -99,14 +100,16 @@ def _parse_configure_file_args(line: str) -> Optional[List[str]]: return args -def _find_placeholders(template_path: str) -> Set[str]: +def _find_placeholders(template_path: str) -> tuple[Set[str], Set[str]]: try: with open(template_path, "r") as f: contents = f.read() except OSError as exc: logging.fatal(f"Cannot read configure_file template {template_path}: {exc}") raise SystemExit(-1) - return {match.group(1) or match.group(2) for match in PLACEHOLDER_RE.finditer(contents)} + required = {match.group(1) or match.group(2) for match in PLACEHOLDER_RE.finditer(contents)} + optional = {match.group(1) for match in CMAKE_DEFINE_RE.finditer(contents)} + return required, optional def _iter_candidate_files(rootdir: str) -> Iterable[str]: @@ -118,7 +121,12 @@ def _iter_candidate_files(rootdir: str) -> Iterable[str]: yield os.path.join(current, filename) -def _find_value_files(rootdir: str, placeholders: Set[str], template_path: str) -> tuple[str, ...]: +def _find_value_files( + rootdir: str, + placeholders: Set[str], + template_path: str, + fail_on_missing: bool = True, +) -> tuple[str, ...]: if not placeholders: return () @@ -136,7 +144,7 @@ def _find_value_files(rootdir: str, placeholders: Set[str], template_path: str) found[placeholder].add(path) missing = sorted([placeholder for placeholder, paths in found.items() if not paths]) - if missing: + if missing and fail_on_missing: logging.fatal( "Missing CMake definitions for configure_file placeholders " f"{', '.join(missing)} in {template_path}" @@ -276,8 +284,23 @@ def parse_configure_files_list( continue variables = configure_vars or {} - placeholders = _find_placeholders(source) - set(variables.keys()) - value_files = _find_value_files(source_dir, placeholders, source) + required_placeholders, optional_placeholders = _find_placeholders(source) + configured_variable_names = set(variables.keys()) + required_placeholders -= configured_variable_names + optional_placeholders -= configured_variable_names + value_files = tuple( + sorted( + set(_find_value_files(source_dir, required_placeholders, source)) + | set( + _find_value_files( + source_dir, + optional_placeholders, + source, + fail_on_missing=False, + ) + ) + ) + ) entry = ConfigureFile( source=source, output=output, diff --git a/test/test_configure_file.py b/test/test_configure_file.py index d6485ca..120f779 100644 --- a/test/test_configure_file.py +++ b/test/test_configure_file.py @@ -245,6 +245,83 @@ def test_render_configure_file_uses_cli_vars(self) -> None: "#define A from-cli\n#define B from-file\n", ) + def test_render_configure_file_handles_cmakedefine(self) -> None: + with tempfile.TemporaryDirectory() as td: + template = Path(td) / "config.h.cmake" + values = Path(td) / "values.cmake" + output = Path(td) / "out" / "config.h" + template.write_text( + "#cmakedefine ENABLED\n" + "# cmakedefine SPACED\n" + "#cmakedefine DISABLED\n" + "#cmakedefine WITH_VALUE @VALUE@\n" + "# cmakedefine01 SPACED_FEATURE\n" + "#cmakedefine01 FEATURE\n" + "#cmakedefine01 MISSING\n" + ) + values.write_text( + "set(ENABLED ON)\n" + "set(SPACED ON)\n" + "set(DISABLED OFF)\n" + "set(WITH_VALUE YES)\n" + "set(VALUE 123)\n" + "set(SPACED_FEATURE TRUE)\n" + "set(FEATURE TRUE)\n" + ) + + subprocess.run( + [sys.executable, RENDERER, str(template), str(output), str(values)], + check=True, + ) + + self.assertEqual( + output.read_text(), + "#define ENABLED\n" + "#define SPACED\n" + "/* #undef DISABLED */\n" + "#define WITH_VALUE 123\n" + "#define SPACED_FEATURE 1\n" + "#define FEATURE 1\n" + "#define MISSING 0\n", + ) + + def test_parse_configure_files_list_finds_cmakedefine_value_files(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) / "src" + build = Path(td) / "build" + root.mkdir() + build.mkdir() + (root / "config.h.cmake").write_text("# cmakedefine ENABLED\n") + (root / "values.cmake").write_text("set(ENABLED ON)\n") + list_file = Path(td) / "configure_files.txt" + list_file.write_text( + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/config.h)\n" + ) + + parsed = parse_configure_files_list(str(list_file), str(root), str(build)) + + self.assertIn("config.h", parsed) + self.assertEqual(len(parsed["config.h"].value_files), 1) + + def test_parse_configure_files_list_allows_missing_cmakedefine(self) -> None: + with tempfile.TemporaryDirectory() as td: + root = Path(td) / "src" + build = Path(td) / "build" + root.mkdir() + build.mkdir() + (root / "config.h.cmake").write_text("#cmakedefine NDEBUG\n") + list_file = Path(td) / "configure_files.txt" + list_file.write_text( + "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config.h.cmake " + "${CMAKE_CURRENT_BINARY_DIR}/config.h)\n" + ) + + parsed = parse_configure_files_list(str(list_file), str(root), str(build)) + + self.assertIn("config.h", parsed) + self.assertEqual(parsed["config.h"].value_files, ()) + def test_pregenerated_include_gets_configure_file_genrule(self) -> None: with tempfile.TemporaryDirectory() as td: root = Path(td) / "src" diff --git a/tools/render_configure_file.py b/tools/render_configure_file.py new file mode 100644 index 0000000..2767632 --- /dev/null +++ b/tools/render_configure_file.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python3 +import argparse +import re +from pathlib import Path + + +SET_RE = re.compile( + r"set\(\s*([A-Za-z_][A-Za-z0-9_]*)\s+(.*?)\s*\)", + re.DOTALL, +) +AT_PLACEHOLDER_RE = re.compile(r"@([A-Za-z_][A-Za-z0-9_]*)@") +DOLLAR_PLACEHOLDER_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)\}") +CMAKE_DEFINE_RE = re.compile( + r"^([ \t]*)#[ \t]*cmakedefine(01)?[ \t]+([A-Za-z_][A-Za-z0-9_]*)(.*)$", + re.MULTILINE, +) + + +def _normalize_value(raw_value: str) -> str: + value = raw_value.strip() + if "#" in value: + value = value.split("#", 1)[0].rstrip() + if value.startswith('"') and value.endswith('"'): + return value[1:-1] + return value + + +def parse_cmake_definitions(contents: str) -> dict[str, str]: + definitions: dict[str, str] = {} + for match in SET_RE.finditer(contents): + definitions[match.group(1)] = _normalize_value(match.group(2)) + return definitions + + +def _cmake_value_is_true(value: str | None) -> bool: + if value is None: + return False + normalized = value.strip().strip('"').upper() + return normalized not in { + "", + "0", + "FALSE", + "OFF", + "NO", + "N", + "IGNORE", + "NOTFOUND", + } and not normalized.endswith("-NOTFOUND") + + +def _render_cmakedefines(template: str, definitions: dict[str, str]) -> str: + def replace(match: re.Match[str]) -> str: + indent, define01, key, suffix = match.groups() + if define01: + return f"{indent}#define {key} {1 if _cmake_value_is_true(definitions.get(key)) else 0}" + if _cmake_value_is_true(definitions.get(key)): + return f"{indent}#define {key}{suffix}" + return f"{indent}/* #undef {key} */" + + return CMAKE_DEFINE_RE.sub(replace, template) + + +def render_template(template: str, definitions: dict[str, str]) -> str: + def replace_at(match: re.Match[str]) -> str: + key = match.group(1) + return definitions.get(key, match.group(0)) + + def replace_dollar(match: re.Match[str]) -> str: + key = match.group(1) + return definitions.get(key, match.group(0)) + + rendered = _render_cmakedefines(template, definitions) + rendered = AT_PLACEHOLDER_RE.sub(replace_at, rendered) + return DOLLAR_PLACEHOLDER_RE.sub(replace_dollar, rendered) + + +def generate_file( + template_path: Path, + output_path: Path, + values_paths: list[Path], + variables: dict[str, str], +) -> None: + definitions: dict[str, str] = {} + for values_path in values_paths: + definitions.update(parse_cmake_definitions(values_path.read_text())) + definitions.update(variables) + rendered = render_template(template_path.read_text(), definitions) + output_path.parent.mkdir(parents=True, exist_ok=True) + output_path.write_text(rendered) + + +def main() -> int: + parser = argparse.ArgumentParser(description="Render a CMake configure_file template") + parser.add_argument("template") + parser.add_argument("output") + parser.add_argument("values", nargs="*") + parser.add_argument( + "--var", + action="append", + default=[], + help="Template variable in the form key=value", + ) + args = parser.parse_args() + + variables = {} + for variable in args.var: + if "=" not in variable: + parser.error(f"--var must be in the form key=value: {variable}") + key, value = variable.split("=", 1) + variables[key] = value + generate_file( + Path(args.template), + Path(args.output), + [Path(v) for v in args.values], + variables, + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From d0014eecc68cdc5ccf85b000611483db7c879dfd Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sun, 26 Apr 2026 23:43:54 -0700 Subject: [PATCH 11/15] Do not parse env_set as set --- configure_file.py | 2 +- test/test_configure_file.py | 12 +++++++++++- tools/render_configure_file.py | 16 +++++++++++++++- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/configure_file.py b/configure_file.py index e662976..a509e94 100644 --- a/configure_file.py +++ b/configure_file.py @@ -8,7 +8,7 @@ PLACEHOLDER_RE = re.compile(r"@([A-Za-z_][A-Za-z0-9_]*)@|\$\{([A-Za-z_][A-Za-z0-9_]*)\}") CMAKE_DEFINE_RE = re.compile(r"^\s*#\s*cmakedefine(?:01)?\s+([A-Za-z_][A-Za-z0-9_]*)", re.MULTILINE) -SET_RE_TEMPLATE = r"set\s*\(\s*{name}(?:\s|\))" +SET_RE_TEMPLATE = r"(?:^|[^A-Za-z0-9_])(?:env_set|set)\s*\(\s*{name}(?:\s|\))" @dataclass(frozen=True) diff --git a/test/test_configure_file.py b/test/test_configure_file.py index 120f779..4afdfc2 100644 --- a/test/test_configure_file.py +++ b/test/test_configure_file.py @@ -253,6 +253,9 @@ def test_render_configure_file_handles_cmakedefine(self) -> None: template.write_text( "#cmakedefine ENABLED\n" "# cmakedefine SPACED\n" + "#cmakedefine ENV_DISABLED\n" + "#cmakedefine CACHE_DISABLED\n" + "#cmakedefine FROM_ENV_VAR\n" "#cmakedefine DISABLED\n" "#cmakedefine WITH_VALUE @VALUE@\n" "# cmakedefine01 SPACED_FEATURE\n" @@ -262,6 +265,10 @@ def test_render_configure_file_handles_cmakedefine(self) -> None: values.write_text( "set(ENABLED ON)\n" "set(SPACED ON)\n" + "env_set(ENV_DISABLED OFF BOOL \"disabled from env_set\")\n" + "set(CACHE_DISABLED OFF CACHE BOOL \"disabled from cache\")\n" + "set(default_value OFF)\n" + "env_set(FROM_ENV_VAR ${default_value} BOOL \"disabled from variable\")\n" "set(DISABLED OFF)\n" "set(WITH_VALUE YES)\n" "set(VALUE 123)\n" @@ -278,6 +285,9 @@ def test_render_configure_file_handles_cmakedefine(self) -> None: output.read_text(), "#define ENABLED\n" "#define SPACED\n" + "/* #undef ENV_DISABLED */\n" + "/* #undef CACHE_DISABLED */\n" + "/* #undef FROM_ENV_VAR */\n" "/* #undef DISABLED */\n" "#define WITH_VALUE 123\n" "#define SPACED_FEATURE 1\n" @@ -292,7 +302,7 @@ def test_parse_configure_files_list_finds_cmakedefine_value_files(self) -> None: root.mkdir() build.mkdir() (root / "config.h.cmake").write_text("# cmakedefine ENABLED\n") - (root / "values.cmake").write_text("set(ENABLED ON)\n") + (root / "values.cmake").write_text("env_set(ENABLED ON BOOL \"enabled\")\n") list_file = Path(td) / "configure_files.txt" list_file.write_text( "configure_file(${CMAKE_CURRENT_SOURCE_DIR}/config.h.cmake " diff --git a/tools/render_configure_file.py b/tools/render_configure_file.py index 2767632..73a10dd 100644 --- a/tools/render_configure_file.py +++ b/tools/render_configure_file.py @@ -5,7 +5,12 @@ SET_RE = re.compile( - r"set\(\s*([A-Za-z_][A-Za-z0-9_]*)\s+(.*?)\s*\)", + r"(? str: value = raw_value.strip() if "#" in value: value = value.split("#", 1)[0].rstrip() + cache_match = re.match( + r"(.+?)\s+CACHE\s+(?:BOOL|STRING|PATH|FILEPATH|INTERNAL)\b.*", + value, + re.DOTALL, + ) + if cache_match: + value = cache_match.group(1).strip() if value.startswith('"') and value.endswith('"'): return value[1:-1] return value @@ -29,6 +41,8 @@ def parse_cmake_definitions(contents: str) -> dict[str, str]: definitions: dict[str, str] = {} for match in SET_RE.finditer(contents): definitions[match.group(1)] = _normalize_value(match.group(2)) + for match in ENV_SET_RE.finditer(contents): + definitions[match.group(1)] = render_template(_normalize_value(match.group(2)), definitions) return definitions From 917f89399816b75bff14b6f4f6ea8b61d1356930 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Wed, 29 Apr 2026 23:05:07 -0700 Subject: [PATCH 12/15] Propagate the dependencies found in files generated to the target that use the generated files --- build.py | 18 +++++++++++++++++ test/test_build.py | 50 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/build.py b/build.py index 2860f1b..10a6565 100644 --- a/build.py +++ b/build.py @@ -526,6 +526,22 @@ def _addAllCCimportDeps( if isinstance(d, BazelCCImport): cls._addAllCCimportDeps(d, ctx) + @classmethod + def _propagateGeneratedSourceCCImportDeps( + cls, + el: "BuildTarget", + ctx: BazelBuildVisitorContext, + ) -> None: + if not isinstance(ctx.current, BazelTarget): + return + + for dep in el.depends: + imp = getattr(dep, "opaque", None) + if not isinstance(imp, BazelCCImport): + continue + ctx.current.addDep(imp) + cls._addAllCCimportDeps(imp, ctx) + @classmethod def _genConfigureFileRule( cls, @@ -1215,6 +1231,7 @@ def _handleCustomCommandForBazelGen( or t.name.endswith(".cpp") ): ctx.current.addSrc(t) + self._propagateGeneratedSourceCCImportDeps(el, ctx) logging.debug(f"Found {t} in {ctx.current.name} CC") self._handleIncludeBazelTarget(el, ctx, workDir) else: @@ -1488,6 +1505,7 @@ def _handleCPPCompileCommand( # Usually when it's none it's because we have pseudo targets return True assert isinstance(ctx.current, BazelTarget) + self._propagateGeneratedSourceCCImportDeps(el, ctx) build = el.producedby assert build is not None diff --git a/test/test_build.py b/test/test_build.py index 806d40b..b66abcf 100644 --- a/test/test_build.py +++ b/test/test_build.py @@ -5,7 +5,7 @@ from pathlib import Path sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) -from bazel import BazelGenRuleTarget +from bazel import BazelCCImport, BazelGenRuleTarget from bazel import BazelTarget, BazelBuild, getObject, bazelcache from build import BazelBuildVisitorContext, Build, BuildTarget, Rule from ninjabuild import canBePruned @@ -109,6 +109,54 @@ def test_handle_cpp_compile_command_filters_flags_and_defines(self) -> None: self.assertEqual(lib.defines, {'"KEEP"', '"DEF2"'}) self.assertEqual(lib.copts, {'"-Wall"', '"-funroll"'}) + def test_generated_source_cc_import_deps_are_propagated_to_current_target(self) -> None: + bazelbuild = BazelBuild("src/") + ctx = BazelBuildVisitorContext( + parentIsPhony=False, + rootdir="/", + bazelbuild=bazelbuild, + flagsToIgnore=[], + prefix="src", + ) + lib = BazelTarget("cc_library", "lib", "src") + ctx.current = lib + + imp = BazelCCImport("boost_filesystem") + dep = BuildTarget("boost_filesystem", ("boost_filesystem", None)).setOpaque(imp) + generated = BuildTarget("generated.cc", ("generated.cc", None)) + generated.setDeps([dep]) + + Build._propagateGeneratedSourceCCImportDeps(generated, ctx) + + self.assertIn(imp, lib.deps) + self.assertIn(imp, bazelbuild.bazelTargets) + + def test_cpp_compile_propagates_cc_import_deps_from_object_target(self) -> None: + bazelbuild = BazelBuild("src/") + ctx = BazelBuildVisitorContext( + parentIsPhony=False, + rootdir="/", + bazelbuild=bazelbuild, + flagsToIgnore=[], + prefix="src", + ) + lib = BazelTarget("cc_library", "flow", "src") + ctx.current = lib + + imp = BazelCCImport("boost_filesystem") + dep = BuildTarget("boost_filesystem", ("boost_filesystem", None)).setOpaque(imp) + obj = BuildTarget("flow/CMakeFiles/flow.dir/Platform.actor.g.cpp.o", ("obj.o", None)) + obj.setDeps([dep]) + src = BuildTarget("flow/Platform.actor.g.cpp", ("flow/Platform.actor.g.cpp", None)).markAsFile() + build = Build([obj], Rule("CXX_COMPILER"), [src], []) + build.vars["FLAGS"] = "" + build.vars["DEFINES"] = "" + + self.assertTrue(build._handleCPPCompileCommand(ctx, obj)) + + self.assertIn(imp, lib.deps) + self.assertIn(imp, bazelbuild.bazelTargets) + def test_get_core_command_extracts_run_directory(self) -> None: with tempfile.TemporaryDirectory() as td: tmp_path = Path(td) From 3dab60685665c681797a31437fc69cf479bc9e29 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Thu, 30 Apr 2026 12:03:40 -0700 Subject: [PATCH 13/15] Refactor how we discover that the command to run for genrule is python based This relies on better detecting the binary for python + also detecting the argument to the python script. --- build.py | 61 +++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 14 deletions(-) diff --git a/build.py b/build.py index 10a6565..8833b7e 100644 --- a/build.py +++ b/build.py @@ -7,11 +7,22 @@ from functools import total_ordering from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Union -from bazel import (BaseBazelTarget, BazelBuild, BazelCCImport, - BazelCCProtoLibrary, BazelExternalDep, BazelGenRuleTarget, - BazelGenRuleTargetOutput, BazelGRPCCCProtoLibrary, - BazelProtoLibrary, BazelTarget, ExportedFile, PyBinaryBazelTarget, - ShBinaryBazelTarget, getObject) +from bazel import ( + BaseBazelTarget, + BazelBuild, + BazelCCImport, + BazelCCProtoLibrary, + BazelExternalDep, + BazelGenRuleTarget, + BazelGenRuleTargetOutput, + BazelGRPCCCProtoLibrary, + BazelProtoLibrary, + BazelTarget, + ExportedFile, + PyBinaryBazelTarget, + ShBinaryBazelTarget, + getObject, +) from configure_file import ConfigureFile, find_configure_file from helpers import resolvePath from visitor import VisitorContext @@ -596,9 +607,8 @@ def _genConfigureFileRule( for key, value in sorted(configure_file.variables.items()) ] ) - genTarget.cmd = ( - f"$(location :{CONFIGURE_FILE_TOOL_TARGET}) " - + " ".join(args) + genTarget.cmd = f"$(location :{CONFIGURE_FILE_TOOL_TARGET}) " + " ".join( + args ) ctx.bazelbuild.bazelTargets.add(genTarget) return next(iter(genTarget.outs)) @@ -1078,8 +1088,25 @@ def _handleCustomCommandForBazelGen( + f"{'/'.join(['..' for d in firstOutput.split('/')[:-1]])}" + "/" ) + + regex = r"^(?:.*/bin/)?python3(?:\.\d+)?$" + start_idx = 1 + script = [] + script_raw_files = [] + if re.match(regex, command): + command = "python3" + if arr[1].endswith(".py"): + start_idx = 2 + script.append(f"$(location {arr[1]})") + script_raw_files.append(arr[1]) + elif command.endswith(".py"): + # Replace command by python3 and pass the script as first argument + script.append(f"$(location {command})") + script_raw_files.append(command) + command = "python3" + lastArgIsOption = False - for arg in arr[1:]: + for arg in arr[start_idx:]: if arg.startswith("-"): # There will be an issue with options that take multiple values ie --foo bar # baz biz @@ -1167,10 +1194,10 @@ def _handleCustomCommandForBazelGen( toolBuildTarget.addOut(f"{sanitized_command}_wrapper.sh") # Add the sha1 of all inputs to force rebuild if intput file changes - if (countInput + countRewrote + countOptions) != len(arr[1:]): + if (countInput + countRewrote + countOptions) != len(arr[start_idx:]): logging.warn( f"Need to write the function for dealing with non fully rewritten arguments for {el.name}" - f", {countInput}, {countRewrote}, {countOptions} {len(arr[1:])}" + f", {countInput}, {countRewrote}, {countOptions} {len(arr[start_idx:])}" ) # Make a sh_binary target out of iter shBinary = ShBinaryBazelTarget(f"{sanitized_command}_cmd", location) @@ -1182,14 +1209,20 @@ def _handleCustomCommandForBazelGen( ) genTarget.addTool(shBinary) - # Not sure that it's actually needed - # FIXME do not do that for the genrule that create the script that runs generator for e in allInputs: - genTarget.addSrc( + if e not in script_raw_files: + genTarget.addSrc( + self._genExportedFile( + filename=e, locationCaller=genTarget.location, ctx=ctx + ) + ) + for e in script_raw_files: + genTarget.addTool( self._genExportedFile( filename=e, locationCaller=genTarget.location, ctx=ctx ) ) + ctx.bazelbuild.bazelTargets.add(toolBuildTarget) ctx.bazelbuild.bazelTargets.add(shBinary) From 7ff649baded8430ee48a81df2406e3fcebc8ce73 Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Thu, 30 Apr 2026 17:53:47 -0700 Subject: [PATCH 14/15] Do not add blindly deps for a genrule target as sources When processsing the target we already parse through its sources output and tools so we don't need to redo it --- build.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/build.py b/build.py index 8833b7e..939d471 100644 --- a/build.py +++ b/build.py @@ -822,7 +822,12 @@ def handleFileForBazelGen( ) if not isinstance(exported, BazelGenRuleTargetOutput): ctx.bazelbuild.bazelTargets.add(exported) - ctx.current.addSrc(exported) + + if not isinstance(ctx.current, BazelGenRuleTarget): + # Do not add sources for genRule targets as we already have done that before + # this is at best redundant and at worse wrong as some sources might be actually + # tools + ctx.current.addSrc(exported) if el.includes is None: return From 55d7948adb49e0083cf48d7b992abd8b30f5dc0c Mon Sep 17 00:00:00 2001 From: Matthieu Patou Date: Sat, 2 May 2026 08:21:06 -0700 Subject: [PATCH 15/15] Add .s/.S (assembly) that are generated to the sources rather than data. --- build.py | 7 ++----- test/test_build.py | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/build.py b/build.py index 939d471..bff5e17 100644 --- a/build.py +++ b/build.py @@ -33,6 +33,7 @@ ) CONFIGURE_FILE_TOOL_PATH = "bazel/tools/render_configure_file.py" CONFIGURE_FILE_TOOL_TARGET = "render_configure_file" +CPP_SOURCE_EXTENSIONS = (".c", ".cc", ".cpp", ".s", ".S") def genShBinaryScript(rootdir: str, command: str) -> str: @@ -1263,11 +1264,7 @@ def _handleCustomCommandForBazelGen( ctx.current.addHdr(t) # The current buildTarget is a C/C++ file it means that the current build (ie. binary/test/lib) # has it as input, so we add it as a src to the current bazelTarget - elif ( - t.name.endswith(".c") - or t.name.endswith(".cc") - or t.name.endswith(".cpp") - ): + elif t.name.endswith(CPP_SOURCE_EXTENSIONS): ctx.current.addSrc(t) self._propagateGeneratedSourceCCImportDeps(el, ctx) logging.debug(f"Found {t} in {ctx.current.name} CC") diff --git a/test/test_build.py b/test/test_build.py index b66abcf..adaede2 100644 --- a/test/test_build.py +++ b/test/test_build.py @@ -251,6 +251,30 @@ def test_custom_command_multiple_inputs_outputs(self) -> None: self.assertIn("$(location :out2.txt)", gen.cmd) self.assertEqual(gen.aliases.get("out1.txt"), "alias/out1.txt") + def test_generated_assembly_output_is_added_as_source(self) -> None: + out = BuildTarget("bindings/c/fdb_c.g.S", ("bindings/c/fdb_c.g.S", None)) + build = Build([out], Rule("CUSTOM_COMMAND"), [], []) + gen = BazelGenRuleTarget("generate_fdb_c_asm", ".") + gen.addOut("bindings/c/fdb_c.g.S") + build.associatedBazelTarget = gen + + lib = BazelTarget("cc_library", "fdb_c", ".") + bazelbuild = BazelBuild("") + ctx = BazelBuildVisitorContext( + parentIsPhony=False, + rootdir="/root", + bazelbuild=bazelbuild, + flagsToIgnore=[], + current=lib, + prefix="", + ) + + build._handleCustomCommandForBazelGen(ctx, out, "tool bindings/c/fdb_c.g.S") + + generated_src = next(iter(gen.outs)) + self.assertIn(generated_src, lib.srcs) + self.assertNotIn(generated_src, lib.data) + def setUp(self) -> None: bazelcache.clear()