Skip to content

Commit

Permalink
Allow cc_shared_library aspect to propagate along all attributes
Browse files Browse the repository at this point in the history
This is rolling forward ff927f7 which I mistakenly confused as the root cause for #16296. The implementation does have an issue with too much propagation which is fixed on this change with required_providers and required_aspect_providers restrictions on the aspect.

Fixes #17091.

RELNOTES:none
PiperOrigin-RevId: 507457624
Change-Id: I7317c4532ef20cd7584c55aacc3eca82c50a618b
  • Loading branch information
oquenchil authored and Copybara-Service committed Feb 6, 2023
1 parent 3df262c commit 590ee17
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 7 deletions.
1 change: 1 addition & 0 deletions src/main/starlark/builtins_bzl/common/cc/cc_import.bzl
Expand Up @@ -220,6 +220,7 @@ cc_import = rule(
),
"_cc_toolchain": attr.label(default = "@" + semantics.get_repo() + "//tools/cpp:current_cc_toolchain"),
},
provides = [CcInfo],
toolchains = cc_helper.use_cpp_toolchain(),
fragments = ["cpp"],
incompatible_use_toolchain_transition = True,
Expand Down
Expand Up @@ -294,4 +294,5 @@ cc_proto_library = rule(
allow_files = False,
),
},
provides = [CcInfo],
)
Expand Up @@ -21,6 +21,7 @@ rely on this. It requires bazel >1.2 and passing the flag

load(":common/cc/cc_helper.bzl", "cc_helper")
load(":common/cc/semantics.bzl", "semantics")
load(":common/proto/proto_info.bzl", "ProtoInfo")

CcInfo = _builtins.toplevel.CcInfo
cc_common = _builtins.toplevel.cc_common
Expand Down Expand Up @@ -620,11 +621,17 @@ def _cc_shared_library_impl(ctx):
def _graph_structure_aspect_impl(target, ctx):
children = []

# For now ignore cases when deps is of type label instead of label_list.
if hasattr(ctx.rule.attr, "deps") and type(ctx.rule.attr.deps) != "Target":
for dep in ctx.rule.attr.deps:
if GraphNodeInfo in dep:
children.append(dep[GraphNodeInfo])
# Collect graph structure info from any possible deplike attribute. The aspect
# itself applies across every deplike attribute (attr_aspects is *), so enumerate
# over all attributes and consume GraphNodeInfo if available.
for fieldname in dir(ctx.rule.attr):
deps = getattr(ctx.rule.attr, fieldname, None)
if type(deps) == "list":
for dep in deps:
if type(dep) == "Target" and GraphNodeInfo in dep:
children.append(dep[GraphNodeInfo])
elif type(deps) == "Target" and GraphNodeInfo in deps:
children.append(deps[GraphNodeInfo])

# TODO(bazel-team): Add flag to Bazel that can toggle the initialization of
# linkable_more_than_once.
Expand Down Expand Up @@ -659,6 +666,8 @@ def _cc_shared_library_permissions_impl(ctx):

graph_structure_aspect = aspect(
attr_aspects = ["*"],
required_providers = [[CcInfo], [ProtoInfo]],
required_aspect_providers = [[CcInfo]],
implementation = _graph_structure_aspect_impl,
)

Expand Down
Expand Up @@ -36,9 +36,15 @@ cc_binary(
name = "binary",
srcs = ["main.cc"],
dynamic_deps = ["foo_so"],
deps = ["foo"],
deps = [
":foo",
],
)

# TODO(bazel-team): Add a test for proto dependencies once these tests are run
# directly from a BUILD file and not from within a shell test. Right now
# mocking what's needed to have a single proto dependency makes it impractical.

cc_binary(
name = "binary_with_bar_so_twice",
srcs = ["main.cc"],
Expand Down Expand Up @@ -331,8 +337,8 @@ build_failure_test(

cc_library(
name = "prebuilt",
hdrs = ["direct_so_file_cc_lib.h"],
srcs = [
"direct_so_file_cc_lib.h",
":just_main_output",
],
)
Expand Down

0 comments on commit 590ee17

Please sign in to comment.