Use tool from action_config for link-executable and link-dynamic-lib actions
This cl finishes the last bit of c++ linking actions migration to crosstool's
action_configs. From now on, the action_config { tool_path: ... } will be used,
instead of top level tool { path: ... }.
RELNOTES: Bazel now uses tools from action_configs in Crosstool by default (as
oposed to using top level tools).
PiperOrigin-RevId: 159677525
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
index 5dd5856..d1a8835 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
@@ -118,7 +118,7 @@
" config_name: 'c++-link-executable'",
" action_name: 'c++-link-executable'",
" tool {",
- " tool_path: 'DUMMY_TOOL'",
+ " tool_path: '" + gccToolPath + "'",
" }",
" implies: 'symbol_counts'",
" implies: 'strip_debug_symbols'",
@@ -136,7 +136,7 @@
" config_name: 'c++-link-dynamic-library'",
" action_name: 'c++-link-dynamic-library'",
" tool {",
- " tool_path: 'DUMMY_TOOL'",
+ " tool_path: '" + gccToolPath + "'",
" }",
" implies: 'build_interface_libraries'",
" implies: 'dynamic_library_linker_tool'",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
index 4e1d5f4..e7ae470 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
@@ -755,9 +755,15 @@
.setToolchain(toolchain)
.setFdoSupport(fdoSupport.getFdoSupport())
.setBuildVariables(buildVariables)
- .setToolPath(getToolPath())
.setFeatureConfiguration(featureConfiguration);
+ // TODO(b/62693279): Cleanup once internal crosstools specify ifso building correctly.
+ if (linkType.equals(LinkTargetType.DYNAMIC_LIBRARY)
+ && !featureConfiguration.hasConfiguredLinkerPathInActionConfig()) {
+ linkCommandLineBuilder.forceToolPath(
+ toolchain.getLinkDynamicLibraryTool().getExecPathString());
+ }
+
if (!isLTOIndexing) {
linkCommandLineBuilder
.setOutput(output)
@@ -889,26 +895,6 @@
executionRequirements.build());
}
- /**
- * Returns the tool path from feature configuration, if the tool in the configuration is sane, or
- * builtin tool, if configuration has a dummy value.
- */
- private String getToolPath() {
- if (!featureConfiguration.actionIsConfigured(linkType.getActionName())) {
- return null;
- }
- String toolPath =
- featureConfiguration
- .getToolForAction(linkType.getActionName())
- .getToolPath(cppConfiguration.getCrosstoolTopPathFragment())
- .getPathString();
- if (linkType.equals(LinkTargetType.DYNAMIC_LIBRARY)
- && !featureConfiguration.hasConfiguredLinkerPathInActionConfig()) {
- toolPath = toolchain.getLinkDynamicLibraryTool().getExecPathString();
- }
- return toolPath;
- }
-
/** The default heuristic on whether we need to use whole-archive for the link. */
private static boolean needWholeArchive(
LinkStaticness staticness,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
index 1a68752..b279440 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java
@@ -53,7 +53,7 @@
@Immutable
public final class LinkCommandLine extends CommandLine {
private final String actionName;
- private final String toolPath;
+ private final String forcedToolPath;
private final boolean codeCoverageEnabled;
private final CppConfiguration cppConfiguration;
private final ActionOwner owner;
@@ -80,7 +80,7 @@
private LinkCommandLine(
String actionName,
- String toolPath,
+ String forcedToolPath,
BuildConfiguration configuration,
ActionOwner owner,
Artifact output,
@@ -103,7 +103,7 @@
CcToolchainProvider ccProvider) {
this.actionName = actionName;
- this.toolPath = toolPath;
+ this.forcedToolPath = forcedToolPath;
this.codeCoverageEnabled = configuration.isCodeCoverageEnabled();
this.cppConfiguration = configuration.getFragment(CppConfiguration.class);
this.variables = variables;
@@ -320,6 +320,9 @@
}
private ImmutableList<String> getToolchainFlags() {
+ if (Staticness.STATIC.equals(linkTargetType.staticness())) {
+ return ImmutableList.of();
+ }
boolean fullyStatic = (linkStaticness == LinkStaticness.FULLY_STATIC);
boolean mostlyStatic = (linkStaticness == LinkStaticness.MOSTLY_STATIC);
boolean sharedLinkopts =
@@ -379,52 +382,23 @@
*/
public List<String> getRawLinkArgv() {
List<String> argv = new ArrayList<>();
-
- // TODO(b/30109612): Extract this switch into individual crosstools once action configs are no
- // longer hardcoded in CppActionConfigs.
- switch (linkTargetType) {
- case EXECUTABLE:
- argv.add(cppConfiguration.getCppExecutable().getPathString());
- argv.addAll(
- featureConfiguration.getCommandLine(
- actionName,
- new Variables.Builder()
- .addAll(variables)
- .addStringSequenceVariable(
- CppLinkActionBuilder.LEGACY_LINK_FLAGS_VARIABLE, getToolchainFlags())
- .build()));
- break;
-
- case STATIC_LIBRARY:
- case PIC_STATIC_LIBRARY:
- case ALWAYS_LINK_STATIC_LIBRARY:
- case ALWAYS_LINK_PIC_STATIC_LIBRARY:
- argv.add(toolPath);
- argv.addAll(featureConfiguration.getCommandLine(actionName, variables));
- break;
-
- // Since the objc case/dynamic libs is not hardcoded in CppConfiguration, we can use the
- // actual tool.
- // TODO(b/30109612): make this pattern the case for all link variants.
- case DYNAMIC_LIBRARY:
- case OBJC_ARCHIVE:
- case OBJC_FULLY_LINKED_ARCHIVE:
- case OBJC_EXECUTABLE:
- case OBJCPP_EXECUTABLE:
- argv.add(toolPath);
- argv.addAll(
- featureConfiguration.getCommandLine(
- actionName,
- new Variables.Builder()
- .addAll(variables)
- .addStringSequenceVariable(
- CppLinkActionBuilder.LEGACY_LINK_FLAGS_VARIABLE, getToolchainFlags())
- .build()));
- break;
-
- default:
- throw new IllegalArgumentException();
+ if (forcedToolPath != null) {
+ argv.add(forcedToolPath);
+ } else {
+ argv.add(
+ featureConfiguration
+ .getToolForAction(linkTargetType.getActionName())
+ .getToolPath(cppConfiguration.getCrosstoolTopPathFragment())
+ .getPathString());
}
+ argv.addAll(
+ featureConfiguration.getCommandLine(
+ actionName,
+ new Variables.Builder()
+ .addAll(variables)
+ .addStringSequenceVariable(
+ CppLinkActionBuilder.LEGACY_LINK_FLAGS_VARIABLE, getToolchainFlags())
+ .build()));
return argv;
}
@@ -635,7 +609,7 @@
private final ActionOwner owner;
private final RuleContext ruleContext;
- @Nullable private String toolPath;
+ private String forcedToolPath;
@Nullable private Artifact output;
private ImmutableList<Artifact> buildInfoHeaderArtifacts = ImmutableList.of();
private Iterable<? extends LinkerInput> linkerInputs = ImmutableList.of();
@@ -714,7 +688,7 @@
return new LinkCommandLine(
actionName,
- toolPath,
+ forcedToolPath,
configuration,
owner,
output,
@@ -751,9 +725,9 @@
return this;
}
- /** Sets the tool path, with tool being the first thing on the command line */
- public Builder setToolPath(String toolPath) {
- this.toolPath = toolPath;
+ /** Use given tool path instead of the one from feature configuration */
+ public Builder forceToolPath(String forcedToolPath) {
+ this.forcedToolPath = forcedToolPath;
return this;
}
diff --git a/tools/osx/crosstool/CROSSTOOL.tpl b/tools/osx/crosstool/CROSSTOOL.tpl
index b1eab6b..5faa884 100644
--- a/tools/osx/crosstool/CROSSTOOL.tpl
+++ b/tools/osx/crosstool/CROSSTOOL.tpl
@@ -1178,7 +1178,7 @@
config_name: "c++-link-executable"
action_name: "c++-link-executable"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "cc_wrapper.sh"
}
implies: "symbol_counts"
implies: "linkstamps"
@@ -1197,7 +1197,7 @@
config_name: "c++-link-dynamic-library"
action_name: "c++-link-dynamic-library"
tool {
- tool_path: "wrapped_clang"
+ tool_path: "cc_wrapper.sh"
}
implies: "has_configured_linker_path"
implies: "symbol_counts"
@@ -1269,7 +1269,8 @@
config_name: "c++-link-interface-dynamic-library"
action_name: "c++-link-interface-dynamic-library"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "wrapped_clang"
+ execution_requirement: "requires-darwin"
}
implies: "strip_debug_symbols"
implies: "apple_env"
@@ -2439,7 +2440,7 @@
config_name: "c++-link-executable"
action_name: "c++-link-executable"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "cc_wrapper.sh"
}
implies: "symbol_counts"
implies: "linkstamps"
@@ -2458,7 +2459,7 @@
config_name: "c++-link-dynamic-library"
action_name: "c++-link-dynamic-library"
tool {
- tool_path: "wrapped_clang"
+ tool_path: "cc_wrapper.sh"
}
implies: "has_configured_linker_path"
implies: "symbol_counts"
@@ -2530,7 +2531,8 @@
config_name: "c++-link-interface-dynamic-library"
action_name: "c++-link-interface-dynamic-library"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "wrapped_clang"
+ execution_requirement: "requires-darwin"
}
implies: "strip_debug_symbols"
implies: "apple_env"
@@ -3702,7 +3704,7 @@
config_name: "c++-link-executable"
action_name: "c++-link-executable"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "cc_wrapper.sh"
}
implies: "symbol_counts"
implies: "linkstamps"
@@ -3721,7 +3723,7 @@
config_name: "c++-link-dynamic-library"
action_name: "c++-link-dynamic-library"
tool {
- tool_path: "wrapped_clang"
+ tool_path: "cc_wrapper.sh"
}
implies: "has_configured_linker_path"
implies: "symbol_counts"
@@ -3793,7 +3795,8 @@
config_name: "c++-link-interface-dynamic-library"
action_name: "c++-link-interface-dynamic-library"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "wrapped_clang"
+ execution_requirement: "requires-darwin"
}
implies: "strip_debug_symbols"
implies: "apple_env"
@@ -5000,7 +5003,7 @@
config_name: "c++-link-executable"
action_name: "c++-link-executable"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "cc_wrapper.sh"
}
implies: "symbol_counts"
implies: "linkstamps"
@@ -5020,7 +5023,7 @@
config_name: "c++-link-dynamic-library"
action_name: "c++-link-dynamic-library"
tool {
- tool_path: "wrapped_clang"
+ tool_path: "cc_wrapper.sh"
}
implies: "has_configured_linker_path"
implies: "symbol_counts"
@@ -5093,7 +5096,8 @@
config_name: "c++-link-interface-dynamic-library"
action_name: "c++-link-interface-dynamic-library"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "wrapped_clang"
+ execution_requirement: "requires-darwin"
}
implies: "strip_debug_symbols"
implies: "apple_env"
@@ -6263,7 +6267,7 @@
config_name: "c++-link-executable"
action_name: "c++-link-executable"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "cc_wrapper.sh"
}
implies: "symbol_counts"
implies: "linkstamps"
@@ -6282,7 +6286,7 @@
config_name: "c++-link-dynamic-library"
action_name: "c++-link-dynamic-library"
tool {
- tool_path: "wrapped_clang"
+ tool_path: "cc_wrapper.sh"
}
implies: "has_configured_linker_path"
implies: "symbol_counts"
@@ -6354,7 +6358,8 @@
config_name: "c++-link-interface-dynamic-library"
action_name: "c++-link-interface-dynamic-library"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "wrapped_clang"
+ execution_requirement: "requires-darwin"
}
implies: "strip_debug_symbols"
implies: "apple_env"
@@ -7509,7 +7514,7 @@
config_name: "c++-link-executable"
action_name: "c++-link-executable"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "cc_wrapper.sh"
}
implies: "symbol_counts"
implies: "linkstamps"
@@ -7528,7 +7533,7 @@
config_name: "c++-link-dynamic-library"
action_name: "c++-link-dynamic-library"
tool {
- tool_path: "wrapped_clang"
+ tool_path: "cc_wrapper.sh"
}
implies: "has_configured_linker_path"
implies: "symbol_counts"
@@ -7600,7 +7605,8 @@
config_name: "c++-link-interface-dynamic-library"
action_name: "c++-link-interface-dynamic-library"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "wrapped_clang"
+ execution_requirement: "requires-darwin"
}
implies: "strip_debug_symbols"
implies: "apple_env"
@@ -8757,7 +8763,7 @@
config_name: "c++-link-executable"
action_name: "c++-link-executable"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "cc_wrapper.sh"
}
implies: "symbol_counts"
implies: "linkstamps"
@@ -8776,7 +8782,7 @@
config_name: "c++-link-dynamic-library"
action_name: "c++-link-dynamic-library"
tool {
- tool_path: "wrapped_clang"
+ tool_path: "cc_wrapper.sh"
}
implies: "has_configured_linker_path"
implies: "symbol_counts"
@@ -8848,7 +8854,8 @@
config_name: "c++-link-interface-dynamic-library"
action_name: "c++-link-interface-dynamic-library"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "wrapped_clang"
+ execution_requirement: "requires-darwin"
}
implies: "strip_debug_symbols"
implies: "apple_env"
@@ -10040,7 +10047,7 @@
config_name: "c++-link-executable"
action_name: "c++-link-executable"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "cc_wrapper.sh"
}
implies: "symbol_counts"
implies: "linkstamps"
@@ -10060,7 +10067,7 @@
config_name: "c++-link-dynamic-library"
action_name: "c++-link-dynamic-library"
tool {
- tool_path: "wrapped_clang"
+ tool_path: "cc_wrapper.sh"
}
implies: "has_configured_linker_path"
implies: "symbol_counts"
@@ -10133,7 +10140,8 @@
config_name: "c++-link-interface-dynamic-library"
action_name: "c++-link-interface-dynamic-library"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "wrapped_clang"
+ execution_requirement: "requires-darwin"
}
implies: "strip_debug_symbols"
implies: "apple_env"
@@ -11288,7 +11296,7 @@
config_name: "c++-link-executable"
action_name: "c++-link-executable"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "cc_wrapper.sh"
}
implies: "symbol_counts"
implies: "linkstamps"
@@ -11307,7 +11315,7 @@
config_name: "c++-link-dynamic-library"
action_name: "c++-link-dynamic-library"
tool {
- tool_path: "wrapped_clang"
+ tool_path: "cc_wrapper.sh"
}
implies: "has_configured_linker_path"
implies: "symbol_counts"
@@ -11379,7 +11387,8 @@
config_name: "c++-link-interface-dynamic-library"
action_name: "c++-link-interface-dynamic-library"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "wrapped_clang"
+ execution_requirement: "requires-darwin"
}
implies: "strip_debug_symbols"
implies: "apple_env"
@@ -12540,7 +12549,7 @@
config_name: "c++-link-executable"
action_name: "c++-link-executable"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "cc_wrapper.sh"
}
implies: "symbol_counts"
implies: "linkstamps"
@@ -12559,7 +12568,7 @@
config_name: "c++-link-dynamic-library"
action_name: "c++-link-dynamic-library"
tool {
- tool_path: "wrapped_clang"
+ tool_path: "cc_wrapper.sh"
}
implies: "has_configured_linker_path"
implies: "symbol_counts"
@@ -12631,7 +12640,8 @@
config_name: "c++-link-interface-dynamic-library"
action_name: "c++-link-interface-dynamic-library"
tool {
- tool_path: "DUMMY_TOOL"
+ tool_path: "wrapped_clang"
+ execution_requirement: "requires-darwin"
}
implies: "strip_debug_symbols"
implies: "apple_env"
