aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar hlopko <hlopko@google.com>2018-05-08 01:19:31 -0700
committerGravatar Copybara-Service <copybara-piper@google.com>2018-05-08 01:21:38 -0700
commit0a502ff58810d1093dffeafa7baf120f02ab29c3 (patch)
treed777c6d6ca061f52b8100c859150acda6cd7ccd1
parent0c5cc1b0253b57c4d04e324fc36536c094a77a6f (diff)
Remove cppConfiguration from LinkBuildVariables
This is to simplify the API that will eventually be exposed to Skylark. Working towards #4571. RELNOTES: None. PiperOrigin-RevId: 195785588
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java6
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java2
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java8
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java34
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java19
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java7
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java8
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java3
-rw-r--r--src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java3
-rw-r--r--src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java60
10 files changed, 59 insertions, 91 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
index c82e329733..62cbb5232c 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java
@@ -429,7 +429,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
ruleContext,
ccCompilationOutputs,
linkingMode,
- CppHelper.useFission(cppConfiguration, ccToolchain),
+ ccToolchain.useFission(),
usePic,
ltoBackendArtifacts);
Artifact dwpFile =
@@ -438,14 +438,14 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory {
// The debug package should include the dwp file only if it was explicitly requested.
Artifact explicitDwpFile = dwpFile;
- if (!CppHelper.useFission(cppConfiguration, ccToolchain)) {
+ if (!ccToolchain.useFission()) {
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
- && CppHelper.useFission(cppConfiguration, ccToolchain)
+ && ccToolchain.useFission()
&& cppConfiguration.buildTestDwpIsActivated()) {
filesToBuild = NestedSetBuilder.fromNestedSet(filesToBuild).add(dwpFile).build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
index 56199a7e75..e85c538ca9 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java
@@ -818,7 +818,7 @@ public final class CcCommon {
allFeatures.add("nonhost");
}
- if (CppHelper.useFission(cppConfiguration, toolchain)) {
+ if (toolchain.useFission()) {
allFeatures.add(CppRuleClasses.PER_OBJECT_DEBUG_INFO);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
index c45002281e..1d8c9528b3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -1362,9 +1362,7 @@ public final class CcCompilationHelper {
// The source action does not generate dwo when it has bitcode
// output (since it isn't generating a native object with debug
// info). In that case the LtoBackendAction will generate the dwo.
- CppHelper.shouldCreatePerObjectDebugInfo(
- cppConfiguration, ccToolchain, featureConfiguration)
- && !bitcodeOutput,
+ ccToolchain.shouldCreatePerObjectDebugInfo(featureConfiguration) && !bitcodeOutput,
isGenerateDotdFile(sourceArtifact));
break;
}
@@ -1600,9 +1598,7 @@ public final class CcCompilationHelper {
? CppHelper.getCompileOutputArtifact(ruleContext, gcnoFileName, configuration)
: null;
- boolean generateDwo =
- CppHelper.shouldCreatePerObjectDebugInfo(
- cppConfiguration, ccToolchain, featureConfiguration);
+ boolean generateDwo = ccToolchain.shouldCreatePerObjectDebugInfo(featureConfiguration);
Artifact dwoFile = generateDwo ? getDwoFile(builder.getOutputFile()) : null;
// TODO(tejohnson): Add support for ThinLTO if needed.
boolean bitcodeOutput =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
index 3e229e15fc..91ec045a89 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java
@@ -121,6 +121,8 @@ public final class CcToolchainProvider extends ToolchainInfo {
private final boolean useLLVMCoverageMapFormat;
private final boolean codeCoverageEnabled;
private final boolean isHostConfiguration;
+ private final boolean forcePic;
+ private final boolean shouldStripBinaries;
public CcToolchainProvider(
ImmutableMap<String, Object> values,
@@ -191,6 +193,13 @@ public final class CcToolchainProvider extends ToolchainInfo {
this.useLLVMCoverageMapFormat = useLLVMCoverageMapFormat;
this.codeCoverageEnabled = codeCoverageEnabled;
this.isHostConfiguration = isHostConfiguration;
+ if (cppConfiguration != null) {
+ this.forcePic = cppConfiguration.forcePic();
+ this.shouldStripBinaries = cppConfiguration.shouldStripBinaries();
+ } else {
+ this.forcePic = false;
+ this.shouldStripBinaries = false;
+ }
}
/** Returns c++ Make variables. */
@@ -249,6 +258,23 @@ public final class CcToolchainProvider extends ToolchainInfo {
return result.build();
}
+ /**
+ * Returns true if Fission is specified and supported by the CROSSTOOL for the build implied by
+ * the given configuration and toolchain.
+ */
+ public boolean useFission() {
+ return Preconditions.checkNotNull(cppConfiguration).fissionIsActiveForCurrentCompilationMode()
+ && supportsFission();
+ }
+
+ /**
+ * Returns true if Fission and PER_OBJECT_DEBUG_INFO are specified and supported by the CROSSTOOL
+ * for the build implied by the given configuration, toolchain and feature configuration.
+ */
+ public boolean shouldCreatePerObjectDebugInfo(FeatureConfiguration featureConfiguration) {
+ return useFission() && featureConfiguration.isEnabled(CppRuleClasses.PER_OBJECT_DEBUG_INFO);
+ }
+
@Override
public void addGlobalMakeVariables(ImmutableMap.Builder<String, String> globalMakeEnvBuilder) {
globalMakeEnvBuilder.putAll(
@@ -1021,5 +1047,13 @@ public final class CcToolchainProvider extends ToolchainInfo {
public boolean isHostConfiguration() {
return isHostConfiguration;
}
+
+ public boolean getForcePic() {
+ return forcePic;
+ }
+
+ public boolean getShouldStripBinaries() {
+ return shouldStripBinaries;
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index 9025818edc..61e7b379c5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -1081,23 +1081,4 @@ public class CppHelper {
return toolchain.supportsInterfaceSharedObjects() && config.getUseInterfaceSharedObjects();
}
- /**
- * Returns true if Fission is specified and supported by the CROSSTOOL for the build implied by
- * the given configuration and toolchain.
- */
- public static boolean useFission(CppConfiguration config, CcToolchainProvider toolchain) {
- return config.fissionIsActiveForCurrentCompilationMode() && toolchain.supportsFission();
- }
-
- /**
- * Returns true if Fission and PER_OBJECT_DEBUG_INFO are specified and supported by the CROSSTOOL
- * for the build implied by the given configuration, toolchain and feature configuration.
- */
- public static boolean shouldCreatePerObjectDebugInfo(
- CppConfiguration config,
- CcToolchainProvider toolchain,
- FeatureConfiguration featureConfiguration) {
- return useFission(config, toolchain)
- && featureConfiguration.isEnabled(CppRuleClasses.PER_OBJECT_DEBUG_INFO);
- }
}
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 83df6d0f23..40af3b8e87 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
@@ -489,8 +489,7 @@ public class CppLinkActionBuilder {
toolchain,
fdoSupport,
usePicForLtoBackendActions,
- CppHelper.shouldCreatePerObjectDebugInfo(
- cppConfiguration, toolchain, featureConfiguration),
+ toolchain.shouldCreatePerObjectDebugInfo(featureConfiguration),
argv)
: new LtoBackendArtifacts(
ltoOutputRootPrefix,
@@ -503,8 +502,7 @@ public class CppLinkActionBuilder {
toolchain,
fdoSupport,
usePicForLtoBackendActions,
- CppHelper.shouldCreatePerObjectDebugInfo(
- cppConfiguration, toolchain, featureConfiguration),
+ toolchain.shouldCreatePerObjectDebugInfo(featureConfiguration),
argv);
return ltoArtifact;
}
@@ -1004,7 +1002,6 @@ public class CppLinkActionBuilder {
thinltoMergedObjectFile,
mustKeepDebug,
symbolCounts,
- cppConfiguration,
toolchain,
featureConfiguration,
useTestOnlyFlags,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java
index 7dd22fc247..826bf6ad10 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariables.java
@@ -101,7 +101,6 @@ public enum LinkBuildVariables {
Artifact thinltoMergedObjectFile,
boolean mustKeepDebug,
Artifact symbolCounts,
- CppConfiguration cppConfiguration,
CcToolchainProvider ccToolchainProvider,
FeatureConfiguration featureConfiguration,
boolean useTestOnlyFlags,
@@ -123,17 +122,16 @@ public enum LinkBuildVariables {
}
// pic
- if (cppConfiguration.forcePic()) {
+ if (ccToolchainProvider.getForcePic()) {
buildVariables.addStringVariable(FORCE_PIC.getVariableName(), "");
}
- if (!mustKeepDebug && cppConfiguration.shouldStripBinaries()) {
+ if (!mustKeepDebug && ccToolchainProvider.getShouldStripBinaries()) {
buildVariables.addStringVariable(STRIP_DEBUG_SYMBOLS.getVariableName(), "");
}
if (isUsingLinkerNotArchiver
- && CppHelper.shouldCreatePerObjectDebugInfo(
- cppConfiguration, ccToolchainProvider, featureConfiguration)) {
+ && ccToolchainProvider.shouldCreatePerObjectDebugInfo(featureConfiguration)) {
buildVariables.addStringVariable(IS_USING_FISSION.getVariableName(), "");
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
index aa6de48b3a..69ff3d9834 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaBinary.java
@@ -139,8 +139,7 @@ public class JavaBinary implements RuleConfiguredTargetFactory {
CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext);
// TODO(b/64384912): Remove in favor of CcToolchainProvider
boolean stripAsDefault =
- CppHelper.useFission(cppConfiguration, ccToolchain)
- && cppConfiguration.getCompilationMode() == CompilationMode.OPT;
+ ccToolchain.useFission() && cppConfiguration.getCompilationMode() == CompilationMode.OPT;
DeployArchiveBuilder unstrippedDeployArchiveBuilder = null;
if (stripAsDefault) {
unstrippedDeployArchiveBuilder = new DeployArchiveBuilder(semantics, ruleContext);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index 71ba13cf01..7b36226405 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -103,7 +103,6 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfig
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension;
import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
import com.google.devtools.build.lib.rules.cpp.CppCompileAction;
-import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
import com.google.devtools.build.lib.rules.cpp.CppHelper;
import com.google.devtools.build.lib.rules.cpp.CppLinkAction;
@@ -542,7 +541,7 @@ public class CompilationSupport {
if (objcProvider.is(Flag.USES_OBJC)) {
activatedCrosstoolSelectables.add(CONTAINS_OBJC);
}
- if (CppHelper.useFission(ruleContext.getFragment(CppConfiguration.class), toolchain)) {
+ if (toolchain.useFission()) {
activatedCrosstoolSelectables.add(CppRuleClasses.PER_OBJECT_DEBUG_INFO);
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
index 53f572b0b3..afa366beef 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java
@@ -180,101 +180,65 @@ public class CcToolchainTest extends BuildViewTestCase {
CcToolchainProvider toolchainProvider =
(CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isFalse();
+ assertThat(toolchainProvider.useFission()).isFalse();
// Mode-specific settings.
useConfiguration("-c", "dbg", "--fission=dbg");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isTrue();
+ assertThat(toolchainProvider.useFission()).isTrue();
useConfiguration("-c", "dbg", "--fission=opt");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isFalse();
+ assertThat(toolchainProvider.useFission()).isFalse();
useConfiguration("-c", "dbg", "--fission=opt,dbg");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isTrue();
+ assertThat(toolchainProvider.useFission()).isTrue();
useConfiguration("-c", "fastbuild", "--fission=opt,dbg");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isFalse();
+ assertThat(toolchainProvider.useFission()).isFalse();
useConfiguration("-c", "fastbuild", "--fission=opt,dbg");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isFalse();
+ assertThat(toolchainProvider.useFission()).isFalse();
// Universally enabled
useConfiguration("-c", "dbg", "--fission=yes");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isTrue();
+ assertThat(toolchainProvider.useFission()).isTrue();
useConfiguration("-c", "opt", "--fission=yes");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isTrue();
+ assertThat(toolchainProvider.useFission()).isTrue();
useConfiguration("-c", "fastbuild", "--fission=yes");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isTrue();
+ assertThat(toolchainProvider.useFission()).isTrue();
// Universally disabled
useConfiguration("-c", "dbg", "--fission=no");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isFalse();
+ assertThat(toolchainProvider.useFission()).isFalse();
useConfiguration("-c", "opt", "--fission=no");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isFalse();
+ assertThat(toolchainProvider.useFission()).isFalse();
useConfiguration("-c", "fastbuild", "--fission=no");
target = getConfiguredTarget("//a:b");
toolchainProvider = (CcToolchainProvider) target.get(ToolchainInfo.PROVIDER);
- assertThat(
- CppHelper.useFission(
- getConfiguration(target).getFragment(CppConfiguration.class), toolchainProvider))
- .isFalse();
+ assertThat(toolchainProvider.useFission()).isFalse();
}
@Test