Skip to content

Commit

Permalink
Automated rollback of commit f7946d0.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

17.k breakages because of too many combinations
[]

*** Original change description ***

Propagate graph_node_aspect over a computed default attribute

Set computed default to deps only when we need to propagate graph_node aspect.

This change makes it possible to collapse cc_binary and cc_test into a single rule.

PiperOrigin-RevId: 574422599
Change-Id: I5245d28fd6778591410d8027a6edfe742c764426
  • Loading branch information
comius authored and copybara-github committed Oct 18, 2023
1 parent 98e1b30 commit c59739e
Show file tree
Hide file tree
Showing 13 changed files with 257 additions and 98 deletions.
41 changes: 21 additions & 20 deletions src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,11 @@

"""cc_binary Starlark implementation replacing native"""

load(":common/cc/cc_binary_attrs.bzl", "cc_binary_attrs")
load(":common/cc/cc_common.bzl", "cc_common")
load(":common/cc/semantics.bzl", "semantics")
load(":common/cc/cc_shared_library.bzl", "GraphNodeInfo", "add_unused_dynamic_deps", "build_exports_map_from_only_dynamic_deps", "build_link_once_static_libs_map", "merge_cc_shared_library_infos", "separate_static_and_dynamic_link_libraries", "sort_linker_inputs", "throw_linked_but_not_exported_errors")
load(":common/cc/cc_helper.bzl", "cc_helper", "linker_mode")
load(":common/cc/cc_info.bzl", "CcInfo")
load(":common/cc/cc_shared_library.bzl", "GraphNodeInfo", "add_unused_dynamic_deps", "build_exports_map_from_only_dynamic_deps", "build_link_once_static_libs_map", "merge_cc_shared_library_infos", "separate_static_and_dynamic_link_libraries", "sort_linker_inputs", "throw_linked_but_not_exported_errors")
load(":common/cc/semantics.bzl", "semantics")
load(":common/cc/cc_common.bzl", "cc_common")

DebugPackageInfo = _builtins.toplevel.DebugPackageInfo
cc_internal = _builtins.internal.cc_internal
Expand Down Expand Up @@ -341,7 +340,7 @@ def _filter_libraries_that_are_linked_dynamically(ctx, feature_configuration, cc
static_linker_inputs = []
linker_inputs = cc_linking_context.linker_inputs.to_list()

all_deps = ctx.attr._deps_analyzed_by_graph_structure_aspect
all_deps = ctx.attr.deps + semantics.get_cc_runtimes(ctx, _is_link_shared(ctx))
graph_structure_aspect_nodes = [dep[GraphNodeInfo] for dep in all_deps if GraphNodeInfo in dep]

can_be_linked_dynamically = {}
Expand Down Expand Up @@ -911,18 +910,20 @@ def _impl(ctx):

return providers

cc_binary = rule(
implementation = _impl,
attrs = cc_binary_attrs,
outputs = {
"stripped_binary": "%{name}.stripped",
"dwp_file": "%{name}.dwp",
},
fragments = ["cpp"] + semantics.additional_fragments(),
exec_groups = {
"cpp_link": exec_group(toolchains = cc_helper.use_cpp_toolchain()),
},
toolchains = cc_helper.use_cpp_toolchain() +
semantics.get_runtimes_toolchain(),
executable = True,
)
def make_cc_binary(cc_binary_attrs, **kwargs):
return rule(
implementation = _impl,
attrs = cc_binary_attrs,
outputs = {
"stripped_binary": "%{name}.stripped",
"dwp_file": "%{name}.dwp",
},
fragments = ["cpp"] + semantics.additional_fragments(),
exec_groups = {
"cpp_link": exec_group(toolchains = cc_helper.use_cpp_toolchain()),
},
toolchains = cc_helper.use_cpp_toolchain() +
semantics.get_runtimes_toolchain(),
executable = True,
**kwargs
)
40 changes: 33 additions & 7 deletions src/main/starlark/builtins_bzl/common/cc/cc_binary_attrs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
"""Attributes for cc_binary.
"""

load(":common/cc/cc_info.bzl", "CcInfo")
load(":common/cc/cc_shared_library.bzl", "dynamic_deps_attrs")
load(":common/cc/semantics.bzl", "semantics")
load(":common/cc/cc_shared_library.bzl", "CcSharedLibraryInfo", "graph_structure_aspect")
load(":common/cc/cc_info.bzl", "CcInfo")

cc_internal = _builtins.internal.cc_internal

cc_binary_attrs = {
cc_binary_attrs_with_aspects = {
"srcs": attr.label_list(
flags = ["DIRECT_COMPILE_TIME_INPUT"],
allow_files = True,
Expand Down Expand Up @@ -54,19 +54,26 @@ cc_binary_attrs = {
allow_rules = semantics.ALLOWED_RULES_IN_DEPS + semantics.ALLOWED_RULES_WITH_WARNINGS_IN_DEPS,
flags = ["SKIP_ANALYSIS_TIME_FILETYPE_CHECK"],
providers = [CcInfo],
aspects = [graph_structure_aspect],
),
"dynamic_deps": attr.label_list(
allow_files = False,
providers = [CcSharedLibraryInfo],
),
"malloc": attr.label(
default = Label("@" + semantics.get_repo() + "//tools/cpp:malloc"),
allow_files = False,
providers = [CcInfo],
allow_rules = ["cc_library"],
aspects = [graph_structure_aspect],
),
"_default_malloc": attr.label(
default = configuration_field(fragment = "cpp", name = "custom_malloc"),
aspects = [graph_structure_aspect],
),
"link_extra_lib": attr.label(
default = Label("@" + semantics.get_repo() + "//tools/cpp:link_extra_lib"),
providers = [CcInfo],
aspects = [graph_structure_aspect],
),
"stamp": attr.int(
values = [-1, 0, 1],
Expand All @@ -91,6 +98,25 @@ cc_binary_attrs = {
"_use_auto_exec_groups": attr.bool(default = True),
}

cc_binary_attrs.update(dynamic_deps_attrs)
cc_binary_attrs.update(semantics.get_distribs_attr())
cc_binary_attrs.update(semantics.get_loose_mode_in_hdrs_check_allowed_attr())
cc_binary_attrs_with_aspects.update(semantics.get_distribs_attr())
cc_binary_attrs_with_aspects.update(semantics.get_loose_mode_in_hdrs_check_allowed_attr())

# Update attributes to contain no aspect implementation.
cc_binary_attrs_without_aspects = dict(cc_binary_attrs_with_aspects)
cc_binary_attrs_without_aspects["deps"] = attr.label_list(
allow_files = semantics.ALLOWED_FILES_IN_DEPS,
allow_rules = semantics.ALLOWED_RULES_IN_DEPS + semantics.ALLOWED_RULES_WITH_WARNINGS_IN_DEPS,
flags = ["SKIP_ANALYSIS_TIME_FILETYPE_CHECK"],
providers = [CcInfo],
)
cc_binary_attrs_without_aspects["malloc"] = attr.label(
default = Label("@" + semantics.get_repo() + "//tools/cpp:malloc"),
allow_files = False,
allow_rules = ["cc_library"],
)
cc_binary_attrs_without_aspects["_default_malloc"] = attr.label(
default = configuration_field(fragment = "cpp", name = "custom_malloc"),
)
cc_binary_attrs_without_aspects["link_extra_lib"] = attr.label(
default = Label("@" + semantics.get_repo() + "//tools/cpp:link_extra_lib"),
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Copyright 2022 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Exports cc_binary variant with aspects.
If dynamic_deps attribute is specified we need to propagate
aspects.
"""

load(":common/cc/cc_binary.bzl", "make_cc_binary")
load(":common/cc/cc_binary_attrs.bzl", "cc_binary_attrs_with_aspects")

cc_binary = make_cc_binary(cc_binary_attrs_with_aspects)
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Copyright 2022 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Exports cc_binary variant without aspects.
If dynamic_deps attribute is not specified we do not propagate
aspects.
"""

load(":common/cc/cc_binary.bzl", "make_cc_binary")
load(":common/cc/cc_binary_attrs.bzl", "cc_binary_attrs_without_aspects")

cc_binary = make_cc_binary(cc_binary_attrs_without_aspects)
30 changes: 30 additions & 0 deletions src/main/starlark/builtins_bzl/common/cc/cc_binary_wrapper.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Copyright 2022 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Macro encapsulating cc_binary rule implementation.
This is to avoid propagating aspect on certain attributes in case
dynamic_deps attribute is not specified.
"""

load(":common/cc/cc_binary_with_aspects.bzl", cc_binary_with_aspects = "cc_binary")
load(":common/cc/cc_binary_without_aspects.bzl", cc_binary_without_aspects = "cc_binary")
load(":common/cc/cc_helper.bzl", "cc_helper")

def cc_binary(**kwargs):
# Propagate an aspect if dynamic_deps attribute is specified.
if "dynamic_deps" in kwargs and cc_helper.is_non_empty_list_or_select(kwargs["dynamic_deps"], "dynamic_deps"):
cc_binary_with_aspects(**kwargs)
else:
cc_binary_without_aspects(**kwargs)
9 changes: 9 additions & 0 deletions src/main/starlark/builtins_bzl/common/cc/cc_helper.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,14 @@ def _generate_def_file(ctx, def_parser, object_files, dll_name):
)
return def_file

def _is_non_empty_list_or_select(value, attr):
if type(value) == "list":
return len(value) > 0
elif type(value) == "select":
return True
else:
fail("Only select or list is valid for {} attr".format(attr))

CC_SOURCE = [".cc", ".cpp", ".cxx", ".c++", ".C", ".cu", ".cl"]
C_SOURCE = [".c"]
OBJC_SOURCE = [".m"]
Expand Down Expand Up @@ -1233,6 +1241,7 @@ cc_helper = struct(
get_copts = _get_copts,
get_expanded_env = _get_expanded_env,
has_target_constraints = _has_target_constraints,
is_non_empty_list_or_select = _is_non_empty_list_or_select,
expand_make_variables_for_copts = _expand_make_variables_for_copts,
build_linking_context_from_libraries = _build_linking_context_from_libraries,
is_stamping_enabled = _is_stamping_enabled,
Expand Down
25 changes: 0 additions & 25 deletions src/main/starlark/builtins_bzl/common/cc/cc_shared_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -840,31 +840,6 @@ cc_shared_library = rule(
fragments = ["cpp"] + semantics.additional_fragments(),
)

def _deps_analyzed_by_graph_structure_aspect(dynamic_deps, linkshared, deps, malloc, link_extra_lib):
if not dynamic_deps:
return []

# Propagate an aspect if dynamic_deps attribute is specified.
all_deps = []
all_deps.extend(deps)

if not linkshared:
all_deps.append(link_extra_lib)
all_deps.append(malloc)
return all_deps

dynamic_deps_attrs = {
"dynamic_deps": attr.label_list(
allow_files = False,
providers = [CcSharedLibraryInfo],
),
"_deps_analyzed_by_graph_structure_aspect": attr.label_list(
providers = [CcInfo],
aspects = [graph_structure_aspect],
default = _deps_analyzed_by_graph_structure_aspect,
),
}

for_testing_dont_use_check_if_target_under_path = _check_if_target_under_path
merge_cc_shared_library_infos = _merge_cc_shared_library_infos
build_link_once_static_libs_map = _build_link_once_static_libs_map
Expand Down
97 changes: 56 additions & 41 deletions src/main/starlark/builtins_bzl/common/cc/cc_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"""cc_test Starlark implementation."""

load(":common/cc/cc_binary.bzl", "cc_binary_impl")
load(":common/cc/cc_binary_attrs.bzl", "cc_binary_attrs")
load(":common/cc/cc_binary_attrs.bzl", "cc_binary_attrs_with_aspects", "cc_binary_attrs_without_aspects")
load(":common/cc/cc_helper.bzl", "cc_helper")
load(":common/cc/semantics.bzl", "semantics")
load(":common/paths.bzl", "paths")
Expand Down Expand Up @@ -77,43 +77,58 @@ def _impl(ctx):
providers.extend(test_providers)
return providers

_cc_test_attrs = dict(cc_binary_attrs)

# Update cc_test defaults:
_cc_test_attrs.update(
_is_test = attr.bool(default = True),
_apple_constraints = attr.label_list(
default = [
"@" + paths.join(semantics.get_platforms_root(), "os:ios"),
"@" + paths.join(semantics.get_platforms_root(), "os:macos"),
"@" + paths.join(semantics.get_platforms_root(), "os:tvos"),
"@" + paths.join(semantics.get_platforms_root(), "os:watchos"),
],
),
# Starlark tests don't get `env_inherit` by default.
env_inherit = attr.string_list(),
stamp = attr.int(values = [-1, 0, 1], default = 0),
linkstatic = attr.bool(default = False),
)
_cc_test_attrs.update(semantics.get_test_malloc_attr())
_cc_test_attrs.update(semantics.get_coverage_attrs())

cc_test = rule(
implementation = _impl,
attrs = _cc_test_attrs,
outputs = {
# TODO(b/198254254): Handle case for windows.
"stripped_binary": "%{name}.stripped",
"dwp_file": "%{name}.dwp",
},
fragments = ["cpp", "coverage"] + semantics.additional_fragments(),
exec_groups = {
"cpp_link": exec_group(toolchains = cc_helper.use_cpp_toolchain()),
# testing.ExecutionInfo defaults to an exec_group of "test".
"test": exec_group(toolchains = [config_common.toolchain_type(_CC_TEST_TOOLCHAIN_TYPE, mandatory = False)]),
},
toolchains = [] +
cc_helper.use_cpp_toolchain() +
semantics.get_runtimes_toolchain(),
test = True,
)
def make_cc_test(with_aspects = False):
"""Makes one of the cc_test rule variants.
This function shall only be used internally in CC ruleset.
Args:
with_aspects: Attaches graph_structure_aspect to `deps` attribute and
implicit deps.
Returns:
A cc_test rule class.
"""
_cc_test_attrs = None
if with_aspects:
_cc_test_attrs = dict(cc_binary_attrs_with_aspects)
else:
_cc_test_attrs = dict(cc_binary_attrs_without_aspects)

# Update cc_test defaults:
_cc_test_attrs.update(
_is_test = attr.bool(default = True),
_apple_constraints = attr.label_list(
default = [
"@" + paths.join(semantics.get_platforms_root(), "os:ios"),
"@" + paths.join(semantics.get_platforms_root(), "os:macos"),
"@" + paths.join(semantics.get_platforms_root(), "os:tvos"),
"@" + paths.join(semantics.get_platforms_root(), "os:watchos"),
],
),
# Starlark tests don't get `env_inherit` by default.
env_inherit = attr.string_list(),
stamp = attr.int(values = [-1, 0, 1], default = 0),
linkstatic = attr.bool(default = False),
)
_cc_test_attrs.update(semantics.get_test_malloc_attr())
_cc_test_attrs.update(semantics.get_coverage_attrs())

return rule(
implementation = _impl,
attrs = _cc_test_attrs,
outputs = {
# TODO(b/198254254): Handle case for windows.
"stripped_binary": "%{name}.stripped",
"dwp_file": "%{name}.dwp",
},
fragments = ["cpp", "coverage"] + semantics.additional_fragments(),
exec_groups = {
"cpp_link": exec_group(toolchains = cc_helper.use_cpp_toolchain()),
# testing.ExecutionInfo defaults to an exec_group of "test".
"test": exec_group(toolchains = [config_common.toolchain_type(_CC_TEST_TOOLCHAIN_TYPE, mandatory = False)]),
},
toolchains = [] +
cc_helper.use_cpp_toolchain() +
semantics.get_runtimes_toolchain(),
test = True,
)
19 changes: 19 additions & 0 deletions src/main/starlark/builtins_bzl/common/cc/cc_test_with_aspects.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Copyright 2022 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""cc_test Starlark implementation."""

load(":common/cc/cc_test.bzl", "make_cc_test")

cc_test = make_cc_test(with_aspects = True)
Loading

0 comments on commit c59739e

Please sign in to comment.