Skip to content

Commit adfd4df

Browse files
oquenchilcopybara-github
authored andcommitted
Remove cc_shared_library_permissions
This mechanism came out of the design doc for cc_shared_library but people aren't using it. The idea was to prevent a cc_library target to be exported by cc_shared_libraries that aren't authorized. In reality though this cannot work on two accounts: 1. The mechanism would only prevent a cc_library from being "claimed to be exported" by the cc_shared_library, however a custom Starlark rule would easily bypass this. 2. The rule cannot control the version script the user passes to the linker so the symbols could in practice still be exported. In general, this mechanism provides a false sense of security, no one is using it and currently the only thing it does is to slightly complicate the codebase and the documentation. In any case adding this functionality later if needed would be a compatible change. RELNOTES:none PiperOrigin-RevId: 510425434 Change-Id: Icf85050b57b1d8dae0d32614dc5951d982b3b3d0
1 parent 3b7e233 commit adfd4df

File tree

7 files changed

+6
-162
lines changed

7 files changed

+6
-162
lines changed

src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@
9999
import com.google.devtools.build.lib.rules.android.databinding.DataBindingV2Provider;
100100
import com.google.devtools.build.lib.rules.config.ConfigRules;
101101
import com.google.devtools.build.lib.rules.core.CoreRules;
102-
import com.google.devtools.build.lib.rules.cpp.CcSharedLibraryPermissionsRule;
103102
import com.google.devtools.build.lib.rules.cpp.CcSharedLibraryRule;
104103
import com.google.devtools.build.lib.rules.cpp.CcStarlarkInternal;
105104
import com.google.devtools.build.lib.rules.cpp.GoogleLegacyStubs;
@@ -507,7 +506,6 @@ public void init(ConfiguredRuleClassProvider.Builder builder) {
507506
builder.addRuleDefinition(new AndroidNdkRepositoryRule());
508507
builder.addRuleDefinition(new LocalConfigPlatformRule());
509508
builder.addRuleDefinition(new CcSharedLibraryRule());
510-
builder.addRuleDefinition(new CcSharedLibraryPermissionsRule());
511509

512510
try {
513511
builder.addWorkspaceFileSuffix(

src/main/java/com/google/devtools/build/lib/rules/cpp/CcSharedLibraryPermissionsRule.java

Lines changed: 0 additions & 44 deletions
This file was deleted.

src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl

Lines changed: 5 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,6 @@ LINKABLE_MORE_THAN_ONCE = "LINKABLE_MORE_THAN_ONCE"
4343
# be dropped by the linker since there are no incoming symbol references.
4444
NO_EXPORTING = "NO_EXPORTING"
4545

46-
CcSharedLibraryPermissionsInfo = provider(
47-
"Permissions for a cc shared library.",
48-
fields = {
49-
"targets": "Matches targets that can be exported.",
50-
},
51-
)
5246
GraphNodeInfo = provider(
5347
"Nodes in the graph of shared libraries.",
5448
fields = {
@@ -195,47 +189,18 @@ def _check_if_target_under_path(value, pattern):
195189

196190
return pattern.package == value.package and pattern.name == value.name
197191

198-
def _check_if_target_can_be_exported(target, current_label, permissions):
199-
if permissions == None:
200-
return True
201-
202-
if (target.workspace_name != current_label.workspace_name or
203-
_same_package_or_above(current_label, target)):
204-
return True
205-
206-
matched_by_target = False
207-
for permission in permissions:
208-
for permission_target in permission[CcSharedLibraryPermissionsInfo].targets:
209-
if _check_if_target_under_path(target, permission_target):
210-
return True
211-
212-
return False
213-
214-
def _check_if_target_should_be_exported_without_filter(target, current_label, permissions):
215-
return _check_if_target_should_be_exported_with_filter(target, current_label, None, permissions)
192+
def _check_if_target_should_be_exported_without_filter(target, current_label):
193+
return _check_if_target_should_be_exported_with_filter(target, current_label, None)
216194

217-
def _check_if_target_should_be_exported_with_filter(target, current_label, exports_filter, permissions):
195+
def _check_if_target_should_be_exported_with_filter(target, current_label, exports_filter):
218196
should_be_exported = False
219197
if exports_filter == None:
220198
should_be_exported = True
221199
else:
222200
for export_filter in exports_filter:
223201
export_filter_label = current_label.relative(export_filter)
224202
if _check_if_target_under_path(target, export_filter_label):
225-
should_be_exported = True
226-
break
227-
228-
if should_be_exported:
229-
if _check_if_target_can_be_exported(target, current_label, permissions):
230-
return True
231-
else:
232-
matched_by_filter_text = ""
233-
if exports_filter:
234-
matched_by_filter_text = " (matched by filter) "
235-
fail(str(target) + matched_by_filter_text +
236-
" cannot be exported from " + str(current_label) +
237-
" because it's not in the same package/subpackage and the library " +
238-
"doesn't have the necessary permissions. Use cc_shared_library_permissions.")
203+
return True
239204

240205
return False
241206

@@ -337,7 +302,6 @@ def _filter_inputs(
337302
linker_input.owner,
338303
ctx.label,
339304
ctx.attr.exports_filter,
340-
_get_permissions(ctx),
341305
):
342306
exports[owner] = True
343307

@@ -389,11 +353,6 @@ def _same_package_or_above(label_a, label_b):
389353

390354
return True
391355

392-
def _get_permissions(ctx):
393-
if ctx.fragments.cpp.experimental_enable_target_export_check():
394-
return ctx.attr.permissions
395-
return None
396-
397356
def _get_deps(ctx):
398357
if len(ctx.attr.deps) and len(ctx.attr.roots):
399358
fail(
@@ -460,7 +419,7 @@ def _cc_shared_library_impl(ctx):
460419
fail("Trying to export a library already exported by a different shared library: " +
461420
str(export.label))
462421

463-
_check_if_target_should_be_exported_without_filter(export.label, ctx.label, _get_permissions(ctx))
422+
_check_if_target_should_be_exported_without_filter(export.label, ctx.label)
464423

465424
link_once_static_libs_map = _build_link_once_static_libs_map(merged_cc_shared_library_info)
466425

@@ -634,41 +593,20 @@ def _graph_structure_aspect_impl(target, ctx):
634593
no_exporting = no_exporting,
635594
)]
636595

637-
def _cc_shared_library_permissions_impl(ctx):
638-
targets = []
639-
for target_filter in ctx.attr.targets:
640-
target_filter_label = ctx.label.relative(target_filter)
641-
if not _check_if_target_under_path(target_filter_label, ctx.label.relative(":__subpackages__")):
642-
fail("A cc_shared_library_permissions rule can only list " +
643-
"targets that are in the same package or a sub-package")
644-
targets.append(target_filter_label)
645-
646-
return [CcSharedLibraryPermissionsInfo(
647-
targets = targets,
648-
)]
649-
650596
graph_structure_aspect = aspect(
651597
attr_aspects = ["*"],
652598
required_providers = [[CcInfo], [ProtoInfo]],
653599
required_aspect_providers = [[CcInfo]],
654600
implementation = _graph_structure_aspect_impl,
655601
)
656602

657-
cc_shared_library_permissions = rule(
658-
implementation = _cc_shared_library_permissions_impl,
659-
attrs = {
660-
"targets": attr.string_list(),
661-
},
662-
)
663-
664603
cc_shared_library = rule(
665604
implementation = _cc_shared_library_impl,
666605
attrs = {
667606
"additional_linker_inputs": attr.label_list(allow_files = True),
668607
"shared_lib_name": attr.string(),
669608
"dynamic_deps": attr.label_list(providers = [CcSharedLibraryInfo]),
670609
"exports_filter": attr.string_list(),
671-
"permissions": attr.label_list(providers = [CcSharedLibraryPermissionsInfo]),
672610
"win_def_file": attr.label(allow_single_file = [".def"]),
673611
"roots": attr.label_list(providers = [CcInfo], aspects = [graph_structure_aspect]),
674612
"deps": attr.label_list(providers = [CcInfo], aspects = [graph_structure_aspect]),

src/main/starlark/builtins_bzl/common/exports.bzl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
load("@_builtins//:common/cc/cc_import.bzl", "cc_import")
1818
load("@_builtins//:common/cc/cc_binary_wrapper.bzl", "cc_binary")
1919
load("@_builtins//:common/cc/cc_test_wrapper.bzl", cc_test = "cc_test_wrapper")
20-
load("@_builtins//:common/cc/experimental_cc_shared_library.bzl", "CcSharedLibraryInfo", "cc_shared_library", "cc_shared_library_permissions")
20+
load("@_builtins//:common/cc/experimental_cc_shared_library.bzl", "CcSharedLibraryInfo", "cc_shared_library")
2121
load("@_builtins//:common/objc/objc_import.bzl", "objc_import")
2222
load("@_builtins//:common/objc/objc_library.bzl", "objc_library")
2323
load("@_builtins//:common/objc/compilation_support.bzl", "compilation_support")
@@ -58,7 +58,6 @@ exported_rules = {
5858
"objc_library": objc_library,
5959
"proto_library": proto_library,
6060
"+cc_shared_library": cc_shared_library,
61-
"+cc_shared_library_permissions": cc_shared_library_permissions,
6261
"+cc_binary": cc_binary,
6362
"+cc_test": cc_test,
6463
"+cc_library": cc_library,

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,6 @@ cc_shared_library(
194194
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:bar",
195195
],
196196
features = ["windows_export_all_symbols"],
197-
permissions = [
198-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:permissions",
199-
],
200197
deps = [
201198
"bar",
202199
"bar2",
@@ -319,18 +316,6 @@ paths_test(
319316
name = "path_matching_test",
320317
)
321318

322-
build_failure_test(
323-
name = "export_without_permissions_test",
324-
message = "doesn't have the necessary permissions",
325-
target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:permissions_fail_so",
326-
)
327-
328-
build_failure_test(
329-
name = "forbidden_target_permissions_test",
330-
message = "can only list targets that are in the same package or a sub-package",
331-
target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:permissions_fail",
332-
)
333-
334319
cc_library(
335320
name = "prebuilt",
336321
hdrs = ["direct_so_file_cc_lib.h"],
@@ -391,15 +376,6 @@ cc_library(
391376
],
392377
)
393378

394-
cc_shared_library_permissions(
395-
name = "permissions",
396-
targets = [
397-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:a_suffix",
398-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux",
399-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux2",
400-
],
401-
)
402-
403379
cc_library(
404380
name = "static_lib_no_exporting",
405381
srcs = [

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,6 @@ cc_library(
3232
],
3333
)
3434

35-
cc_shared_library(
36-
name = "permissions_fail_so",
37-
deps = [
38-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:bar",
39-
],
40-
tags = TAGS,
41-
)
42-
43-
cc_shared_library_permissions(
44-
name = "permissions_fail",
45-
tags = TAGS,
46-
targets = [
47-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:foo",
48-
],
49-
)
50-
5135
cc_library(
5236
name = "a",
5337
srcs = ["a.cc"],

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/BUILD.builtin_test

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,3 @@ cc_shared_library(
2121
":diff_pkg",
2222
],
2323
)
24-
25-
cc_shared_library_permissions(
26-
name = "permissions",
27-
targets = [
28-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:bar",
29-
],
30-
)

0 commit comments

Comments
 (0)