Skip to content

Commit

Permalink
Rename roots in cc_shared_library to deps
Browse files Browse the repository at this point in the history
The attribute 'roots' was initially called 'exports', however this generated
confusion as it did not behave like 'exports' in other rules nor does the
cc_shared_library rule guarantee that the symbols from targets in that
attribute would be exported.

From 'exports' it was renamed 'roots' to express that these are the targets at
the top of the tree whose transitive closure would be linked into this shared
library (dynamically or statically). However, 'roots' also departs from the
standard attribute name used across all rules, 'deps'. 'roots' does not even
convey that the attribute accepts other targets.

Now that the attribute 'static_deps' has been removed, 'deps' again is the best
attribute name we could have. It adds consistency with other rules and makes sure
that the attribute assumed by tooling by default to contain the main
dependencies is the same.

With the --experimental_cc_shared_library flag, the attribute 'deps' will still
work. Without the experimental flag, 'deps' must be used. Eventually, we will
remove the experimental flag, therefore all targets should rename the
attribute.

RELNOTES[inc]: The attribute cc_shared_library.roots is renamed to 'deps'

PiperOrigin-RevId: 505705317
Change-Id: I69a25f6a835c87626aeeef6c9c3def5c148a31f7
  • Loading branch information
oquenchil authored and copybara-github committed Jan 30, 2023
1 parent 6fe6a95 commit 68aad18
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ def _filter_inputs(
ctx,
feature_configuration,
cc_toolchain,
deps,
transitive_exports,
preloaded_deps_direct_labels,
link_once_static_libs_map):
Expand All @@ -260,7 +261,7 @@ def _filter_inputs(
graph_structure_aspect_nodes = []
dependency_linker_inputs = []
direct_exports = {}
for export in ctx.attr.roots:
for export in deps:
direct_exports[str(export.label)] = True
dependency_linker_inputs.extend(export[CcInfo].linking_context.linker_inputs.to_list())
graph_structure_aspect_nodes.append(export[GraphNodeInfo])
Expand Down Expand Up @@ -403,16 +404,40 @@ def _get_permissions(ctx):
return ctx.attr.permissions
return None

def _get_deps(ctx):
if len(ctx.attr.deps) and len(ctx.attr.roots):
fail(
"You are using the attribute 'roots' and 'deps'. 'deps' is the " +
"new name for the attribute 'roots'. The attribute 'roots' will be" +
"removed in the future",
attr = "roots",
)

deps = ctx.attr.deps
if not len(deps):
deps = ctx.attr.roots

return deps

def _cc_shared_library_impl(ctx):
semantics.check_experimental_cc_shared_library(ctx)

if len(ctx.attr.static_deps) and not cc_common.check_experimental_cc_shared_library():
fail(
"This attribute is a no-op and its usage" +
" is forbidden after cc_shared_library is no longer experimental. " +
"Remove it from every cc_shared_library target",
attr = "static_deps",
)
if not cc_common.check_experimental_cc_shared_library():
if len(ctx.attr.static_deps):
fail(
"This attribute is a no-op and its usage" +
" is forbidden after cc_shared_library is no longer experimental. " +
"Remove it from every cc_shared_library target",
attr = "static_deps",
)
if len(ctx.attr.roots):
fail(
"This attribute has been renamed to 'deps'. Simply rename the" +
" attribute on the target.",
attr = "roots",
)

deps = _get_deps(ctx)

cc_toolchain = cc_helper.find_cpp_toolchain(ctx)
feature_configuration = cc_common.configure_features(
Expand All @@ -424,7 +449,7 @@ def _cc_shared_library_impl(ctx):

merged_cc_shared_library_info = _merge_cc_shared_library_infos(ctx)
exports_map = _build_exports_map_from_only_dynamic_deps(merged_cc_shared_library_info)
for export in ctx.attr.roots:
for export in deps:
# Do not check for overlap between targets matched by the current
# rule's exports_filter and what is in exports_map. A library in roots
# will have to be linked in statically into the current rule with 100%
Expand Down Expand Up @@ -457,6 +482,7 @@ def _cc_shared_library_impl(ctx):
ctx,
feature_configuration,
cc_toolchain,
deps,
exports_map,
preloaded_deps_direct_labels,
link_once_static_libs_map,
Expand Down Expand Up @@ -534,7 +560,7 @@ def _cc_shared_library_impl(ctx):

runfiles = runfiles.merge(ctx.runfiles(files = precompiled_only_dynamic_libraries_runfiles))

for export in ctx.attr.roots:
for export in deps:
export_label = str(export.label)
if GraphNodeInfo in export and export[GraphNodeInfo].no_exporting:
if export_label in curr_link_once_static_libs_set:
Expand Down Expand Up @@ -654,6 +680,7 @@ cc_shared_library = rule(
"preloaded_deps": attr.label_list(providers = [CcInfo]),
"win_def_file": attr.label(allow_single_file = [".def"]),
"roots": attr.label_list(providers = [CcInfo], aspects = [graph_structure_aspect]),
"deps": attr.label_list(providers = [CcInfo], aspects = [graph_structure_aspect]),
"static_deps": attr.string_list(),
"user_link_flags": attr.string_list(),
"_def_parser": semantics.get_def_parser(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,28 +52,28 @@ cc_binary(
cc_shared_library(
name = "python_module",
features = ["windows_export_all_symbols"],
roots = [":a_suffix"],
deps = [":a_suffix"],
shared_lib_name = "python_module.pyd",
)

cc_shared_library(
name = "a_so",
features = ["windows_export_all_symbols"],
roots = [":a_suffix"],
deps = [":a_suffix"],
)

cc_shared_library(
name = "diamond_so",
dynamic_deps = [":a_so"],
features = ["windows_export_all_symbols"],
roots = [":qux"],
deps = [":qux"],
)

cc_shared_library(
name = "diamond2_so",
dynamic_deps = [":a_so"],
features = ["windows_export_all_symbols"],
roots = [":qux2"],
deps = [":qux2"],
)

cc_binary(
Expand Down Expand Up @@ -101,7 +101,7 @@ cc_shared_library(
dynamic_deps = ["bar_so"],
features = ["windows_export_all_symbols"],
preloaded_deps = ["preloaded_dep"],
roots = [
deps = [
"baz",
"foo",
"a_suffix",
Expand Down Expand Up @@ -195,7 +195,7 @@ cc_shared_library(
permissions = [
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:permissions",
],
roots = [
deps = [
"bar",
"bar2",
] + select({
Expand Down Expand Up @@ -346,7 +346,7 @@ filegroup(
cc_shared_library(
name = "direct_so_file",
features = ["windows_export_all_symbols"],
roots = [
deps = [
":direct_so_file_cc_lib",
],
)
Expand All @@ -367,7 +367,7 @@ filegroup(
cc_shared_library(
name = "renamed_so_file",
features = ["windows_export_all_symbols"],
roots = [
deps = [
":direct_so_file_cc_lib2",
],
shared_lib_name = "renamed_so_file.so",
Expand Down Expand Up @@ -417,23 +417,23 @@ cc_library(

cc_shared_library(
name = "lib_with_no_exporting_roots_1",
roots = [":static_lib_no_exporting"],
deps = [":static_lib_no_exporting"],
)

cc_shared_library(
name = "lib_with_no_exporting_roots_2",
roots = [":static_lib_no_exporting"],
deps = [":static_lib_no_exporting"],
dynamic_deps = [":lib_with_no_exporting_roots_3"],
)

cc_shared_library(
name = "lib_with_no_exporting_roots_3",
roots = [":static_lib_no_exporting"],
deps = [":static_lib_no_exporting"],
)

cc_shared_library(
name = "lib_with_no_exporting_roots",
roots = [
deps = [
":static_lib_no_exporting",
":static_lib_exporting",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ cc_binary(
cc_shared_library(
name = "should_fail_shared_lib",
dynamic_deps = ["//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:bar_so"],
roots = [
deps = [
":intermediate",
],
tags = TAGS,
Expand All @@ -34,7 +34,7 @@ cc_library(

cc_shared_library(
name = "permissions_fail_so",
roots = [
deps = [
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:bar",
],
tags = TAGS,
Expand Down Expand Up @@ -69,7 +69,7 @@ cc_shared_library(
":b_so",
":b2_so",
],
roots = [
deps = [
":a",
],
tags = TAGS,
Expand All @@ -87,15 +87,15 @@ cc_binary(

cc_shared_library(
name = "b_so",
roots = [
deps = [
":b",
],
tags = TAGS,
)

cc_shared_library(
name = "b2_so",
roots = [
deps = [
":b",
],
tags = TAGS,
Expand Down

0 comments on commit 68aad18

Please sign in to comment.