From f999a8e23b3b738cf2cb9b0f896cb79458de2ffa Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 5 Sep 2024 09:36:32 -0700 Subject: [PATCH] Implement archiver_flags as cc_args BEGIN_PUBLIC Implement archiver_flags as cc_args This is the first change in a series that will reimplement the legacy features defined in Bazel's Java code using the new toolchain rules. These implementations are redefined as raw arguments using the new toolchain rules, which allows for the definition shared and reused across different contexts. This first change implements archiver_flags, which is used to produce the arguments for the archiver tool used by `cc_library`. This CL also adds a test that compares the feature implementation produced by the new rules to the feature implementation defined in Bazel's Java code. This should make it easier to review the final result and ensure it is functionally equivalent to the Java implementation. END_PUBLIC PiperOrigin-RevId: 671403834 Change-Id: I2d4f15b49619a11995a50c86439340ea532e360e --- cc/toolchains/args/BUILD | 42 ++++++++++++ cc/toolchains/args/archiver_flags/BUILD | 68 +++++++++++++++++++ cc/toolchains/features/legacy/BUILD | 1 + .../legacy_features_as_args/BUILD | 19 ++++++ .../compare_feature.bzl | 61 +++++++++++++++++ .../goldens/macos/archiver_flags.textproto | 40 +++++++++++ .../goldens/unix/archiver_flags.textproto | 39 +++++++++++ 7 files changed, 270 insertions(+) create mode 100644 cc/toolchains/args/archiver_flags/BUILD create mode 100644 tests/rule_based_toolchain/legacy_features_as_args/BUILD create mode 100644 tests/rule_based_toolchain/legacy_features_as_args/compare_feature.bzl create mode 100644 tests/rule_based_toolchain/legacy_features_as_args/goldens/macos/archiver_flags.textproto create mode 100644 tests/rule_based_toolchain/legacy_features_as_args/goldens/unix/archiver_flags.textproto diff --git a/cc/toolchains/args/BUILD b/cc/toolchains/args/BUILD index e69de29..b12019c 100644 --- a/cc/toolchains/args/BUILD +++ b/cc/toolchains/args/BUILD @@ -0,0 +1,42 @@ +load("//cc/toolchains:feature.bzl", "cc_feature") + +package(default_visibility = ["//visibility:public"]) + +# All of these arguments originate from the legacy features defined in Bazel's Java code: +# https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java + +# This feature replaces the need for action configs to list legacy features +# in `implies` to produce a working toolchain. The full list is the set of +# features that are implied (enabled) by built-in action config definitions. +# Note that some other legacy features are still hidden and enabled by default, +# and others exist that are NOT enabled at all by default. As args are built +# out, the `implies` entry should be removed and then moved into `args`. +cc_feature( + name = "experimental_replace_legacy_action_config_features", + args = [ + "//cc/toolchains/args/archiver_flags", + ], + feature_name = "experimental_replace_legacy_action_config_features", + # TODO: Convert remaining items in this list into their actual args. + implies = [ + "//cc/toolchains/features/legacy:build_interface_libraries", + "//cc/toolchains/features/legacy:compiler_input_flags", + "//cc/toolchains/features/legacy:compiler_output_flags", + "//cc/toolchains/features/legacy:dynamic_library_linker_tool", + "//cc/toolchains/features/legacy:fission_support", + "//cc/toolchains/features/legacy:legacy_compile_flags", + "//cc/toolchains/features/legacy:legacy_link_flags", + "//cc/toolchains/features/legacy:library_search_directories", + "//cc/toolchains/features/legacy:linkstamps", + "//cc/toolchains/features/legacy:output_execpath_flags", + "//cc/toolchains/features/legacy:strip_debug_symbols", + "//cc/toolchains/features/legacy:unfiltered_compile_flags", + "//cc/toolchains/features/legacy:user_compile_flags", + "//cc/toolchains/features/legacy:user_link_flags", + "//cc/toolchains/features/legacy:force_pic_flags", + "//cc/toolchains/features/legacy:libraries_to_link", + "//cc/toolchains/features/legacy:linker_param_file", + "//cc/toolchains/features/legacy:runtime_library_search_directories", + "//cc/toolchains/features/legacy:shared_flag", + ], +) diff --git a/cc/toolchains/args/archiver_flags/BUILD b/cc/toolchains/args/archiver_flags/BUILD new file mode 100644 index 0000000..d293ed9 --- /dev/null +++ b/cc/toolchains/args/archiver_flags/BUILD @@ -0,0 +1,68 @@ +load("//cc/toolchains:args.bzl", "cc_args") +load("//cc/toolchains:args_list.bzl", "cc_args_list") +load("//cc/toolchains:nested_args.bzl", "cc_nested_args") + +package(default_visibility = ["//visibility:private"]) + +cc_args_list( + name = "archiver_flags", + args = [ + ":create_static_archive", + ":output_execpath", + ":libraries_to_link", + ], + visibility = ["//visibility:public"], +) + +cc_args( + name = "create_static_archive", + actions = ["//cc/toolchains/actions:cpp_link_static_library"], + args = select({ + "@platforms//os:macos": ["-static"], + "//conditions:default": ["rcsD"], + }), +) + +cc_args( + name = "output_execpath", + actions = ["//cc/toolchains/actions:cpp_link_static_library"], + args = select({ + "@platforms//os:macos": ["-o"], + "//conditions:default": [], + }) + ["{output_execpath}"], + format = {"output_execpath": "//cc/toolchains/variables:output_execpath"}, + requires_not_none = "//cc/toolchains/variables:output_execpath", +) + +cc_args( + name = "libraries_to_link", + actions = ["//cc/toolchains/actions:cpp_link_static_library"], + nested = ["libraries_to_link_expansion"], + requires_not_none = "//cc/toolchains/variables:libraries_to_link", +) + +cc_nested_args( + name = "libraries_to_link_expansion", + iterate_over = "//cc/toolchains/variables:libraries_to_link", + nested = [ + ":link_obj_file", + ":link_object_file_group", + ], +) + +cc_nested_args( + name = "link_obj_file", + args = ["{object_file}"], + format = {"object_file": "//cc/toolchains/variables:libraries_to_link.name"}, + requires_equal = "//cc/toolchains/variables:libraries_to_link.type", + requires_equal_value = "object_file", +) + +cc_nested_args( + name = "link_object_file_group", + args = ["{object_files}"], + format = {"object_files": "//cc/toolchains/variables:libraries_to_link.object_files"}, + iterate_over = "//cc/toolchains/variables:libraries_to_link.object_files", + requires_equal = "//cc/toolchains/variables:libraries_to_link.type", + requires_equal_value = "object_file_group", +) diff --git a/cc/toolchains/features/legacy/BUILD b/cc/toolchains/features/legacy/BUILD index f7e4dd9..b1db5be 100644 --- a/cc/toolchains/features/legacy/BUILD +++ b/cc/toolchains/features/legacy/BUILD @@ -127,6 +127,7 @@ cc_external_feature( cc_external_feature( name = "archiver_flags", + deprecation = "Use //cc/toolchains/args/archiver_flags instead", feature_name = "archiver_flags", overridable = True, ) diff --git a/tests/rule_based_toolchain/legacy_features_as_args/BUILD b/tests/rule_based_toolchain/legacy_features_as_args/BUILD new file mode 100644 index 0000000..24f3c44 --- /dev/null +++ b/tests/rule_based_toolchain/legacy_features_as_args/BUILD @@ -0,0 +1,19 @@ +load(":compare_feature.bzl", "compare_feature_implementation") + +# These tests validate that the produced legacy feature implementations +# properly reflect the implementations housed in the Java source of truth at +# https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java. +# +# Note: the golden textprotos are not a 1:1 match of the textprotos inlined in +# the Java files. These aim to have identical behavior, but with allowance +# for slight differences in structure due to implementation details. This also +# makes it easier to review the final results. + +compare_feature_implementation( + name = "archiver_flags_test", + actual_implementation = "//cc/toolchains/args/archiver_flags", + expected = select({ + "@platforms//os:macos": "//tests/rule_based_toolchain/legacy_features_as_args:goldens/macos/archiver_flags.textproto", + "//conditions:default": "//tests/rule_based_toolchain/legacy_features_as_args:goldens/unix/archiver_flags.textproto", + }), +) diff --git a/tests/rule_based_toolchain/legacy_features_as_args/compare_feature.bzl b/tests/rule_based_toolchain/legacy_features_as_args/compare_feature.bzl new file mode 100644 index 0000000..10e63a8 --- /dev/null +++ b/tests/rule_based_toolchain/legacy_features_as_args/compare_feature.bzl @@ -0,0 +1,61 @@ +# Copyright 2024 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. +"""Test helper for cc_arg_list validation.""" + +load("@bazel_skylib//rules:diff_test.bzl", "diff_test") +load("//cc:cc_toolchain_config_lib.bzl", "feature") +load("//cc/toolchains:cc_toolchain_info.bzl", "ArgsListInfo") +load("//cc/toolchains/impl:legacy_converter.bzl", "convert_args") + +def _generate_textproto_for_args_impl(ctx): + out = ctx.actions.declare_file(ctx.attr.output.name) + converted_args = [convert_args(arg) for arg in ctx.attr.actual_implementation[ArgsListInfo].args] + feature_impl = feature( + name = ctx.attr.feature_name, + flag_sets = [fs for one_arg in converted_args for fs in one_arg.flag_sets], + env_sets = [es for one_arg in converted_args for es in one_arg.env_sets], + ) + strip_types = [line for line in proto.encode_text(feature_impl).splitlines() if "type_name:" not in line] + + # Ensure trailing newline. + strip_types.append("") + ctx.actions.write(out, "\n".join(strip_types)) + return DefaultInfo(files = depset([out])) + +_generate_textproto_for_args = rule( + implementation = _generate_textproto_for_args_impl, + attrs = { + "actual_implementation": attr.label( + mandatory = True, + providers = [ArgsListInfo], + ), + "feature_name": attr.string(mandatory = True), + "output": attr.output(mandatory = True), + }, +) + +def compare_feature_implementation(name, actual_implementation, expected): + output_filename = name + ".actual.textproto" + _generate_textproto_for_args( + name = name + "_implementation", + actual_implementation = actual_implementation, + feature_name = name, + output = output_filename, + testonly = True, + ) + diff_test( + name = name, + file1 = expected, + file2 = output_filename, + ) diff --git a/tests/rule_based_toolchain/legacy_features_as_args/goldens/macos/archiver_flags.textproto b/tests/rule_based_toolchain/legacy_features_as_args/goldens/macos/archiver_flags.textproto new file mode 100644 index 0000000..b783c56 --- /dev/null +++ b/tests/rule_based_toolchain/legacy_features_as_args/goldens/macos/archiver_flags.textproto @@ -0,0 +1,40 @@ +enabled: false +flag_sets { + actions: "c++-link-static-library" + flag_groups { + flags: "-static" + } +} +flag_sets { + actions: "c++-link-static-library" + flag_groups { + expand_if_available: "output_execpath" + flags: "-o" + flags: "%{output_execpath}" + } +} +flag_sets { + actions: "c++-link-static-library" + flag_groups { + expand_if_available: "libraries_to_link" + flag_groups { + flag_groups { + expand_if_equal { + name: "libraries_to_link.type" + value: "object_file" + } + flags: "%{libraries_to_link.name}" + } + flag_groups { + expand_if_equal { + name: "libraries_to_link.type" + value: "object_file_group" + } + flags: "%{libraries_to_link.object_files}" + iterate_over: "libraries_to_link.object_files" + } + iterate_over: "libraries_to_link" + } + } +} +name: "archiver_flags_test" diff --git a/tests/rule_based_toolchain/legacy_features_as_args/goldens/unix/archiver_flags.textproto b/tests/rule_based_toolchain/legacy_features_as_args/goldens/unix/archiver_flags.textproto new file mode 100644 index 0000000..a1944bb --- /dev/null +++ b/tests/rule_based_toolchain/legacy_features_as_args/goldens/unix/archiver_flags.textproto @@ -0,0 +1,39 @@ +enabled: false +flag_sets { + actions: "c++-link-static-library" + flag_groups { + flags: "rcsD" + } +} +flag_sets { + actions: "c++-link-static-library" + flag_groups { + expand_if_available: "output_execpath" + flags: "%{output_execpath}" + } +} +flag_sets { + actions: "c++-link-static-library" + flag_groups { + expand_if_available: "libraries_to_link" + flag_groups { + flag_groups { + expand_if_equal { + name: "libraries_to_link.type" + value: "object_file" + } + flags: "%{libraries_to_link.name}" + } + flag_groups { + expand_if_equal { + name: "libraries_to_link.type" + value: "object_file_group" + } + flags: "%{libraries_to_link.object_files}" + iterate_over: "libraries_to_link.object_files" + } + iterate_over: "libraries_to_link" + } + } +} +name: "archiver_flags_test"