bazel_to_cmake: Improve handling of .proto files in multiple targets

There are a number of issues with the older mechanism:

1. The mechanism used by btc_protobuf.cmake to collect sources collected
   INTERFACE sources during proto generation, which could invoke protoc
   on dependencies multiple times.

2. Some .proto files are included in multiple different proto_library() rules.
   This would again cause the protoc compiler to be invoked multiple times,
   and could lead to duplicate identical outputs and/or problems locating the
   appropriate generated file.

That bazel_to_cmake appeared to work appears to be somewhat of an accident;
Protoc was invoked on .proto files more than necessary, and include directories
were over-specified, so by luck the dependency mechanism in ninja/make appeared
to build the correct target in many of these cases.

Now, however, these accidents are improved with the following fixes:

1. Invoke protoc directly using generator expressions to collect the
   correct imports only from the proto_library(deps=) targets.
   Also declares outputs and proto inputs explicitly.

2. Instead of invoking the protoc compiler rules for each proto_library(),
   target, invoke protoc for each file target included by a proto_library()

3. Generalize dependency handling to collect matching providers in
   a 'ProviderCollector' to better specify target_link_libraries vs.
   add_dependencies.  This solves some cases of bad aliasing that was
   potentially generated (i.e. an alias to an alias).

This includes a bunch of other changes that move some code around and
rework using StringIO buffers and other fixes as well.

See bug report: #197

PiperOrigin-RevId: 676089562
Change-Id: I64eba6e9e232af354a2b35d1b11e7c8b185aa1bc

tools/cmake/BUILD

@@ -23,9 +23,11 @@ SRCS = [
     "bazel_to_cmake/bzl_library/third_party_http_archive.py",
     "bazel_to_cmake/bzl_library/upb_proto_library.py",
     "bazel_to_cmake/cmake_builder.py",
+    "bazel_to_cmake/cmake_provider.py",
     "bazel_to_cmake/cmake_repository_test.py",
     "bazel_to_cmake/cmake_repository.py",
     "bazel_to_cmake/cmake_target.py",
+    "bazel_to_cmake/emit_alias.py",
     "bazel_to_cmake/emit_cc_test.py",
     "bazel_to_cmake/emit_cc.py",
     "bazel_to_cmake/emit_filegroup.py",
@@ -42,6 +44,7 @@ SRCS = [
     "bazel_to_cmake/native_rules.py",
     "bazel_to_cmake/package.py",
     "bazel_to_cmake/platforms.py",
+    "bazel_to_cmake/provider_util.py",
     "bazel_to_cmake/starlark/__init__.py",
     "bazel_to_cmake/starlark/bazel_build_file.py",
     "bazel_to_cmake/starlark/bazel_glob_test.py",

tools/cmake/bazel_to_cmake/bzl_library/bazel_skylib.py

@@ -15,13 +15,15 @@
 
 # pylint: disable=invalid-name,relative-beyond-top-level,missing-function-docstring,missing-class-docstring,g-long-lambda
 
+import itertools
 import json
 import pathlib
 from typing import Any, Dict, List, Optional, Tuple, TypeVar, Union, cast
 
 from .. import native_rules_genrule
 from ..cmake_builder import CMakeBuilder
-from ..cmake_target import CMakeAddDependenciesProvider
+from ..cmake_provider import CMakeAddDependenciesProvider
+from ..cmake_provider import default_providers
 from ..cmake_target import CMakeTarget
 from ..evaluation import EvaluationState
 from ..starlark.bazel_globals import BazelGlobals
@@ -43,6 +45,7 @@
 from ..util import quote_string
 from ..util import write_file_if_not_already_equal
 
+
 T = TypeVar("T")
 
 
@@ -189,10 +192,17 @@ def _expand_template_impl(
       cast(RelativeLabel, _context.evaluate_configurable(template))
   )
 
-  deps: List[CMakeTarget] = []
-  template_paths = state.get_file_paths([resolved_template], deps)
+  template_collector = state.collect_targets([resolved_template])
+
+  add_dependencies: set[CMakeTarget] = set(
+      itertools.chain(
+          template_collector.add_dependencies(),
+          template_collector.file_paths(),
+      )
+  )
+  template_paths = list(template_collector.file_paths())
+  assert len(template_paths) == 1, f"For {template_collector}"
 
-  assert len(template_paths) == 1
   template_path = template_paths[0]
   script_path = pathlib.PurePath(__file__).parent.joinpath("expand_template.py")
 
@@ -205,25 +215,27 @@ def _expand_template_impl(
       subs_path,
       json.dumps(_context.evaluate_configurable(substitutions)).encode("utf-8"),
   )
-  deps.append(CMakeTarget(template_path))
-  deps.append(CMakeTarget(script_path.as_posix()))
-  deps.append(CMakeTarget(subs_path.as_posix()))
+  add_dependencies.add(CMakeTarget(template_path))
+  add_dependencies.add(CMakeTarget(script_path.as_posix()))
+  add_dependencies.add(CMakeTarget(subs_path.as_posix()))
 
   builder: CMakeBuilder = _context.access(CMakeBuilder)
   builder.addtext(f"""
+# expand_template({_target.as_label()})
 add_custom_command(
 OUTPUT {quote_path(out_file)}
 COMMAND ${{Python3_EXECUTABLE}} {quote_path(script_path)}
         {quote_path(template_path)}
         {quote_path(subs_path)}
         {quote_path(out_file)}
-DEPENDS {quote_list(deps)}
+DEPENDS {quote_list(sorted(add_dependencies))}
 VERBATIM
 )
 add_custom_target({cmake_target_pair.target} DEPENDS {quote_path(out_file)})
 """)
   _context.add_analyzed_target(
-      _target, TargetInfo(*cmake_target_pair.as_providers())
+      _target,
+      TargetInfo(*default_providers(cmake_target_pair)),
   )
 
 

tools/cmake/bazel_to_cmake/bzl_library/grpc_generate_cc.py

@@ -14,15 +14,16 @@
 """CMake implementation of "@tensorstore//bazel:cc_grpc_library.bzl"."""
 
 # pylint: disable=invalid-name,missing-function-docstring,relative-beyond-top-level,g-long-lambda
+import io
+import pathlib
 from typing import Any, List, Optional, cast
 
 from ..cmake_builder import CMakeBuilder
-from ..cmake_target import CMakeAddDependenciesProvider
-from ..cmake_target import CMakeLinkLibrariesProvider
-from ..cmake_target import CMakeTarget
+from ..cmake_provider import CMakeAddDependenciesProvider
+from ..cmake_provider import default_providers
 from ..evaluation import EvaluationState
 from ..native_aspect_proto import btc_protobuf
-from ..native_aspect_proto import get_aspect_dep
+from ..native_aspect_proto import plugin_generated_files
 from ..native_aspect_proto import PluginSettings
 from ..starlark.bazel_globals import BazelGlobals
 from ..starlark.bazel_globals import register_bzl_library
@@ -34,8 +35,8 @@
 from ..starlark.invocation_context import RelativeLabel
 from ..starlark.provider import TargetInfo
 from ..starlark.select import Configurable
-
-from .upb_proto_library import UPB_REPO  # pylint: disable=unused-import
+from ..util import quote_path_list
+from .upb_proto_library import UPB_PLUGIN  # pylint: disable=unused-import
 
 GRPC_REPO = RepositoryId("com_github_grpc_grpc")
 
@@ -49,6 +50,7 @@ def _empty_target_list(t: TargetId) -> List[TargetId]:
 
 _GRPC = PluginSettings(
     name="grpc",
+    language="grpc",
     plugin=GRPC_REPO.parse_target("//src/compiler:grpc_cpp_plugin"),
     exts=[".grpc.pb.h", ".grpc.pb.cc"],
     runtime=[GRPC_REPO.parse_target("//:grpc++_codegen_proto")],
@@ -92,6 +94,17 @@ def _generate_grpc_cc_impl(
 ):
   del kwargs
   state = _context.access(EvaluationState)
+  repo = state.workspace.all_repositories.get(
+      _context.caller_package_id.repository_id
+  )
+  assert repo is not None
+
+  # Only a single source is allowed.
+  resolved_srcs = _context.resolve_target_or_label_list(
+      _context.evaluate_configurable_list(srcs)
+  )
+  assert len(resolved_srcs) == 1
+  proto_target = resolved_srcs[0]
 
   plugin_settings = _GRPC
   if plugin is not None:
@@ -100,79 +113,91 @@ def _generate_grpc_cc_impl(
     )
     plugin_settings = PluginSettings(
         name=_GRPC.name,
+        language=_GRPC.language,
         plugin=resolved_plugin,
         exts=_GRPC.exts,
         runtime=_GRPC.runtime,
         aspectdeps=_empty_target_list,
     )
 
-  assert plugin_settings.plugin is not None
-  cmake_target_pair = state.generate_cmake_target_pair(_target, alias=False)
+  proto_target_info = _context.get_target_info(proto_target)
+  proto_library_provider = proto_target_info.get(ProtoLibraryProvider)
+  assert proto_library_provider is not None
 
-  # Only a single source is allowed.
-  resolved_srcs = _context.resolve_target_or_label_list(
-      _context.evaluate_configurable_list(srcs)
-  )
-  assert len(resolved_srcs) == 1
-  proto_library_target = resolved_srcs[0]
+  cmake_target_pair = state.generate_cmake_target_pair(_target, alias=False)
 
   # Construct the generated paths, installing this rule as a dependency.
   # TODO: Handle skip_import_prefix?
-  generated_paths = []
-  cmake_deps: List[CMakeTarget] = [
-      get_aspect_dep(
-          _context,
-          proto_library_target.get_target_id(
-              f"{proto_library_target.target_name}__upb_library"
-          ),
-      )
-  ]
-  proto_target_info = _context.get_target_info(proto_library_target)
-  proto_provider = proto_target_info.get(ProtoLibraryProvider)
-  assert proto_provider is not None
+  proto_cmake_target = state.generate_cmake_target_pair(
+      proto_library_provider.bazel_target
+  ).target
+
+  import_targets = set(
+      state.collect_deps(proto_library_provider.deps).link_libraries()
+  )
+  import_targets.add(proto_cmake_target)
 
   # Emit the generated files.
   # See also native_aspect_proto.py
-  repo = state.workspace.all_repositories.get(
-      _context.caller_package_id.repository_id
-  )
-  assert repo is not None
-  for src in proto_provider.srcs:
-    assert src.target_name.endswith(
-        ".proto"
-    ), f"{src.as_label()} must end in .proto"
-    target_name = src.target_name.removesuffix(".proto")
-    for ext in plugin_settings.exts:
-      generated_target = src.get_target_id(f"{target_name}{ext}")
-      generated_path = generated_target.as_rooted_path(repo.cmake_binary_dir)
+  generated_files = []
+  for src in proto_library_provider.srcs:
+    for t in plugin_generated_files(
+        src,
+        plugin_settings,
+        repo.cmake_binary_dir,
+    ):
       _context.add_analyzed_target(
-          generated_target,
+          t[0],
           TargetInfo(
-              FilesProvider([str(generated_path)]),
               CMakeAddDependenciesProvider(cmake_target_pair.target),
+              FilesProvider([t[1]]),
           ),
       )
-      generated_paths.append(generated_path)
-
+      generated_files.append(t[1])
   _context.add_analyzed_target(
       _target,
       TargetInfo(
-          *cmake_target_pair.as_providers(), FilesProvider(generated_paths)
+          *default_providers(cmake_target_pair),
+          FilesProvider(generated_files),
       ),
   )
 
-  out = btc_protobuf(
+  src_collector = state.collect_targets(proto_library_provider.srcs)
+  state.collect_deps(UPB_PLUGIN.aspectdeps(resolved_srcs[0]))
+
+  # Augment output with strip import prefix
+  output_dir = repo.cmake_binary_dir
+  if proto_library_provider.strip_import_prefix:
+    include_path = str(
+        pathlib.PurePosixPath(_context.caller_package_id.package_name).joinpath(
+            proto_library_provider.strip_import_prefix
+        )
+    )
+    if include_path[0] == "/":
+      include_path = include_path[1:]
+    output_dir = output_dir.joinpath(include_path)
+
+  # Emit.
+  out = io.StringIO()
+  out.write(f"\n# {_target.as_label()}")
+
+  btc_protobuf(
       _context,
-      cmake_target_pair.target,
-      proto_target_info.get(CMakeLinkLibrariesProvider).target,
-      plugin_settings,
-      cmake_deps=cmake_deps,
+      out,
+      plugin_settings=plugin_settings,
+      # varname=str(cmake_target_pair.target),
+      proto_src=next(src_collector.file_paths()),
+      generated_files=generated_files,
+      import_targets=import_targets,
+      cmake_depends=set(src_collector.add_dependencies()),
       flags=flags,
+      output_dir=repo.replace_with_cmake_macro_dirs([output_dir])[0],
+  )
+  sep = "\n    "
+
+  out.write(
+      f"add_custom_target({cmake_target_pair.target} DEPENDS\n"
+      f"  {quote_path_list(generated_files, separator=sep)})\n"
   )
 
-  builder = _context.access(CMakeBuilder)
-  builder.addtext(f"""
-# {_target.as_label()}
-add_custom_target({cmake_target_pair.target})
-{out}
-""")
+  _context.access(CMakeBuilder).addtext(out.getvalue())

tools/cmake/bazel_to_cmake/bzl_library/local_mirror.py

@@ -25,6 +25,7 @@
 from ..cmake_builder import LOCAL_MIRROR_DOWNLOAD_SECTION
 from ..cmake_repository import CMakeRepository
 from ..cmake_repository import make_repo_mapping
+from ..cmake_target import CMakePackage
 from ..evaluation import EvaluationState
 from ..starlark.bazel_globals import BazelGlobals
 from ..starlark.bazel_globals import register_bzl_library
@@ -53,7 +54,7 @@ def _local_mirror_impl(
 
   state = _context.access(EvaluationState)
 
-  cmake_name: str = kwargs["cmake_name"]
+  cmake_name = CMakePackage(kwargs["cmake_name"])
   repository_id = RepositoryId(kwargs["name"])
   new_repository = CMakeRepository(
       repository_id=repository_id,