diff options
author | hlopko <hlopko@google.com> | 2018-05-08 01:19:31 -0700 |
---|---|---|
committer | Copybara-Service <copybara-piper@google.com> | 2018-05-08 01:21:38 -0700 |
commit | 0a502ff58810d1093dffeafa7baf120f02ab29c3 (patch) | |
tree | d777c6d6ca061f52b8100c859150acda6cd7ccd1 | |
parent | 0c5cc1b0253b57c4d04e324fc36536c094a77a6f (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
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 |