Skip to content

Commit

Permalink
C++: Allows adding linkopts through file.
Browse files Browse the repository at this point in the history 
The file can be generated during execution by a different rule.

RELNOTES:none
PiperOrigin-RevId: 185361140
  • Loading branch information
@oquenchil
oquenchil authored and Copybara-Service committed Feb 12, 2018
1 parent c527b4f commit c7e343a
Show file tree
Hide file tree
Showing 7 changed files with 377 additions and 1 deletion.
7 changes: 7 additions & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/rules/cpp/BazelCcBinaryRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package com.google.devtools.build.lib.bazel.rules.cpp;

import static com.google.devtools.build.lib.packages.Attribute.attr;
import static com.google.devtools.build.lib.packages.BuildType.LABEL;
import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;

import com.google.devtools.build.lib.analysis.BaseRuleClasses;
Expand All @@ -25,6 +26,7 @@
import com.google.devtools.build.lib.packages.RuleClass.Builder;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppRuleClasses;
import com.google.devtools.build.lib.util.FileType;

/** Rule definition for cc_binary rules. */
public final class BazelCcBinaryRule implements RuleDefinition {
Expand Down Expand Up @@ -73,6 +75,11 @@ public RuleClass build(Builder builder, RuleDefinitionEnvironment env) {
attr("linkshared", BOOLEAN)
.value(false)
.nonconfigurable("used to *determine* the rule's configuration"))
/*<!-- #BLAZE_RULE(cc_binary).ATTRIBUTE(linkopts_file) -->
A file with additional options to pass to the linker. The file can be generated during
execution.
<!-- #END_BLAZE_RULE.ATTRIBUTE -->*/
.add(attr("linkopts_file", LABEL).allowedFileTypes(FileType.of("linkopts_file")))
.cfg(CppRuleClasses.LIPO_ON_DEMAND)
.build();
}
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.cpp;

import static com.google.devtools.build.lib.packages.BuildType.LABEL;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -165,6 +167,7 @@ public ConfiguredTarget create(RuleContext context)
return CcBinary.init(semantics, context, /*fake =*/ false);
}

// TODO(plf): Split up this method.
public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleContext, boolean fake)
throws InterruptedException, RuleErrorException {
ruleContext.checkSrcsSamePackage(true);
Expand Down Expand Up @@ -286,6 +289,12 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
linkCompileOutputSeparately,
semantics);
linkActionBuilder.setUseTestOnlyFlags(ruleContext.isTestTarget());
if (ruleContext.isAttrDefined("linkopts_file", LABEL)) {
Artifact linkoptsFile = ruleContext.getPrerequisiteArtifact("linkopts_file", Mode.DONT_CHECK);
if (linkoptsFile != null) {
linkActionBuilder.setLinkoptsParamFile(linkoptsFile);
}
}
if (linkStaticness == LinkStaticness.DYNAMIC) {
linkActionBuilder.setRuntimeInputs(
ArtifactCategory.DYNAMIC_LIBRARY,
Expand Down
25 changes: 25 additions & 0 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ public static String getCppActionConfigs(
" implies: 'libraries_to_link'",
" implies: 'force_pic_flags'",
" implies: 'legacy_link_flags'",
" implies: 'linkopts_file'",
" implies: 'linker_param_file'",
" implies: 'fission_support'",
" implies: 'sysroot'",
Expand All @@ -462,6 +463,7 @@ public static String getCppActionConfigs(
" implies: 'library_search_directories'",
" implies: 'libraries_to_link'",
" implies: 'legacy_link_flags'",
" implies: 'linkopts_file'",
" implies: 'linker_param_file'",
" implies: 'fission_support'",
" implies: 'sysroot'",
Expand Down Expand Up @@ -1056,6 +1058,29 @@ public static String getFeaturesToAppearLastInToolchain(
" }",
" }",
"}"),
ifTrue(
!existingFeatureNames.contains("linkopts_file"),
"feature {",
" name: 'linkopts_file'",
" flag_set {",
" expand_if_all_available: 'linkopts_file'",
" action: 'c++-link-executable'",
" action: 'c++-link-dynamic-library'",
" flag_group {",
" flag: '-Wl,@%{linkopts_file}'",
" }",
" }",
" flag_set {",
" expand_if_all_available: 'linkopts_file'",
" action: 'c++-link-static-library'",
" action: 'c++-link-alwayslink-static-library'",
" action: 'c++-link-pic-static-library'",
" action: 'c++-link-alwayslink-pic-static-library'",
" flag_group {",
" flag: '@%{linkopts_file}'",
" }",
" }",
"}"),
ifTrue(
!existingFeatureNames.contains("linker_param_file"),
"feature {",
Expand Down
24 changes: 23 additions & 1 deletion src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ public class CppLinkActionBuilder {
*/
public static final String LINKER_PARAM_FILE_VARIABLE = "linker_param_file";

/** A build variable for linker flags read from a file provided by the user. */
public static final String LINK_OPTS_FILE_VARIABLE = "linkopts_file";

/** A build variable for the execpath of the output of the linker. */
public static final String OUTPUT_EXECPATH_VARIABLE = "output_execpath";

Expand Down Expand Up @@ -195,6 +198,7 @@ public class CppLinkActionBuilder {
private final ImmutableSet.Builder<Linkstamp> linkstampsBuilder = ImmutableSet.builder();
private ImmutableList<String> additionalLinkstampDefines = ImmutableList.of();
private final List<String> linkopts = new ArrayList<>();
private Artifact linkoptsFile;
private LinkTargetType linkType = LinkTargetType.STATIC_LIBRARY;
private LinkStaticness linkStaticness = LinkStaticness.FULLY_STATIC;
private String libraryIdentifier = null;
Expand Down Expand Up @@ -918,6 +922,7 @@ public CppLinkAction build() throws InterruptedException {
runtimeLinkerInputs,
/* output= */ null,
paramFile,
linkoptsFile,
thinltoParamFile,
thinltoMergedObjectFile,
ltoOutputRootPrefix,
Expand All @@ -932,6 +937,7 @@ public CppLinkAction build() throws InterruptedException {
runtimeLinkerInputs,
output,
paramFile,
linkoptsFile,
thinltoParamFile,
thinltoMergedObjectFile,
/* ltoOutputRootPrefix= */ PathFragment.EMPTY_FRAGMENT,
Expand Down Expand Up @@ -1273,7 +1279,7 @@ public CppLinkActionBuilder addVariablesExtensions(List<VariablesExtension> vari
for (VariablesExtension variablesExtension : variablesExtensions) {
addVariablesExtension(variablesExtension);
}
return this;
return this;
}

/**
Expand Down Expand Up @@ -1474,6 +1480,15 @@ public CppLinkActionBuilder addLinkParams(
return this;
}

/** Will pass a file with additional options to the linker. */
public CppLinkActionBuilder setLinkoptsParamFile(Artifact linkoptsFile) {
Preconditions.checkState(this.linkoptsFile == null);
Preconditions.checkNotNull(linkoptsFile);
this.linkoptsFile = linkoptsFile;
addActionInput(linkoptsFile);
return this;
}

/** Sets whether this link action will be used for a cc_fake_binary; false by default. */
public CppLinkActionBuilder setFake(boolean fake) {
this.fake = fake;
Expand Down Expand Up @@ -1593,6 +1608,7 @@ private class CppLinkVariablesExtension implements VariablesExtension {
private final Artifact interfaceLibraryBuilder;
private final Artifact interfaceLibraryOutput;
private final Artifact paramFile;
private final Artifact linkoptsFile;
private final Artifact thinltoParamFile;
private final Artifact thinltoMergedObjectFile;
private final PathFragment ltoOutputRootPrefix;
Expand All @@ -1607,6 +1623,7 @@ public CppLinkVariablesExtension(
ImmutableList<LinkerInput> runtimeLinkerInputs,
Artifact output,
Artifact paramFile,
Artifact linkoptsFile,
Artifact thinltoParamFile,
Artifact thinltoMergedObjectFile,
PathFragment ltoOutputRootPrefix,
Expand All @@ -1621,6 +1638,7 @@ public CppLinkVariablesExtension(
this.interfaceLibraryBuilder = interfaceLibraryBuilder;
this.interfaceLibraryOutput = interfaceLibraryOutput;
this.paramFile = paramFile;
this.linkoptsFile = linkoptsFile;
this.thinltoParamFile = thinltoParamFile;
this.thinltoMergedObjectFile = thinltoMergedObjectFile;
this.ltoOutputRootPrefix = ltoOutputRootPrefix;
Expand Down Expand Up @@ -1679,6 +1697,10 @@ public void addVariables(Variables.Builder buildVariables) {
buildVariables.addStringVariable(LINKER_PARAM_FILE_VARIABLE, paramFile.getExecPathString());
}

if (linkoptsFile != null) {
buildVariables.addStringVariable(LINK_OPTS_FILE_VARIABLE, linkoptsFile.getExecPathString());
}

// output exec path
if (outputArtifact != null) {
buildVariables.addStringVariable(
Expand Down
20 changes: 20 additions & 0 deletions src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,26 @@ public void testLinkerParamFileIsExported() throws Exception {
assertThat(variableValue).matches(".*bin/x/bin" + OsUtils.executableExtension() + "-2.params$");
}

@Test
public void testLinkoptsFileIsExported() throws Exception {
AnalysisMock.get().ccSupport().setupCrosstool(mockToolsConfig);
useConfiguration();

scratch.file(
"x/BUILD",
"cc_binary(",
" name = 'bin',",
" srcs = ['bin.cc'],",
" linkopts_file= 'bin.linkopts_file'",
")");

ConfiguredTarget target = getConfiguredTarget("//x:bin");
Variables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE);
String variableValue =
getVariableValue(variables, CppLinkActionBuilder.LINK_OPTS_FILE_VARIABLE);
assertThat(variableValue).matches("x/bin.linkopts_file");
}

@Test
public void testInterfaceLibraryBuildingVariablesWhenGenerationPossible() throws Exception {
// Make sure the interface shared object generation is enabled in the configuration
Expand Down
23 changes: 23 additions & 0 deletions tools/cpp/CROSSTOOL.tpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ toolchain {
implies: 'input_param_flags'
implies: 'legacy_link_flags'
implies: 'linker_subsystem_flag'
implies: 'linkopts_file'
implies: 'linker_param_file'
implies: 'msvc_env'
implies: 'use_linker'
Expand All @@ -443,6 +444,7 @@ toolchain {
implies: 'input_param_flags'
implies: 'legacy_link_flags'
implies: 'linker_subsystem_flag'
implies: 'linkopts_file'
implies: 'linker_param_file'
implies: 'msvc_env'
implies: 'use_linker'
Expand All @@ -460,6 +462,7 @@ toolchain {
implies: 'nologo'
implies: 'archiver_flags'
implies: 'input_param_flags'
implies: 'linkopts_file'
implies: 'linker_param_file'
implies: 'msvc_env'
}
Expand All @@ -473,6 +476,7 @@ toolchain {
implies: 'nologo'
implies: 'archiver_flags'
implies: 'input_param_flags'
implies: 'linkopts_file'
implies: 'linker_param_file'
implies: 'msvc_env'
}
Expand All @@ -488,6 +492,7 @@ toolchain {
implies: 'nologo'
implies: 'archiver_flags'
implies: 'input_param_flags'
implies: 'linkopts_file'
implies: 'linker_param_file'
implies: 'msvc_env'
}
Expand All @@ -501,6 +506,7 @@ toolchain {
implies: 'nologo'
implies: 'archiver_flags'
implies: 'input_param_flags'
implies: 'linkopts_file'
implies: 'linker_param_file'
implies: 'msvc_env'
}
Expand All @@ -512,6 +518,7 @@ toolchain {
tool_path: '%{msvc_lib_path}'
}
implies: 'nologo'
implies: 'linkopts_file'
implies: 'linker_param_file'
implies: 'msvc_env'
}
Expand Down Expand Up @@ -847,6 +854,22 @@ toolchain {
}
}

feature {
name: 'linkopts_file'
flag_set {
expand_if_all_available: 'linkopts_file'
action: 'c++-link-executable'
action: 'c++-link-dynamic-library'
action: 'c++-link-static-library'
action: 'c++-link-alwayslink-static-library'
action: 'c++-link-pic-static-library'
action: 'c++-link-alwayslink-pic-static-library'
flag_group {
flag: '@%{linkopts_file}'
}
}
}

feature {
name: 'linker_param_file'
flag_set {
Expand Down
Loading

0 comments on commit c7e343a

Please sign in to comment.