Change cc_shared_library exports logic.

Changes in this CL:
1. 'exports' attribute renamed to roots
2. Removed restrictions for exporting a rule in a different repository
3. Kept restrictions for rules in the same repository but not in the same
package or subpackages
4. To get around restrictions, instead of the exported_by tag, one can use the
cc_shared_library_permissions rule
5. Added exports_filter that will match libraries in the transitive closure of
a root. Anything matched by the exports_filter will also be exported.
Restrictions as in 4. still apply.

RELNOTES:none
PiperOrigin-RevId: 310547916
Change-Id: I32d8e0d4dd4bcc9c9e92f4ff3c5b2a01564c31b2
This commit is contained in:
Googler 2020-05-08 06:35:32 -07:00 committed by Copybara-Service
parent dbe8807224
commit 8c31dd406c
12 changed files with 192 additions and 94 deletions

View File

@ -15,18 +15,14 @@ load("//cc:find_cc_toolchain.bzl", "find_cc_toolchain")
# used sparingly after making sure it's safe to use.
LINKABLE_MORE_THAN_ONCE = "LINKABLE_MORE_THAN_ONCE"
_EXPORTED_BY_TAG_BEGINNING = "exported_by="
def exported_by(labels):
str_builder = []
for label in labels:
str_builder.append(label)
return _EXPORTED_BY_TAG_BEGINNING + ",".join(str_builder)
CcSharedLibraryPermissionsInfo = provider(
fields = {
"targets": "Matches targets that can be exported.",
},
)
GraphNodeInfo = provider(
fields = {
"children": "Other GraphNodeInfo from dependencies of this target",
"exported_by": "Labels of targets that can export the library of this node",
"label": "Label of the target visited",
"linkable_more_than_once": "Linkable into more than a single cc_shared_library",
},
@ -161,6 +157,47 @@ def _check_if_target_under_path(value, pattern):
return pattern.package == value.package and pattern.name == value.name
def _check_if_target_can_be_exported(target, current_label, permissions):
if (target.workspace_name != current_label.workspace_name or
_same_package_or_above(current_label, target)):
return True
matched_by_target = False
for permission in permissions:
for permission_target in permission[CcSharedLibraryPermissionsInfo].targets:
if _check_if_target_under_path(target, permission_target):
return True
return False
def _check_if_target_should_be_exported_without_filter(target, current_label, permissions):
return _check_if_target_should_be_exported_with_filter(target, current_label, None, permissions)
def _check_if_target_should_be_exported_with_filter(target, current_label, exports_filter, permissions):
should_be_exported = False
if exports_filter == None:
should_be_exported = True
else:
for export_filter in exports_filter:
export_filter_label = current_label.relative(export_filter)
if _check_if_target_under_path(target, export_filter_label):
should_be_exported = True
break
if should_be_exported:
if _check_if_target_can_be_exported(target, current_label, permissions):
return True
else:
matched_by_filter_text = ""
if exports_filter:
matched_by_filter_text = " (matched by filter) "
fail(str(target) + matched_by_filter_text +
" cannot be exported from " + str(current_label) +
" because it's not in the same package/subpackage and the library " +
"doesn't have the necessary permissions. Use cc_shared_library_permissions.")
return False
def _filter_inputs(
ctx,
feature_configuration,
@ -174,7 +211,7 @@ def _filter_inputs(
graph_structure_aspect_nodes = []
dependency_linker_inputs = []
direct_exports = {}
for export in ctx.attr.exports:
for export in ctx.attr.roots:
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])
@ -191,6 +228,7 @@ def _filter_inputs(
preloaded_deps_direct_labels,
)
exports = {}
owners_seen = {}
for linker_input in dependency_linker_inputs:
owner = str(linker_input.owner)
@ -221,10 +259,19 @@ def _filter_inputs(
for static_dep_path in ctx.attr.static_deps:
static_dep_path_label = ctx.label.relative(static_dep_path)
owner_label = linker_input.owner
if _check_if_target_under_path(linker_input.owner, static_dep_path_label):
can_be_linked_statically = True
break
if _check_if_target_should_be_exported_with_filter(
linker_input.owner,
ctx.label,
ctx.attr.exports_filter,
ctx.attr.permissions,
):
exports[owner] = True
can_be_linked_statically = True
if can_be_linked_statically:
if not link_statically_labels[owner]:
link_once_static_libs.append(owner)
@ -233,7 +280,7 @@ def _filter_inputs(
fail("We can't link " +
str(owner) + " either statically or dynamically")
return (linker_inputs, link_once_static_libs)
return (exports, linker_inputs, link_once_static_libs)
def _same_package_or_above(label_a, label_b):
if label_a.workspace_name != label_b.workspace_name:
@ -262,23 +309,12 @@ 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.exports:
for export in ctx.attr.roots:
if str(export.label) in exports_map:
fail("Trying to export a library already exported by a different shared library: " +
str(export.label))
can_be_exported = _same_package_or_above(ctx.label, export.label)
if not can_be_exported:
for exported_by in export[GraphNodeInfo].exported_by:
exported_by_label = Label(exported_by)
if _check_if_target_under_path(ctx.label, exported_by_label):
can_be_exported = True
break
if not can_be_exported:
fail(str(export.label) + " cannot be exported from " + str(ctx.label) +
" because it's not in the same package/subpackage or the library " +
"to be exported doesn't have this cc_shared_library in the exported_by tag.")
_check_if_target_should_be_exported_without_filter(export.label, ctx.label, ctx.attr.permissions)
preloaded_deps_direct_labels = {}
preloaded_dep_merged_cc_info = None
@ -292,7 +328,7 @@ def _cc_shared_library_impl(ctx):
link_once_static_libs_map = _build_link_once_static_libs_map(merged_cc_shared_library_info)
(linker_inputs, link_once_static_libs) = _filter_inputs(
(exports, linker_inputs, link_once_static_libs) = _filter_inputs(
ctx,
feature_configuration,
cc_toolchain,
@ -324,9 +360,8 @@ def _cc_shared_library_impl(ctx):
for dep in ctx.attr.dynamic_deps:
runfiles = runfiles.merge(dep[DefaultInfo].data_runfiles)
exports = []
for export in ctx.attr.exports:
exports.append(str(export.label))
for export in ctx.attr.roots:
exports[str(export.label)] = True
return [
DefaultInfo(
@ -335,7 +370,7 @@ def _cc_shared_library_impl(ctx):
),
CcSharedLibraryInfo(
dynamic_deps = merged_cc_shared_library_info,
exports = exports,
exports = exports.keys(),
link_once_static_libs = link_once_static_libs,
linker_input = cc_common.create_linker_input(
owner = ctx.label,
@ -353,42 +388,54 @@ def _graph_structure_aspect_impl(target, ctx):
if GraphNodeInfo in dep:
children.append(dep[GraphNodeInfo])
exported_by = []
# TODO(bazel-team): Add flag to Bazel that can toggle the initialization of
# linkable_more_than_once.
linkable_more_than_once = False
if hasattr(ctx.rule.attr, "tags"):
for tag in ctx.rule.attr.tags:
if tag.startswith(_EXPORTED_BY_TAG_BEGINNING) and len(tag) > len(_EXPORTED_BY_TAG_BEGINNING):
for target in tag[len(_EXPORTED_BY_TAG_BEGINNING):].split(","):
# Only absolute labels allowed. Targets in same package
# or subpackage can be exported anyway.
if not target.startswith("//") and not target.startswith("@"):
fail("Labels in exported_by of " + str(target) +
" must be absolute.")
Label(target) # Checking synthax is ok.
exported_by.append(target)
elif tag == LINKABLE_MORE_THAN_ONCE:
if tag == LINKABLE_MORE_THAN_ONCE:
linkable_more_than_once = True
return [GraphNodeInfo(
label = ctx.label,
children = children,
exported_by = exported_by,
linkable_more_than_once = linkable_more_than_once,
)]
def _cc_shared_library_permissions_impl(ctx):
targets = []
for target_filter in ctx.attr.targets:
target_filter_label = ctx.label.relative(target_filter)
if not _check_if_target_under_path(target_filter_label, ctx.label.relative(":__subpackages__")):
fail("A cc_shared_library_permissions rule can only list " +
"targets that are in the same package or a sub-package")
targets.append(target_filter_label)
return [CcSharedLibraryPermissionsInfo(
targets = targets,
)]
graph_structure_aspect = aspect(
attr_aspects = ["*"],
implementation = _graph_structure_aspect_impl,
)
cc_shared_library_permissions = rule(
implementation = _cc_shared_library_permissions_impl,
attrs = {
"targets": attr.string_list(),
},
)
cc_shared_library = rule(
implementation = _cc_shared_library_impl,
attrs = {
"additional_linker_inputs": attr.label_list(allow_files = True),
"dynamic_deps": attr.label_list(providers = [CcSharedLibraryInfo]),
"exports": attr.label_list(providers = [CcInfo], aspects = [graph_structure_aspect]),
"exports_filter": attr.string_list(),
"permissions": attr.label_list(providers = [CcSharedLibraryPermissionsInfo]),
"preloaded_deps": attr.label_list(providers = [CcInfo]),
"roots": attr.label_list(providers = [CcInfo], aspects = [graph_structure_aspect]),
"static_deps": attr.string_list(),
"user_link_flags": attr.string_list(),
"_cc_toolchain": attr.label(default = "@bazel_tools//tools/cpp:current_cc_toolchain"),

View File

@ -1,6 +1,6 @@
load("//cc:defs.bzl", "cc_binary", "cc_library", "cc_test")
load("//examples:experimental_cc_shared_library.bzl", "LINKABLE_MORE_THAN_ONCE", "cc_shared_library")
load(":starlark_tests.bzl", "additional_inputs_test", "link_once_repeated_test", "linking_suffix_test", "paths_test")
load(":starlark_tests.bzl", "additional_inputs_test", "build_failure_test", "linking_suffix_test", "paths_test")
package(
default_visibility = ["//examples/test_cc_shared_library:__subpackages__"],
@ -28,6 +28,11 @@ cc_shared_library(
],
dynamic_deps = ["bar_so"],
preloaded_deps = ["preloaded_dep"],
roots = [
"baz",
"foo",
"a_suffix",
],
static_deps = [
"//examples/test_cc_shared_library:qux",
"//examples/test_cc_shared_library:qux2",
@ -37,24 +42,6 @@ cc_shared_library(
"-Wl,--version-script=$(location :foo.lds)",
"-Wl,--script=$(location :additional_script.txt)",
],
exports = [
"foo",
# Not a problem to export "baz" which is depended on by foo
"baz",
# Case1
# This is fine. bar3 can be exported by "foo_so"
# even though bar3 is linked statically by bar_so, since bar_so
# doesn't export bar3 this is fine.
"bar3",
# Case2
# This is fine. foo depends on bar4. Even though bar is
# linked dynamically, when bar is pruned, we still have a dependency
# edge to bar4 from foo. Also bar_so doesn't export bar4.
#
# Note that even though the target claims to export bar4. Unless the version script is
# changed, the symbols from bar4 won't be exported.
"bar4",
],
)
cc_library(
@ -77,10 +64,17 @@ cc_library(
],
)
cc_library(
name = "a_suffix",
srcs = ["a_suffix.cc"],
hdrs = ["a_suffix.h"],
)
cc_library(
name = "baz",
srcs = ["baz.cc"],
hdrs = ["baz.h"],
deps = ["bar3"],
)
cc_library(
@ -101,6 +95,18 @@ cc_shared_library(
additional_linker_inputs = [
":bar.lds",
],
exports_filter = [
"bar3", # Exported transitive dependency
"//examples/test_cc_shared_library3:bar",
],
permissions = [
"//examples/test_cc_shared_library3:permissions",
],
roots = [
"bar",
"bar2",
"@test_repo//:bar",
],
static_deps = [
"//examples/test_cc_shared_library:barX",
"@test_repo//:bar",
@ -109,11 +115,6 @@ cc_shared_library(
user_link_flags = [
"-Wl,--version-script=$(location :bar.lds)",
],
exports = [
"bar",
"bar2",
"@test_repo//:bar",
],
)
cc_library(
@ -139,18 +140,16 @@ cc_library(
name = "bar2",
srcs = ["bar2.cc"],
hdrs = ["bar2.h"],
deps = ["bar3"],
)
cc_library(
name = "bar3",
srcs = ["bar3.cc"],
hdrs = ["bar3.h"],
)
cc_library(
name = "bar4",
srcs = ["bar4.cc"],
hdrs = ["bar4.h"],
deps = [
"//examples/test_cc_shared_library3:bar",
],
)
sh_test(
@ -174,11 +173,25 @@ additional_inputs_test(
target_under_test = ":foo_so",
)
link_once_repeated_test(
build_failure_test(
name = "link_once_repeated_test",
message = "already linked statically in " +
"//examples/test_cc_shared_library:foo_so but not exported.",
target_under_test = "//examples/test_cc_shared_library/failing_targets:should_fail_binary",
)
paths_test(
name = "path_matching_test",
)
build_failure_test(
name = "export_without_permissions_test",
message = "doesn't have the necessary permissions",
target_under_test = "//examples/test_cc_shared_library/failing_targets:permissions_fail_so",
)
build_failure_test(
name = "forbidden_target_permissions_test",
message = "can only list targets that are in the same package or a sub-package",
target_under_test = "//examples/test_cc_shared_library/failing_targets:permissions_fail",
)

View File

@ -0,0 +1,3 @@
#include "examples/test_cc_shared_library/a_suffix.h"
int a_suffix() { return 42; }

View File

@ -0,0 +1,6 @@
#ifndef EXAMPLES_TEST_CC_SHARED_LIBRARY_A_SUFFIX_H_
#define EXAMPLES_TEST_CC_SHARED_LIBRARY_A_SUFFIX_H_
int a_suffix();
#endif // EXAMPLES_TEST_CC_SHARED_LIBRARY_A_SUFFIX_H_

View File

@ -1,3 +0,0 @@
#include "examples/test_cc_shared_library/bar4.h"
int bar4() { return 42; }

View File

@ -1,6 +0,0 @@
#ifndef EXAMPLES_TEST_CC_SHARED_LIBRARY_BAR_4_H_
#define EXAMPLES_TEST_CC_SHARED_LIBRARY_BAR_4_H_
int bar4();
#endif // EXAMPLES_TEST_CC_SHARED_LIBRARY_BAR_4_H_

View File

@ -1,18 +1,37 @@
load("//cc:defs.bzl", "cc_binary")
load("//examples:experimental_cc_shared_library.bzl", "cc_shared_library", "cc_shared_library_permissions")
package(
default_visibility = ["//examples/test_cc_shared_library:__pkg__"],
)
TAGS = [
"manual",
"nobuilder",
]
cc_binary(
name = "should_fail_binary",
dynamic_deps = ["//examples/test_cc_shared_library:foo_so"],
tags = [
"manual",
"nobuilder",
],
tags = TAGS,
deps = [
"//examples/test_cc_shared_library:foo",
"//examples/test_cc_shared_library:qux",
],
)
cc_shared_library(
name = "permissions_fail_so",
roots = [
"//examples/test_cc_shared_library3:bar",
],
tags = TAGS,
)
cc_shared_library_permissions(
name = "permissions_fail",
tags = TAGS,
targets = [
"//examples/test_cc_shared_library:foo",
],
)

View File

@ -11,7 +11,7 @@ def _linking_suffix_test_impl(ctx):
for arg in reversed(actions[1].argv):
if arg.find(".a") != -1 or arg.find("-l") != -1:
asserts.equals(env, "libbar4.a", arg[arg.rindex("/") + 1:])
asserts.equals(env, "liba_suffix.a", arg[arg.rindex("/") + 1:])
break
return analysistest.end(env)
@ -36,14 +36,18 @@ def _additional_inputs_test_impl(ctx):
additional_inputs_test = analysistest.make(_additional_inputs_test_impl)
def _link_once_repeated_test_impl(ctx):
def _build_failure_test_impl(ctx):
env = analysistest.begin(ctx)
asserts.expect_failure(env, "already linked statically")
asserts.expect_failure(env, ctx.attr.message)
return analysistest.end(env)
link_once_repeated_test = analysistest.make(_link_once_repeated_test_impl, expect_failure = True)
build_failure_test = analysistest.make(
_build_failure_test_impl,
expect_failure = True,
attrs = {"message": attr.string()},
)
def _paths_test_impl(ctx):
env = unittest.begin(ctx)

View File

@ -1,10 +1,8 @@
load("@rules_cc//cc:defs.bzl", "cc_library")
load("@rules_cc//examples:experimental_cc_shared_library.bzl", "exported_by")
cc_library(
name = "bar",
srcs = ["bar.cc"],
hdrs = ["bar.h"],
tags = [exported_by(["@rules_cc//examples/test_cc_shared_library:bar_so"])],
visibility = ["//visibility:public"],
)

View File

@ -0,0 +1,17 @@
load("@rules_cc//cc:defs.bzl", "cc_library")
load("//examples:experimental_cc_shared_library.bzl", "cc_shared_library_permissions")
cc_library(
name = "bar",
srcs = ["bar.cc"],
hdrs = ["bar.h"],
visibility = ["//visibility:public"],
)
cc_shared_library_permissions(
name = "permissions",
targets = [
"//examples/test_cc_shared_library3:bar",
],
visibility = ["//examples/test_cc_shared_library:__pkg__"],
)

View File

View File