From b65e786d95dbd5c6e0c5e21f1f39e34064b1a480 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Wed, 10 Jun 2026 09:35:52 -0700 Subject: [PATCH] Fix another case of tool_paths vs action configs This also dedups the helper function of this, which was merged in from 2 prs at once --- cc/common/cc_helper.bzl | 50 +++++++------------ .../cc_toolchain_provider_helper.bzl | 30 ++++++++--- 2 files changed, 42 insertions(+), 38 deletions(-) diff --git a/cc/common/cc_helper.bzl b/cc/common/cc_helper.bzl index 36afcad71..29f1a6ff5 100644 --- a/cc/common/cc_helper.bzl +++ b/cc/common/cc_helper.bzl @@ -464,52 +464,39 @@ def _get_compilation_contexts_from_deps(deps): compilation_contexts.append(dep[CcInfo].compilation_context) return compilation_contexts -def _tool_path(cc_toolchain, tool, feature_configuration = None, action_name = None): - tool = cc_toolchain._tool_paths.get(tool, None) - if tool: - return tool +def _tool_path(tool_paths, tool, feature_configuration = None, action_name = None, default = ""): + path = tool_paths.get(tool, None) + if path: + return path if feature_configuration != None and action_name != None: if not cc_common.action_is_enabled( feature_configuration = feature_configuration, action_name = action_name, ): - return None - - return cc_common.get_tool_for_action( - feature_configuration = feature_configuration, - action_name = action_name, - ) - return None + return default -def _tool_path_for_action(cc_toolchain, tool, feature_configuration, action_name): - path = cc_toolchain._tool_paths.get(tool, None) - if path: - return path - if action_name != None and cc_common.action_is_enabled( - feature_configuration = feature_configuration, - action_name = action_name, - ): return cc_common.get_tool_for_action( feature_configuration = feature_configuration, action_name = action_name, - ) or "" - return "" + ) or default + return default def _get_toolchain_global_make_variables(cc_toolchain, feature_configuration): + tool_paths = cc_toolchain._tool_paths result = { - "CC": _tool_path_for_action(cc_toolchain, "gcc", feature_configuration, ACTION_NAMES.c_compile), - "AR": _tool_path_for_action(cc_toolchain, "ar", feature_configuration, ACTION_NAMES.cpp_link_static_library), - "NM": _tool_path_for_action(cc_toolchain, "nm", feature_configuration, None), - "LD": _tool_path_for_action(cc_toolchain, "ld", feature_configuration, ACTION_NAMES.cpp_link_executable), - "STRIP": _tool_path_for_action(cc_toolchain, "strip", feature_configuration, ACTION_NAMES.strip), + "CC": _tool_path(tool_paths, "gcc", feature_configuration, ACTION_NAMES.c_compile), + "AR": _tool_path(tool_paths, "ar", feature_configuration, ACTION_NAMES.cpp_link_static_library), + "NM": _tool_path(tool_paths, "nm", feature_configuration, None), + "LD": _tool_path(tool_paths, "ld", feature_configuration, ACTION_NAMES.cpp_link_executable), + "STRIP": _tool_path(tool_paths, "strip", feature_configuration, ACTION_NAMES.strip), "C_COMPILER": cc_toolchain.compiler, } # buildifier: disable=unsorted-dict-items - obj_copy_tool = _tool_path_for_action(cc_toolchain, "objcopy", feature_configuration, ACTION_NAMES.objcopy_embed_data) + obj_copy_tool = _tool_path(tool_paths, "objcopy", feature_configuration, ACTION_NAMES.objcopy_embed_data) if obj_copy_tool != None: # objcopy is optional in Crostool. result["OBJCOPY"] = obj_copy_tool - gcov_tool = _tool_path_for_action(cc_toolchain, "gcov-tool", feature_configuration, None) + gcov_tool = _tool_path(tool_paths, "gcov-tool", feature_configuration, None) if gcov_tool: # gcovtool is optional in Crostool. result["GCOVTOOL"] = gcov_tool @@ -1029,9 +1016,9 @@ def _get_coverage_environment(ctx, cc_config, cc_toolchain, feature_configuratio # buildifier: disable=unsorted-dict-items env = { - "COVERAGE_GCOV_PATH": _tool_path(cc_toolchain, "gcov", feature_configuration, ACTION_NAMES.gcov), - "LLVM_COV": _tool_path(cc_toolchain, "llvm-cov", feature_configuration, ACTION_NAMES.llvm_cov), - "LLVM_PROFDATA": _tool_path(cc_toolchain, "llvm-profdata", feature_configuration, ACTION_NAMES.llvm_profdata), + "COVERAGE_GCOV_PATH": _tool_path(cc_toolchain._tool_paths, "gcov", feature_configuration, ACTION_NAMES.gcov, default = None), + "LLVM_COV": _tool_path(cc_toolchain._tool_paths, "llvm-cov", feature_configuration, ACTION_NAMES.llvm_cov, default = None), + "LLVM_PROFDATA": _tool_path(cc_toolchain._tool_paths, "llvm-profdata", feature_configuration, ACTION_NAMES.llvm_profdata, default = None), "GENERATE_LLVM_LCOV": "1" if cc_config.generate_llvm_lcov() else "0", } for k in list(env.keys()): @@ -1101,6 +1088,7 @@ cc_helper = struct( tokenize = _tokenize, is_valid_shared_library_artifact = _is_valid_shared_library_artifact, is_valid_shared_library_name = _is_valid_shared_library_name, + tool_path = _tool_path, get_toolchain_global_make_variables = _get_toolchain_global_make_variables, get_cc_flags_make_variable = _get_cc_flags_make_variable, get_compilation_contexts_from_deps = _get_compilation_contexts_from_deps, diff --git a/cc/private/rules_impl/cc_toolchain_provider_helper.bzl b/cc/private/rules_impl/cc_toolchain_provider_helper.bzl index 294419bff..8a57b175c 100644 --- a/cc/private/rules_impl/cc_toolchain_provider_helper.bzl +++ b/cc/private/rules_impl/cc_toolchain_provider_helper.bzl @@ -15,6 +15,7 @@ """A helper for creating CcToolchainProvider.""" load("@bazel_skylib//lib:paths.bzl", "paths") +load("//cc:action_names.bzl", "ACTION_NAMES") load("//cc/common:cc_common.bzl", "cc_common") load("//cc/common:cc_helper.bzl", "cc_helper") load("//cc/common:cc_helper_internal.bzl", "get_relative_path") @@ -151,8 +152,23 @@ def get_cc_toolchain_provider(ctx, attributes): ) tool_paths = _compute_tool_paths(toolchain_config_info, tools_directory) toolchain_features = cc_common.cc_toolchain_features(toolchain_config_info = toolchain_config_info, tools_directory = tools_directory) + + # action_is_enabled only returns true for action_configs that are in + # requested_features (or transitively implied). Request the action names + # we fall back to below so the guard in tool_path returns true + # when those action_configs are declared. + feature_configuration = toolchain_features.configure_features(requested_features = [ + ACTION_NAMES.c_compile, + ACTION_NAMES.cpp_link_static_library, + ACTION_NAMES.cpp_link_executable, + ACTION_NAMES.strip, + ACTION_NAMES.objcopy_embed_data, + ACTION_NAMES.gcov, + ACTION_NAMES.llvm_profdata, + ]) + fdo_context = create_fdo_context( - llvm_profdata = tool_paths.get("llvm-profdata"), + llvm_profdata = cc_helper.tool_path(tool_paths, "llvm-profdata", feature_configuration, ACTION_NAMES.llvm_profdata), all_files = attributes.all_files, zipper = attributes.zipper, cc_toolchain_config_info = toolchain_config_info, @@ -230,15 +246,15 @@ def get_cc_toolchain_provider(ctx, attributes): solib_dir = solib_directory, additional_make_variables = _additional_make_variables(toolchain_config_info.make_variables), legacy_cc_flags_make_variable = _legacy_cc_flags_make_variable(toolchain_config_info.make_variables), - objcopy_executable = tool_paths.get("objcopy", ""), - compiler_executable = tool_paths.get("gcc", ""), + objcopy_executable = cc_helper.tool_path(tool_paths, "objcopy", feature_configuration, ACTION_NAMES.objcopy_embed_data), + compiler_executable = cc_helper.tool_path(tool_paths, "gcc", feature_configuration, ACTION_NAMES.c_compile), preprocessor_executable = tool_paths.get("cpp", ""), nm_executable = tool_paths.get("nm", ""), objdump_executable = tool_paths.get("objdump", ""), - ar_executable = tool_paths.get("ar", ""), - strip_executable = tool_paths.get("strip", ""), - ld_executable = tool_paths.get("ld", ""), - gcov_executable = tool_paths.get("gcov", ""), + ar_executable = cc_helper.tool_path(tool_paths, "ar", feature_configuration, ACTION_NAMES.cpp_link_static_library), + strip_executable = cc_helper.tool_path(tool_paths, "strip", feature_configuration, ACTION_NAMES.strip), + ld_executable = cc_helper.tool_path(tool_paths, "ld", feature_configuration, ACTION_NAMES.cpp_link_executable), + gcov_executable = cc_helper.tool_path(tool_paths, "gcov", feature_configuration, ACTION_NAMES.gcov), build_variables_dict = build_variables_dict, build_variables = build_variables, all_files = attributes.all_files,