Skip to content

Commit

Permalink
Allow setting supports_fission crosstool capability using feature
Browse files Browse the repository at this point in the history
`supports_fission` can now be expressed using 'per_object_debug_info' feature (should be enabled for it to take effect).

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: 226950450
  • Loading branch information
hlopko authored and Copybara-Service committed Dec 26, 2018
1 parent 867b1c5 commit c2e8bcb
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.Runfiles.Builder;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction;
Expand All @@ -44,6 +45,8 @@
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder;
import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression;
import com.google.devtools.build.lib.rules.java.JavaCcLinkParamsProvider;
Expand Down Expand Up @@ -760,10 +763,12 @@ public Pair<Artifact, Artifact> getLauncher(
JavaCommon common,
DeployArchiveBuilder deployArchiveBuilder,
DeployArchiveBuilder unstrippedDeployArchiveBuilder,
Runfiles.Builder runfilesBuilder,
Builder runfilesBuilder,
List<String> jvmFlags,
JavaTargetAttributes.Builder attributesBuilder,
boolean shouldStrip) {
boolean shouldStrip,
CcToolchainProvider ccToolchain,
FeatureConfiguration featureConfiguration) {
Artifact launcher = JavaHelper.launcherArtifactForTarget(this, ruleContext);
return new Pair<>(launcher, launcher);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont
ruleContext,
ccCompilationOutputs,
linkingMode,
ccToolchain.useFission(),
ccToolchain.shouldCreatePerObjectDebugInfo(featureConfiguration),
usePic,
ccLinkingOutputsBinary.getAllLtoArtifacts());
Artifact dwpFile =
Expand All @@ -484,14 +484,13 @@ public static ConfiguredTarget init(CppSemantics semantics, RuleContext ruleCont

// The debug package should include the dwp file only if it was explicitly requested.
Artifact explicitDwpFile = dwpFile;
if (!ccToolchain.useFission()) {
if (!ccToolchain.shouldCreatePerObjectDebugInfo(featureConfiguration)) {
explicitDwpFile = null;
} else {
// For cc_test rules, include the dwp in the runfiles if Fission is enabled and the test was
// built statically.
if (TargetUtils.isTestRule(ruleContext.getRule())
&& linkingMode != Link.LinkingMode.DYNAMIC
&& ccToolchain.useFission()
&& cppConfiguration.buildTestDwpIsActivated()) {
filesToBuild = NestedSetBuilder.fromNestedSet(filesToBuild).add(dwpFile).build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ public static FeatureConfiguration configureFeaturesOrThrowEvalException(
allFeatures.add("nonhost");
}

if (toolchain.useFission()) {
if (toolchain.useFission() && !cppConfiguration.disableLegacyCrosstoolFields()) {
allFeatures.add(CppRuleClasses.PER_OBJECT_DEBUG_INFO);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.java;

import static com.google.devtools.build.lib.rules.cpp.CppRuleClasses.STATIC_LINKING_MODE;
import static com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression.COMPRESSED;
import static com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression.UNCOMPRESSED;

import com.google.common.base.Optional;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.actions.Artifact;
Expand All @@ -40,6 +42,8 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.rules.cpp.CcCommon;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppHelper;
Expand Down Expand Up @@ -140,9 +144,18 @@ public ConfiguredTarget create(RuleContext ruleContext)
ruleContext.getConfiguration().getFragment(CppConfiguration.class);
CcToolchainProvider ccToolchain =
CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext);
// TODO(b/64384912): Remove in favor of CcToolchainProvider
FeatureConfiguration featureConfiguration =
CcCommon.configureFeaturesOrReportRuleError(
ruleContext,
/* requestedFeatures= */ ImmutableSet.<String>builder()
.addAll(ruleContext.getFeatures())
.add(STATIC_LINKING_MODE)
.build(),
/* unsupportedFeatures= */ ruleContext.getDisabledFeatures(),
ccToolchain);
boolean stripAsDefault =
ccToolchain.useFission() && cppConfiguration.getCompilationMode() == CompilationMode.OPT;
ccToolchain.shouldCreatePerObjectDebugInfo(featureConfiguration)
&& cppConfiguration.getCompilationMode() == CompilationMode.OPT;
DeployArchiveBuilder unstrippedDeployArchiveBuilder = null;
if (stripAsDefault) {
unstrippedDeployArchiveBuilder = new DeployArchiveBuilder(semantics, ruleContext);
Expand All @@ -156,7 +169,9 @@ public ConfiguredTarget create(RuleContext ruleContext)
runfilesBuilder,
jvmFlags,
attributesBuilder,
stripAsDefault);
stripAsDefault,
ccToolchain,
featureConfiguration);
Artifact launcher = launcherAndUnstrippedLauncher.first;
Artifact unstrippedLauncher = launcherAndUnstrippedLauncher.second;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.Runfiles.Builder;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.Substitution.ComputedSubstitution;
Expand All @@ -38,6 +39,8 @@
import com.google.devtools.build.lib.packages.Attribute.LabelListLateBoundDefault;
import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction;
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider.ClasspathType;
import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaOptimizationMode;
Expand Down Expand Up @@ -421,6 +424,8 @@ ImmutableList<Artifact> translate(
* @param jvmFlags the list of flags to pass to the JVM when running the Java binary (mutable).
* @param attributesBuilder the builder to construct the list of attributes of this target
* (mutable).
* @param ccToolchain to be used to build the launcher
* @param featureConfiguration to be used to configure the cc toolchain
* @return the launcher and unstripped launcher as an artifact pair. If shouldStrip is false, then
* they will be the same.
* @throws InterruptedException
Expand All @@ -430,10 +435,12 @@ Pair<Artifact, Artifact> getLauncher(
final JavaCommon common,
DeployArchiveBuilder deployArchiveBuilder,
DeployArchiveBuilder unstrippedDeployArchiveBuilder,
Runfiles.Builder runfilesBuilder,
Builder runfilesBuilder,
List<String> jvmFlags,
JavaTargetAttributes.Builder attributesBuilder,
boolean shouldStrip)
boolean shouldStrip,
CcToolchainProvider ccToolchain,
FeatureConfiguration featureConfiguration)
throws InterruptedException, RuleErrorException;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ private FeatureConfiguration getFeatureConfiguration(
if (objcProvider.is(Flag.USES_OBJC)) {
activatedCrosstoolSelectables.add(CONTAINS_OBJC);
}
if (toolchain.useFission()) {
if (toolchain.useFission() && !toolchain.getCppConfiguration().disableLegacyCrosstoolFields()) {
activatedCrosstoolSelectables.add(CppRuleClasses.PER_OBJECT_DEBUG_INFO);
}

Expand Down
1 change: 1 addition & 0 deletions src/main/protobuf/crosstool_config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ message CToolchain {
optional bool supports_incremental_linker = 41 [default = false];
// Legacy field, ignored by Bazel.
optional bool supports_normalizing_ar = 26 [default = false];
// Legacy field, use 'per_object_debug_info' feature instead.
optional bool supports_fission = 43 [default = false];
// Legacy field, ignored by Bazel.
optional bool supports_dsym = 51 [default = false];
Expand Down

0 comments on commit c2e8bcb

Please sign in to comment.