From b27a514425cf3851e89b1bbb830a201113aa59b9 Mon Sep 17 00:00:00 2001 From: hlopko Date: Fri, 20 Apr 2018 00:30:44 -0700 Subject: Thread more information through CcToolchainProvider for CcCommon.configureFeatures The goal is to enable creation of feature configuration without the rule context. This will enable us to have cleaner API for this in Skylark. RELNOTES: None PiperOrigin-RevId: 193630386 --- .../devtools/build/lib/rules/cpp/CcCommon.java | 33 +++++++++------------- .../devtools/build/lib/rules/cpp/CcToolchain.java | 19 ++++++++----- .../build/lib/rules/cpp/CcToolchainProvider.java | 28 ++++++++++++++++-- .../build/lib/rules/objc/CompilationSupport.java | 2 +- 4 files changed, 53 insertions(+), 29 deletions(-) (limited to 'src/main/java/com/google/devtools/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 94d9495911..81febbc1ea 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 @@ -673,19 +673,11 @@ public final class CcCommon { withBaselineCoverage); } - private static String getHostOrNonHostFeature(RuleContext ruleContext) { - if (ruleContext.getConfiguration().isHostConfiguration()) { - return "host"; - } else { - return "nonhost"; - } - } - - public static ImmutableList getCoverageFeatures(RuleContext ruleContext) { + public static ImmutableList getCoverageFeatures(CcToolchainProvider toolchain) { ImmutableList.Builder coverageFeatures = ImmutableList.builder(); - if (ruleContext.getConfiguration().isCodeCoverageEnabled()) { + if (toolchain.isCodeCoverageEnabled()) { coverageFeatures.add(CppRuleClasses.COVERAGE); - if (ruleContext.getFragment(CppConfiguration.class).useLLVMCoverageMapFormat()) { + if (toolchain.useLLVMCoverageMapFormat()) { coverageFeatures.add(CppRuleClasses.LLVM_COVERAGE_MAP_FORMAT); } else { coverageFeatures.add(CppRuleClasses.GCC_COVERAGE_MAP_FORMAT); @@ -704,6 +696,7 @@ public final class CcCommon { ImmutableSet requestedFeatures, ImmutableSet unsupportedFeatures, CcToolchainProvider toolchain) { + CppConfiguration cppConfiguration = toolchain.getCppConfiguration(); ImmutableSet.Builder allRequestedFeaturesBuilder = ImmutableSet.builder(); ImmutableSet.Builder unsupportedFeaturesBuilder = ImmutableSet.builder(); unsupportedFeaturesBuilder.addAll(unsupportedFeatures); @@ -742,21 +735,23 @@ public final class CcCommon { ImmutableList.Builder allFeatures = new ImmutableList.Builder() - .addAll( - ImmutableSet.of( - toolchain.getCompilationMode().toString(), - getHostOrNonHostFeature(ruleContext))) + .addAll(ImmutableSet.of(toolchain.getCompilationMode().toString())) .addAll(DEFAULT_FEATURES) .addAll(DEFAULT_ACTION_CONFIGS) .addAll(requestedFeatures) .addAll(toolchain.getFeatures().getDefaultFeaturesAndActionConfigs()); - if (CppHelper.useFission(ruleContext.getFragment(CppConfiguration.class), toolchain)) { - allFeatures.add(CppRuleClasses.PER_OBJECT_DEBUG_INFO); + + if (toolchain.isHostConfiguration()) { + allFeatures.add("host"); + } else { + allFeatures.add("nonhost"); } - CppConfiguration cppConfiguration = toolchain.getCppConfiguration(); + if (CppHelper.useFission(cppConfiguration, toolchain)) { + allFeatures.add(CppRuleClasses.PER_OBJECT_DEBUG_INFO); + } - allFeatures.addAll(getCoverageFeatures(ruleContext)); + allFeatures.addAll(getCoverageFeatures(toolchain)); if (cppConfiguration.getFdoInstrument() != null && !allUnsupportedFeatures.contains(CppRuleClasses.FDO_INSTRUMENT)) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java index 9e8d491e27..172aaf2ed1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java @@ -41,6 +41,7 @@ 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.SpawnAction; import com.google.devtools.build.lib.analysis.actions.SymlinkAction; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; @@ -311,12 +312,13 @@ public class CcToolchain implements RuleConfiguredTargetFactory { return null; } + BuildConfiguration configuration = Preconditions.checkNotNull(ruleContext.getConfiguration()); CppConfiguration cppConfiguration = - Preconditions.checkNotNull(ruleContext.getFragment(CppConfiguration.class)); + Preconditions.checkNotNull(configuration.getFragment(CppConfiguration.class)); CppToolchainInfo toolchainInfo = getCppToolchainInfo(ruleContext, cppConfiguration); PathFragment fdoZip = null; - if (ruleContext.getConfiguration().getCompilationMode() == CompilationMode.OPT) { + if (configuration.getCompilationMode() == CompilationMode.OPT) { if (cppConfiguration.getFdoPath() != null) { fdoZip = cppConfiguration.getFdoPath(); } else if (cppConfiguration.getFdoOptimizeLabel() != null) { @@ -404,8 +406,8 @@ public class CcToolchain implements RuleConfiguredTargetFactory { final NestedSet libcLink = inputsForLibc(ruleContext); String purposePrefix = Actions.escapeLabel(label) + "_"; String runtimeSolibDirBase = "_solib_" + "_" + Actions.escapeLabel(label); - final PathFragment runtimeSolibDir = ruleContext.getConfiguration() - .getBinFragment().getRelative(runtimeSolibDirBase); + final PathFragment runtimeSolibDir = + configuration.getBinFragment().getRelative(runtimeSolibDirBase); // Static runtime inputs. TransitiveInfoCollection staticRuntimeLibDep = selectDep(ruleContext, "static_runtime_libs", @@ -452,7 +454,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory { artifact, toolchainInfo.getSolibDirectory(), runtimeSolibDirBase, - ruleContext.getConfiguration())); + configuration)); } } dynamicRuntimeLinkSymlinks = dynamicRuntimeLinkSymlinksBuilder.build(); @@ -468,7 +470,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory { dynamicRuntimeLinkInputs, toolchainInfo.getSolibDirectory(), runtimeSolibDirBase, - ruleContext.getConfiguration()); + configuration); dynamicRuntimeLinkMiddleman = dynamicRuntimeLinkMiddlemanSet.isEmpty() ? null : Iterables.getOnlyElement(dynamicRuntimeLinkMiddlemanSet); } else { @@ -566,7 +568,10 @@ public class CcToolchain implements RuleConfiguredTargetFactory { : null, builtInIncludeDirectories, sysroot, - fdoMode); + fdoMode, + cppConfiguration.useLLVMCoverageMapFormat(), + configuration.isCodeCoverageEnabled(), + configuration.isHostConfiguration()); TemplateVariableInfo templateVariableInfo = createMakeVariableProvider(cppConfiguration, sysroot); 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 c9402b0b47..c4a0332881 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 @@ -82,7 +82,10 @@ public final class CcToolchainProvider extends ToolchainInfo { /* linkDynamicLibraryTool= */ null, /* builtInIncludeDirectories= */ ImmutableList.of(), /* sysroot= */ null, - FdoMode.OFF); + FdoMode.OFF, + /* useLLVMCoverageMapFormat= */ false, + /* codeCoverageEnabled= */ false, + /* isHostConfiguration= */ false); @Nullable private final CppConfiguration cppConfiguration; private final CppToolchainInfo toolchainInfo; @@ -114,6 +117,9 @@ public final class CcToolchainProvider extends ToolchainInfo { private final ImmutableList builtInIncludeDirectories; @Nullable private final PathFragment sysroot; private final FdoMode fdoMode; + private final boolean useLLVMCoverageMapFormat; + private final boolean codeCoverageEnabled; + private final boolean isHostConfiguration; public CcToolchainProvider( ImmutableMap values, @@ -146,7 +152,10 @@ public final class CcToolchainProvider extends ToolchainInfo { Artifact linkDynamicLibraryTool, ImmutableList builtInIncludeDirectories, @Nullable PathFragment sysroot, - FdoMode fdoMode) { + FdoMode fdoMode, + boolean useLLVMCoverageMapFormat, + boolean codeCoverageEnabled, + boolean isHostConfiguration) { super(values, Location.BUILTIN); this.cppConfiguration = cppConfiguration; this.toolchainInfo = toolchainInfo; @@ -178,6 +187,9 @@ public final class CcToolchainProvider extends ToolchainInfo { this.builtInIncludeDirectories = builtInIncludeDirectories; this.sysroot = sysroot; this.fdoMode = fdoMode; + this.useLLVMCoverageMapFormat = useLLVMCoverageMapFormat; + this.codeCoverageEnabled = codeCoverageEnabled; + this.isHostConfiguration = isHostConfiguration; } /** Returns c++ Make variables. */ @@ -827,5 +839,17 @@ public final class CcToolchainProvider extends ToolchainInfo { public int hashCode() { return System.identityHashCode(this); } + + public boolean useLLVMCoverageMapFormat() { + return useLLVMCoverageMapFormat; + } + + public boolean isCodeCoverageEnabled() { + return codeCoverageEnabled; + } + + public boolean isHostConfiguration() { + return isHostConfiguration; + } } 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 def979fdb3..2fd613481e 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 @@ -547,7 +547,7 @@ public class CompilationSupport { activatedCrosstoolSelectables.addAll(ruleContext.getFeatures()); - activatedCrosstoolSelectables.addAll(CcCommon.getCoverageFeatures(ruleContext)); + activatedCrosstoolSelectables.addAll(CcCommon.getCoverageFeatures(toolchain)); try { return ccToolchain -- cgit v1.2.3