Use OS config features for linking and simplify (#6612)

This lets us use a single undconditional feature for linking with flag
sets that are enabled based on the underlying OS. While here, tidy up
the feature names a bit.

The diff here may look really bad without aggressive whitespace
ignoring, but none of the contents of the two flag sets changed --
they've just be indented more and placed into a single list.

bazel/cc_toolchains/cc_toolchain_linking.bzl

@@ -11,237 +11,237 @@ load(
     "flag_group",
     "flag_set",
     "variable_with_value",
+    "with_feature_set",
 )
 load(
     ":cc_toolchain_actions.bzl",
     "all_link_actions",
 )
 
-default_link_libraries_feature = feature(
-    name = "default_link_libraries",
+link_libraries_feature = feature(
+    name = "link_libraries",
     enabled = True,
-    flag_sets = [flag_set(
-        actions = all_link_actions,
-        flag_groups = [
-            flag_group(
-                expand_if_available = "linkstamp_paths",
-                flags = ["%{linkstamp_paths}"],
-                iterate_over = "linkstamp_paths",
-            ),
-            flag_group(
-                expand_if_available = "libraries_to_link",
-                flag_groups = [
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "object_file_group",
-                        ),
-                        flags = ["-Wl,--start-lib"],
-                    ),
-                    flag_group(
-                        expand_if_true = "libraries_to_link.is_whole_archive",
-                        flags = ["-Wl,-whole-archive"],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "object_file_group",
+    flag_sets = [
+        flag_set(
+            actions = all_link_actions,
+            flag_groups = [
+                flag_group(
+                    expand_if_available = "linkstamp_paths",
+                    flags = ["%{linkstamp_paths}"],
+                    iterate_over = "linkstamp_paths",
+                ),
+                flag_group(
+                    expand_if_available = "libraries_to_link",
+                    flag_groups = [
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "object_file_group",
+                            ),
+                            flags = ["-Wl,--start-lib"],
                         ),
-                        flags = ["%{libraries_to_link.object_files}"],
-                        iterate_over = "libraries_to_link.object_files",
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "object_file",
+                        flag_group(
+                            expand_if_true = "libraries_to_link.is_whole_archive",
+                            flags = ["-Wl,-whole-archive"],
                         ),
-                        flags = ["%{libraries_to_link.name}"],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "interface_library",
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "object_file_group",
+                            ),
+                            flags = ["%{libraries_to_link.object_files}"],
+                            iterate_over = "libraries_to_link.object_files",
                         ),
-                        flags = ["%{libraries_to_link.name}"],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "static_library",
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "object_file",
+                            ),
+                            flags = ["%{libraries_to_link.name}"],
                         ),
-                        flags = ["%{libraries_to_link.name}"],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "dynamic_library",
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "interface_library",
+                            ),
+                            flags = ["%{libraries_to_link.name}"],
                         ),
-                        flags = ["-l%{libraries_to_link.name}"],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "versioned_dynamic_library",
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "static_library",
+                            ),
+                            flags = ["%{libraries_to_link.name}"],
                         ),
-                        flags = ["-l:%{libraries_to_link.name}"],
-                    ),
-                    flag_group(
-                        expand_if_true = "libraries_to_link.is_whole_archive",
-                        flags = ["-Wl,-no-whole-archive"],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "object_file_group",
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "dynamic_library",
+                            ),
+                            flags = ["-l%{libraries_to_link.name}"],
                         ),
-                        flags = ["-Wl,--end-lib"],
-                    ),
-                ],
-                iterate_over = "libraries_to_link",
-            ),
-            # Note that the params file comes at the end, after the
-            # libraries to link above.
-            flag_group(
-                expand_if_available = "linker_param_file",
-                flags = ["@%{linker_param_file}"],
-            ),
-        ],
-    )],
-)
-
-macos_link_libraries_feature = feature(
-    name = "macos_link_libraries",
-    enabled = True,
-    flag_sets = [flag_set(
-        actions = all_link_actions,
-        flag_groups = [
-            flag_group(
-                expand_if_available = "linkstamp_paths",
-                flags = ["%{linkstamp_paths}"],
-                iterate_over = "linkstamp_paths",
-            ),
-            flag_group(
-                expand_if_available = "libraries_to_link",
-                flag_groups = [
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "object_file_group",
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "versioned_dynamic_library",
+                            ),
+                            flags = ["-l:%{libraries_to_link.name}"],
                         ),
-                        flags = ["-Wl,--start-lib"],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "object_file_group",
+                        flag_group(
+                            expand_if_true = "libraries_to_link.is_whole_archive",
+                            flags = ["-Wl,-no-whole-archive"],
                         ),
-                        flag_groups = [
-                            flag_group(
-                                expand_if_false = "libraries_to_link.is_whole_archive",
-                                flags = ["%{libraries_to_link.object_files}"],
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "object_file_group",
                             ),
-                            flag_group(
-                                expand_if_true = "libraries_to_link.is_whole_archive",
-                                flags = ["-Wl,-force_load,%{libraries_to_link.object_files}"],
+                            flags = ["-Wl,--end-lib"],
+                        ),
+                    ],
+                    iterate_over = "libraries_to_link",
+                ),
+                # Note that the params file comes at the end, after the
+                # libraries to link above.
+                flag_group(
+                    expand_if_available = "linker_param_file",
+                    flags = ["@%{linker_param_file}"],
+                ),
+            ],
+            with_features = [with_feature_set(not_features = ["macos_target"])],
+        ),
+        flag_set(
+            actions = all_link_actions,
+            flag_groups = [
+                flag_group(
+                    expand_if_available = "linkstamp_paths",
+                    flags = ["%{linkstamp_paths}"],
+                    iterate_over = "linkstamp_paths",
+                ),
+                flag_group(
+                    expand_if_available = "libraries_to_link",
+                    flag_groups = [
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "object_file_group",
                             ),
-                        ],
-                        iterate_over = "libraries_to_link.object_files",
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "object_file",
+                            flags = ["-Wl,--start-lib"],
                         ),
-                        flag_groups = [
-                            flag_group(
-                                expand_if_false = "libraries_to_link.is_whole_archive",
-                                flags = ["%{libraries_to_link.name}"],
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "object_file_group",
                             ),
-                            flag_group(
-                                expand_if_true = "libraries_to_link.is_whole_archive",
-                                flags = ["-Wl,-force_load,%{libraries_to_link.name}"],
+                            flag_groups = [
+                                flag_group(
+                                    expand_if_false = "libraries_to_link.is_whole_archive",
+                                    flags = ["%{libraries_to_link.object_files}"],
+                                ),
+                                flag_group(
+                                    expand_if_true = "libraries_to_link.is_whole_archive",
+                                    flags = ["-Wl,-force_load,%{libraries_to_link.object_files}"],
+                                ),
+                            ],
+                            iterate_over = "libraries_to_link.object_files",
+                        ),
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "object_file",
                             ),
-                        ],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "interface_library",
+                            flag_groups = [
+                                flag_group(
+                                    expand_if_false = "libraries_to_link.is_whole_archive",
+                                    flags = ["%{libraries_to_link.name}"],
+                                ),
+                                flag_group(
+                                    expand_if_true = "libraries_to_link.is_whole_archive",
+                                    flags = ["-Wl,-force_load,%{libraries_to_link.name}"],
+                                ),
+                            ],
                         ),
-                        flag_groups = [
-                            flag_group(
-                                expand_if_false = "libraries_to_link.is_whole_archive",
-                                flags = ["%{libraries_to_link.name}"],
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "interface_library",
                             ),
-                            flag_group(
-                                expand_if_true = "libraries_to_link.is_whole_archive",
-                                flags = ["-Wl,-force_load,%{libraries_to_link.name}"],
+                            flag_groups = [
+                                flag_group(
+                                    expand_if_false = "libraries_to_link.is_whole_archive",
+                                    flags = ["%{libraries_to_link.name}"],
+                                ),
+                                flag_group(
+                                    expand_if_true = "libraries_to_link.is_whole_archive",
+                                    flags = ["-Wl,-force_load,%{libraries_to_link.name}"],
+                                ),
+                            ],
+                        ),
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "static_library",
                             ),
-                        ],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "static_library",
+                            flag_groups = [
+                                flag_group(
+                                    expand_if_false = "libraries_to_link.is_whole_archive",
+                                    flags = ["%{libraries_to_link.name}"],
+                                ),
+                                flag_group(
+                                    expand_if_true = "libraries_to_link.is_whole_archive",
+                                    flags = ["-Wl,-force_load,%{libraries_to_link.name}"],
+                                ),
+                            ],
                         ),
-                        flag_groups = [
-                            flag_group(
-                                expand_if_false = "libraries_to_link.is_whole_archive",
-                                flags = ["%{libraries_to_link.name}"],
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "dynamic_library",
                             ),
-                            flag_group(
-                                expand_if_true = "libraries_to_link.is_whole_archive",
-                                flags = ["-Wl,-force_load,%{libraries_to_link.name}"],
+                            flags = ["-l%{libraries_to_link.name}"],
+                        ),
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "versioned_dynamic_library",
                             ),
-                        ],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "dynamic_library",
+                            flags = ["-l:%{libraries_to_link.name}"],
                         ),
-                        flags = ["-l%{libraries_to_link.name}"],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "versioned_dynamic_library",
+                        flag_group(
+                            expand_if_true = "libraries_to_link.is_whole_archive",
+                            flag_groups = [
+                                flag_group(
+                                    expand_if_false = "macos_flags",
+                                    flags = ["-Wl,-no-whole-archive"],
+                                ),
+                            ],
                         ),
-                        flags = ["-l:%{libraries_to_link.name}"],
-                    ),
-                    flag_group(
-                        expand_if_true = "libraries_to_link.is_whole_archive",
-                        flag_groups = [
-                            flag_group(
-                                expand_if_false = "macos_flags",
-                                flags = ["-Wl,-no-whole-archive"],
+                        flag_group(
+                            expand_if_equal = variable_with_value(
+                                name = "libraries_to_link.type",
+                                value = "object_file_group",
                             ),
-                        ],
-                    ),
-                    flag_group(
-                        expand_if_equal = variable_with_value(
-                            name = "libraries_to_link.type",
-                            value = "object_file_group",
+                            flags = ["-Wl,--end-lib"],
                         ),
-                        flags = ["-Wl,--end-lib"],
-                    ),
-                ],
-                iterate_over = "libraries_to_link",
-            ),
-            # Note that the params file comes at the end, after the
-            # libraries to link above.
-            flag_group(
-                expand_if_available = "linker_param_file",
-                flags = ["@%{linker_param_file}"],
-            ),
-        ],
-    )],
+                    ],
+                    iterate_over = "libraries_to_link",
+                ),
+                # Note that the params file comes at the end, after the
+                # libraries to link above.
+                flag_group(
+                    expand_if_available = "linker_param_file",
+                    flags = ["@%{linker_param_file}"],
+                ),
+            ],
+            with_features = [with_feature_set(["macos_target"])],
+        ),
+    ],
 )
 
 # Archive actions have an entirely independent set of flags and don't
 # interact with either compiler or link actions.
-default_archiver_flags_feature = feature(
-    name = "default_archiver_flags",
+archiving_feature = feature(
+    name = "archiving",
     enabled = True,
     flag_sets = [flag_set(
         actions = [ACTION_NAMES.cpp_link_static_library],
@@ -283,5 +283,6 @@ default_archiver_flags_feature = feature(
 # Note that the order of features is significant in this list and determines the
 # relative order of flags from the features listed.
 linking_features = [
-    default_archiver_flags_feature,
+    link_libraries_feature,
+    archiving_feature,
 ]

bazel/cc_toolchains/clang_cc_toolchain_config.bzl

@@ -38,12 +38,7 @@ load(
     "libcxx_feature",
 )
 load(":cc_toolchain_debugging.bzl", "debugging_features")
-load(
-    ":cc_toolchain_linking.bzl",
-    "default_link_libraries_feature",
-    "linking_features",
-    "macos_link_libraries_feature",
-)
+load(":cc_toolchain_linking.bzl", "linking_features")
 load(":cc_toolchain_modules.bzl", "modules_features")
 load(":cc_toolchain_optimization.bzl", "optimization_features")
 load(":cc_toolchain_sanitizer_features.bzl", "sanitizer_features")
@@ -151,10 +146,6 @@ def _build_features(ctx):
         feature(name = "supports_dynamic_linker", enabled = ctx.attr.target_os == "linux"),
         feature(name = "supports_start_end_lib", enabled = ctx.attr.target_os == "linux"),
     ]
-    if ctx.attr.target_os == "macos":
-        features.append(macos_link_libraries_feature)
-    else:
-        features.append(default_link_libraries_feature)
 
     # Lastly, we add a feature that enables others in the default `fastbuild`
     # mode. This is also a good place to add any project-specific features.