Skip to content

Commit

Permalink
Allow setting supports_start_end_lib crosstool field using feature
Browse files Browse the repository at this point in the history
This cl allows toolchain owners to express that toolchain supports --start-lib/--end-lib using enabled feature "supports_start_end_lib".

This cl is a step towards #5883. Also
see the rollout doc here:
https://docs.google.com/document/d/1uv4c1zag6KvdI31qdx8C6jiTognXPQrxgsUpVefm9fM/edit#.

Flag removing legacy behavior is #6861

RELNOTES: None.
PiperOrigin-RevId: 226292303
  • Loading branch information
hlopko authored and Copybara-Service committed Dec 20, 2018
1 parent e56be0d commit 0762948
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 16 deletions.
14 changes: 12 additions & 2 deletions site/docs/crosstool-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,7 @@ The following is a reference of `CROSSTOOL` build variables.



### CROSSTOOL features
### Well-known features

The following is a reference of `CROSSTOOL` features and their activation
conditions.
Expand All @@ -950,7 +950,7 @@ conditions.
<tr>
<td><strong>Feature</strong>
</td>
<td><strong>Activation Condition</strong>
<td><strong>Documentation</strong>
</td>
</tr>
<tr>
Expand Down Expand Up @@ -993,4 +993,14 @@ conditions.
requested.
</td>
</tr>
<tr>
<td><strong><code>supports_start_end_lib</code></strong>
</td>
<td>If enabled (and the option <code>--start_end_lib</code> is set), Bazel
will not link against static libraries but instead use the
<code>--start-lib/--end-lib</code> linker options to link against objects
directly. This speeds up the build since Bazel doesn't have to build
static libraries.
</td>
</tr>
</table>
Original file line number Diff line number Diff line change
Expand Up @@ -577,8 +577,9 @@ public boolean supportsExecOrigin() {
}

/** Returns whether the toolchain supports the --start-lib/--end-lib options. */
public boolean supportsStartEndLib() {
return toolchainInfo.supportsStartEndLib();
public boolean supportsStartEndLib(FeatureConfiguration featureConfiguration) {
return toolchainInfo.supportsStartEndLib()
|| featureConfiguration.isEnabled(CppRuleClasses.SUPPORTS_START_END_LIB);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -810,23 +810,28 @@ public static Artifact createDefFileActions(
* Returns true if the build implied by the given config and toolchain uses --start-lib/--end-lib
* ld options.
*/
public static boolean useStartEndLib(CppConfiguration config, CcToolchainProvider toolchain) {
return config.startEndLibIsRequested() && toolchain.supportsStartEndLib();
public static boolean useStartEndLib(
CppConfiguration config,
CcToolchainProvider toolchain,
FeatureConfiguration featureConfiguration) {
return config.startEndLibIsRequested() && toolchain.supportsStartEndLib(featureConfiguration);
}

/**
* Returns the type of archives being used by the build implied by the given config and toolchain.
*/
public static Link.ArchiveType getArchiveType(
CppConfiguration config, CcToolchainProvider toolchain) {
return useStartEndLib(config, toolchain)
CppConfiguration config,
CcToolchainProvider toolchain,
FeatureConfiguration featureConfiguration) {
return useStartEndLib(config, toolchain, featureConfiguration)
? Link.ArchiveType.START_END_LIB
: Link.ArchiveType.REGULAR;
}

/**
* Returns true if interface shared objects should be used in the build implied by the given
* config and toolchain.
* cppConfiguration and toolchain.
*/
public static boolean useInterfaceSharedObjects(
CppConfiguration config, CcToolchainProvider toolchain) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,9 @@ public static Label ccToolchainTypeAttribute(RuleDefinitionEnvironment env) {
/** A string constant for is_cc_fake_binary feature. */
public static final String IS_CC_FAKE_BINARY = "is_cc_fake_binary";

/** A feature marking that the toolchain can use --start-lib/--end-lib flags */
public static final String SUPPORTS_START_END_LIB = "supports_start_end_lib";

/** Ancestor for all rules that do include scanning. */
public static final class CcIncludeScanningRule implements RuleDefinition {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,8 @@ private void addDynamicInputLinkOptions(
|| input.getArtifactCategory() == ArtifactCategory.INTERFACE_LIBRARY);
Preconditions.checkState(
!Link.useStartEndLib(
input, CppHelper.getArchiveType(cppConfiguration, ccToolchainProvider)));
input,
CppHelper.getArchiveType(cppConfiguration, ccToolchainProvider, featureConfiguration)));
if (featureConfiguration.isEnabled(CppRuleClasses.TARGETS_WINDOWS)
&& ccToolchainProvider.supportsInterfaceSharedObjects()) {
// On Windows, dynamic library (dll) cannot be linked directly when using toolchains that
Expand Down Expand Up @@ -379,7 +380,8 @@ private void addStaticInputLinkOptions(

// start-lib/end-lib library: adds its input object files.
if (Link.useStartEndLib(
input, CppHelper.getArchiveType(cppConfiguration, ccToolchainProvider))) {
input,
CppHelper.getArchiveType(cppConfiguration, ccToolchainProvider, featureConfiguration))) {
Iterable<Artifact> archiveMembers = input.getObjectFiles();
if (!Iterables.isEmpty(archiveMembers)) {
ImmutableList.Builder<Artifact> nonLtoArchiveMembersBuilder = ImmutableList.builder();
Expand Down Expand Up @@ -506,7 +508,8 @@ private Map<Artifact, Artifact> generateLtoMap() {
// As a bonus, we can rephrase --nostart_end_lib as --features=-start_end_lib and get rid
// of a command line option.

Preconditions.checkState(CppHelper.useStartEndLib(cppConfiguration, ccToolchainProvider));
Preconditions.checkState(
CppHelper.useStartEndLib(cppConfiguration, ccToolchainProvider, featureConfiguration));
Map<Artifact, Artifact> ltoMap = new HashMap<>();
for (LtoBackendArtifacts l : allLtoArtifacts) {
ltoMap.put(l.getBitcodeFile(), l.getObjectFile());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,10 @@ public boolean apply(Artifact artifact) {
+ "}";

public static final String COPY_DYNAMIC_LIBRARIES_TO_BINARY_CONFIGURATION =
"" + "feature { " + " name: 'copy_dynamic_libraries_to_binary'" + "}";
"" + "feature { name: 'copy_dynamic_libraries_to_binary' }";

public static final String SUPPORTS_START_END_LIB_FEATURE =
"" + "feature {" + " name: 'supports_start_end_lib'" + " enabled: true" + "}";

public static final String TARGETS_WINDOWS_CONFIGURATION =
""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.packages.util.MockCcSupport;
import com.google.devtools.build.lib.rules.core.CoreRules;
import com.google.devtools.build.lib.rules.platform.PlatformRules;
import com.google.devtools.build.lib.rules.repository.CoreWorkspaceRules;
Expand Down Expand Up @@ -279,6 +280,26 @@ public void testStartEndLib() throws Exception {
}
}

@Test
public void testStartEndLibThroughFeature() throws Exception {
AnalysisMock.get()
.ccSupport()
.setupCrosstool(mockToolsConfig, MockCcSupport.SUPPORTS_START_END_LIB_FEATURE);
useConfiguration("--start_end_lib");
scratch.file(
"test/BUILD",
"cc_library(name='lib', srcs=['lib.c'])",
"cc_binary(name='bin', srcs=['bin.c'])");

ConfiguredTarget target = getConfiguredTarget("//test:bin");
CppLinkAction action = (CppLinkAction) getGeneratingAction(getExecutable(target));
for (Artifact input : action.getInputs()) {
String name = input.getFilename();
assertThat(!CppFileTypes.ARCHIVE.matches(name) && !CppFileTypes.PIC_ARCHIVE.matches(name))
.isTrue();
}
}

@Test
public void testTempsWithDifferentExtensions() throws Exception {
useConfiguration("--cpu=k8", "--save_temps");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool;
import com.google.devtools.build.lib.rules.cpp.Link.LinkingMode;
import com.google.devtools.build.lib.testutil.TestConstants;
Expand Down Expand Up @@ -175,7 +176,7 @@ public void testSimpleCompleteConfiguration() throws Exception {
assertThat(ccProvider.getAbiGlibcVersion()).isEqualTo("abi-libc-version");

assertThat(ccProvider.supportsGoldLinker()).isTrue();
assertThat(ccProvider.supportsStartEndLib()).isFalse();
assertThat(ccProvider.supportsStartEndLib(FeatureConfiguration.EMPTY)).isFalse();
assertThat(ccProvider.supportsInterfaceSharedObjects()).isFalse();
assertThat(ccProvider.supportsEmbeddedRuntimes()).isFalse();
assertThat(ccProvider.toolchainNeedsPic()).isFalse();
Expand Down Expand Up @@ -458,7 +459,7 @@ public void testComprehensiveCompleteConfiguration() throws Exception {
assertThat(ccProviderA.getToolPathFragment(Tool.STRIP))
.isEqualTo(getToolPath("path/to/strip-A"));
assertThat(ccProviderA.supportsGoldLinker()).isTrue();
assertThat(ccProviderA.supportsStartEndLib()).isTrue();
assertThat(ccProviderA.supportsStartEndLib(FeatureConfiguration.EMPTY)).isTrue();
assertThat(ccProviderA.supportsEmbeddedRuntimes()).isTrue();
assertThat(ccProviderA.toolchainNeedsPic()).isTrue();

Expand Down Expand Up @@ -559,7 +560,7 @@ public void testComprehensiveCompleteConfiguration() throws Exception {
assertThat(ccProviderC.getAbiGlibcVersion()).isEqualTo("abi-libc-version-C");
// Don't bother with testing the list of tools again.
assertThat(ccProviderC.supportsGoldLinker()).isFalse();
assertThat(ccProviderC.supportsStartEndLib()).isFalse();
assertThat(ccProviderC.supportsStartEndLib(FeatureConfiguration.EMPTY)).isFalse();
assertThat(ccProviderC.supportsInterfaceSharedObjects()).isFalse();
assertThat(ccProviderC.supportsEmbeddedRuntimes()).isFalse();
assertThat(ccProviderC.toolchainNeedsPic()).isFalse();
Expand Down

0 comments on commit 0762948

Please sign in to comment.