Skip to content

Commit

Permalink
bazel_to_cmake: Allow proto aspects to use multiple computed dependen…
Browse files Browse the repository at this point in the history
…cies.

Better capture aspect dependencies by returning a `List[TargetId]` of possible dependencies for a proto target. Currently these dependencies are required by upb targets which rely on a generated upb minitable.

The aspect code previously relied on transitive dependencies to achieve this.

Related to #197
NOTE: The CMake build is still not working after this submit.
PiperOrigin-RevId: 674940141
Change-Id: If640ddf6f2229d4b935112c11d5ca49b95752697
  • Loading branch information
laramiel authored and copybara-github committed Sep 15, 2024
1 parent 4629236 commit 99573a2
Show file tree
Hide file tree
Showing 24 changed files with 513 additions and 419 deletions.
44 changes: 3 additions & 41 deletions tools/cmake/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ SRCS = [
"bazel_to_cmake/cmake_repository_test.py",
"bazel_to_cmake/cmake_repository.py",
"bazel_to_cmake/cmake_target.py",
"bazel_to_cmake/emit_cc.py",
"bazel_to_cmake/emit_cc_test.py",
"bazel_to_cmake/emit_cc.py",
"bazel_to_cmake/emit_filegroup.py",
"bazel_to_cmake/evaluation.py",
"bazel_to_cmake/golden_test.py",
"bazel_to_cmake/main.py",
"bazel_to_cmake/native_aspect_proto.py",
"bazel_to_cmake/native_aspect.py",
"bazel_to_cmake/native_rules_alias.py",
"bazel_to_cmake/native_rules_cc_proto.py",
"bazel_to_cmake/native_rules_cc.py",
Expand Down Expand Up @@ -163,43 +165,3 @@ tensorstore_pytest_test(
"skip-windows", # path canonicalization isn't correct when testing on windows.
],
)

cc_library(
name = "bazel_to_cmake/testdata/upb_proto_library/x",
srcs = ["bazel_to_cmake/testdata/upb_proto_library/x.cc"],
)

proto_library(
name = "bazel_to_cmake/testdata/rules_proto/a_proto",
srcs = ["bazel_to_cmake/testdata/rules_proto/a.proto"],
)

proto_library(
name = "bazel_to_cmake/testdata/rules_proto/c_proto",
srcs = ["bazel_to_cmake/testdata/rules_proto/c.proto"],
)

proto_library(
name = "bazel_to_cmake/testdata/rules_proto/b_proto",
srcs = ["bazel_to_cmake/testdata/rules_proto/b.proto"],
)

proto_library(
name = "bazel_to_cmake/testdata/upb_proto_library/a_proto",
srcs = ["bazel_to_cmake/testdata/upb_proto_library/a.proto"],
)

proto_library(
name = "bazel_to_cmake/testdata/rules_proto/x_proto",
srcs = ["bazel_to_cmake/testdata/rules_proto/x.proto"],
)

proto_library(
name = "bazel_to_cmake/testdata/rules_proto/src/subdir/y_proto",
srcs = ["bazel_to_cmake/testdata/rules_proto/src/subdir/y.proto"],
)

proto_library(
name = "bazel_to_cmake/testdata/upb_proto_library/b_proto",
srcs = ["bazel_to_cmake/testdata/upb_proto_library/b.proto"],
)
5 changes: 3 additions & 2 deletions tools/cmake/bazel_to_cmake/bzl_library/bazel_skylib.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

from .. import native_rules_genrule
from ..cmake_builder import CMakeBuilder
from ..cmake_target import CMakeDepsProvider
from ..cmake_target import CMakeAddDependenciesProvider
from ..cmake_target import CMakeTarget
from ..evaluation import EvaluationState
from ..starlark.bazel_globals import BazelGlobals
Expand Down Expand Up @@ -180,7 +180,8 @@ def _expand_template_impl(
_context.add_analyzed_target(
_out_target,
TargetInfo(
CMakeDepsProvider([cmake_target_pair.dep]), FilesProvider([out_file])
CMakeAddDependenciesProvider(cmake_target_pair.dep),
FilesProvider([out_file]),
),
)

Expand Down
16 changes: 12 additions & 4 deletions tools/cmake/bazel_to_cmake/bzl_library/grpc_generate_cc.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
from typing import Any, List, Optional, cast

from ..cmake_builder import CMakeBuilder
from ..cmake_target import CMakeDepsProvider
from ..cmake_target import CMakeLibraryTargetProvider
from ..cmake_target import CMakeAddDependenciesProvider
from ..cmake_target import CMakeLinkLibrariesProvider
from ..cmake_target import CMakeTarget
from ..evaluation import EvaluationState
from ..native_aspect_proto import btc_protobuf
Expand All @@ -41,11 +41,18 @@

_SEP = "\n "


def _empty_target_list(t: TargetId) -> List[TargetId]:
del t
return []


_GRPC = PluginSettings(
name="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")],
aspectdeps=_empty_target_list,
)


Expand Down Expand Up @@ -96,6 +103,7 @@ def _generate_grpc_cc_impl(
plugin=resolved_plugin,
exts=_GRPC.exts,
runtime=_GRPC.runtime,
aspectdeps=_empty_target_list,
)

assert plugin_settings.plugin is not None
Expand Down Expand Up @@ -141,7 +149,7 @@ def _generate_grpc_cc_impl(
generated_target,
TargetInfo(
FilesProvider([str(generated_path)]),
CMakeDepsProvider([cmake_target_pair.target]),
CMakeAddDependenciesProvider(cmake_target_pair.target),
),
)
generated_paths.append(generated_path)
Expand All @@ -156,7 +164,7 @@ def _generate_grpc_cc_impl(
out = btc_protobuf(
_context,
cmake_target_pair.target,
proto_target_info.get(CMakeLibraryTargetProvider).target,
proto_target_info.get(CMakeLinkLibrariesProvider).target,
plugin_settings,
cmake_deps=cmake_deps,
flags=flags,
Expand Down
58 changes: 30 additions & 28 deletions tools/cmake/bazel_to_cmake/bzl_library/upb_proto_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
# pylint: disable=relative-beyond-top-level
from typing import List, Optional

from ..native_aspect_proto import add_proto_aspect
from ..native_aspect import add_proto_aspect
from ..native_aspect_proto import aspect_genproto_library_target
from ..native_aspect_proto import PluginSettings
from ..native_rules_cc_proto import cc_proto_library_impl
Expand Down Expand Up @@ -61,6 +61,23 @@
# visibility = ["//visibility:public"],
# )


def _minitable_target(t: TargetId) -> List[TargetId]:
return [t.get_target_id(f"{t.target_name}__minitable_library")]


def _upb_target(t: TargetId) -> List[TargetId]:
return [
t.get_target_id(f"{t.target_name}__upb_library"),
] + _minitable_target(t)


def _upbdefs_target(t: TargetId) -> List[TargetId]:
return [
t.get_target_id(f"{t.target_name}__upbdefs_library"),
] + _minitable_target(t)


_UPB_MINITABLE = PluginSettings(
name="upb_minitable",
plugin=UPB_REPO.parse_target(
Expand All @@ -72,6 +89,7 @@
"//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me"
),
],
aspectdeps=_minitable_target,
)

_UPB = PluginSettings(
Expand All @@ -83,6 +101,7 @@
"//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me"
),
],
aspectdeps=_upb_target,
)

# STAGE1 is used for bootstrapping upb via cmake.
Expand All @@ -95,6 +114,7 @@
"//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me"
),
],
aspectdeps=_upb_target,
)


Expand All @@ -108,36 +128,24 @@
),
UPB_REPO.parse_target("//upb:port"),
],
aspectdeps=_upbdefs_target,
)


def _minitable_target(t: TargetId) -> TargetId:
return t.get_target_id(f"{t.target_name}__minitable_library")


def _upb_target(t: TargetId) -> TargetId:
return t.get_target_id(f"{t.target_name}__upb_library")


def _upbdefs_target(t: TargetId) -> TargetId:
return t.get_target_id(f"{t.target_name}__upbdefs_library")


def upb_minitable_aspect(
context: InvocationContext,
proto_target: TargetId,
visibility: Optional[List[RelativeLabel]] = None,
**kwargs,
):
aspect_target = _minitable_target(proto_target)
aspect_target = _UPB_MINITABLE.aspectdeps(proto_target)[0]
context.add_rule(
aspect_target,
lambda: aspect_genproto_library_target(
context,
target=aspect_target,
proto_target=proto_target,
plugin_settings=_UPB_MINITABLE,
aspect_dependency=_minitable_target,
**kwargs,
),
visibility=visibility,
Expand All @@ -154,17 +162,14 @@ def upb_aspect(
if proto_target.repository_id == UPB_REPO:
plugin = _UPB_STAGE1

aspect_target = _upb_target(proto_target)
minitable_target = _minitable_target(proto_target)
aspect_target = plugin.aspectdeps(proto_target)[0]
context.add_rule(
aspect_target,
lambda: aspect_genproto_library_target(
context,
target=aspect_target,
proto_target=proto_target,
plugin_settings=plugin,
aspect_dependency=_upb_target,
extra_deps=[minitable_target],
**kwargs,
),
visibility=visibility,
Expand All @@ -177,17 +182,14 @@ def upbdefs_aspect(
visibility: Optional[List[RelativeLabel]] = None,
**kwargs,
):
aspect_target = _upbdefs_target(proto_target)
minitable_target = _minitable_target(proto_target)
aspect_target = _UPBDEFS.aspectdeps(proto_target)[0]
context.add_rule(
aspect_target,
lambda: aspect_genproto_library_target(
context,
target=aspect_target,
proto_target=proto_target,
plugin_settings=_UPBDEFS,
aspect_dependency=_upbdefs_target,
extra_deps=[minitable_target],
**kwargs,
),
visibility=visibility,
Expand Down Expand Up @@ -232,7 +234,7 @@ def bazel_upb_minitable_proto_library(
lambda: cc_proto_library_impl(
context,
target,
_aspect_target=_minitable_target,
_aspectdeps=_minitable_target,
_mnemonic="upb_minitable_proto_library",
**kwargs,
),
Expand Down Expand Up @@ -277,7 +279,7 @@ def bazel_upb_c_proto_library(
lambda: cc_proto_library_impl(
context,
target,
_aspect_target=_minitable_target,
_aspectdeps=_upb_target,
_mnemonic="upb_c_proto_library",
**kwargs,
),
Expand Down Expand Up @@ -306,7 +308,7 @@ def bazel_upb_proto_reflection_library(
lambda: cc_proto_library_impl(
context,
target,
_aspect_target=_upbdefs_target,
_aspectdeps=_upbdefs_target,
_mnemonic="upb_proto_reflection_library",
**kwargs,
),
Expand Down Expand Up @@ -358,7 +360,7 @@ def bazel_upb_c_proto_library(
lambda: cc_proto_library_impl(
context,
target,
_aspect_target=_upb_target,
_aspectdeps=_upb_target,
_mnemonic="upb_c_proto_library",
**kwargs,
),
Expand All @@ -378,7 +380,7 @@ def bazel_upb_proto_reflection_library(
lambda: cc_proto_library_impl(
context,
target,
_aspect_target=_upbdefs_target,
_aspectdeps=_upbdefs_target,
_mnemonic="upb_proto_reflection_library",
**kwargs,
),
Expand Down
8 changes: 5 additions & 3 deletions tools/cmake/bazel_to_cmake/cmake_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import hashlib
import pathlib
import re
from typing import Any, Dict, List, NamedTuple, Optional
from typing import Any, Dict, Iterable, List, NamedTuple, Optional

from .cmake_target import CMakePackage
from .cmake_target import CMakeTarget
Expand All @@ -27,7 +27,7 @@
from .starlark.bazel_target import RepositoryId
from .starlark.bazel_target import TargetId
from .util import make_relative_path
from .util import PathSequence
from .util import PathLike

_SPLIT_RE = re.compile("[:/]+")
_BIG = 35
Expand Down Expand Up @@ -92,7 +92,9 @@ def get_persisted_canonical_name(
assert target_id.repository_id == self.repository_id
return self.persisted_canonical_name.get(target_id, None)

def replace_with_cmake_macro_dirs(self, paths: PathSequence) -> List[str]:
def replace_with_cmake_macro_dirs(
self, paths: Iterable[PathLike]
) -> List[str]:
"""Substitute reposotory path prefixes with CMake PROJECT_{*}_DIR macros."""
result: list[str] = []
for x in paths:
Expand Down
22 changes: 11 additions & 11 deletions tools/cmake/bazel_to_cmake/cmake_target.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,20 @@ def __repr__(self):
return f"{self.__class__.__name__}({repr(self.packages)})"


class CMakeDepsProvider(Provider):
"""CMake deps corresponding to a Bazel target."""
class CMakeAddDependenciesProvider(Provider):
"""CMake add_dependencies required by a Bazel target."""

__slots__ = ("targets",)
__slots__ = ("target",)

def __init__(self, targets: List[CMakeTarget]):
self.targets = targets
def __init__(self, target: CMakeTarget):
self.target = target

def __repr__(self):
return f"{self.__class__.__name__}({repr(self.targets)})"
return f"{self.__class__.__name__}({repr(self.target)})"


class CMakeLibraryTargetProvider(Provider):
"""CMake target corresponding to a Bazel library target."""
class CMakeLinkLibrariesProvider(Provider):
"""CMake link_libraries required by a Bazel target."""

__slots__ = ("target",)

Expand All @@ -71,7 +71,7 @@ def __repr__(self):

AnyCMakeTargetProvider = TypeVar(
"AnyCMakeTargetProvider",
CMakeLibraryTargetProvider,
CMakeLinkLibrariesProvider,
CMakeExecutableTargetProvider,
)

Expand All @@ -94,11 +94,11 @@ def as_providers(
self,
provider: Optional[
Type[AnyCMakeTargetProvider]
] = CMakeLibraryTargetProvider,
] = CMakeLinkLibrariesProvider,
):
a = (provider(self.target),) if provider is not None else tuple()
return (
CMakeDepsProvider([self.dep]),
CMakeAddDependenciesProvider(self.dep),
CMakePackageDepsProvider([self.cmake_package]),
) + a

Expand Down
Loading

0 comments on commit 99573a2

Please sign in to comment.