Skip to content

Commit

Permalink
Use tool from action_config for link-executable and link-dynamic-lib …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
hlopko authored and philwo committed Jun 21, 2017
1 parent c9b6f4a commit a81264e
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 107 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public static String getCppActionConfigs(
" 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'",
Expand All @@ -136,7 +136,7 @@ public static String getCppActionConfigs(
" 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'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -755,9 +755,15 @@ public CppLinkAction build() throws InterruptedException {
.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)
Expand Down Expand Up @@ -889,26 +895,6 @@ public CppLinkAction build() throws InterruptedException {
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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -80,7 +80,7 @@ public final class LinkCommandLine extends CommandLine {

private LinkCommandLine(
String actionName,
String toolPath,
String forcedToolPath,
BuildConfiguration configuration,
ActionOwner owner,
Artifact output,
Expand All @@ -103,7 +103,7 @@ private LinkCommandLine(
CcToolchainProvider ccProvider) {

this.actionName = actionName;
this.toolPath = toolPath;
this.forcedToolPath = forcedToolPath;
this.codeCoverageEnabled = configuration.isCodeCoverageEnabled();
this.cppConfiguration = configuration.getFragment(CppConfiguration.class);
this.variables = variables;
Expand Down Expand Up @@ -320,6 +320,9 @@ public static void extractArgumentsForDynamicLinkParamFile(
}

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 =
Expand Down Expand Up @@ -379,52 +382,23 @@ private ImmutableList<String> getToolchainFlags() {
*/
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;
}

Expand Down Expand Up @@ -635,7 +609,7 @@ public static final class Builder {
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();
Expand Down Expand Up @@ -714,7 +688,7 @@ public LinkCommandLine build() {

return new LinkCommandLine(
actionName,
toolPath,
forcedToolPath,
configuration,
owner,
output,
Expand Down Expand Up @@ -751,9 +725,9 @@ public Builder setFdoSupport(FdoSupport fdoSupport) {
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;
}

Expand Down
Loading

0 comments on commit a81264e

Please sign in to comment.