From 4fdd66db55d36c923b1e9fa30a158e1596408670 Mon Sep 17 00:00:00 2001 From: Manuel Klimek Date: Fri, 22 Jan 2016 15:18:53 +0000 Subject: Allow switching on header processing (parse_headers or preprocess_headers) for targets in the transitive closure of a target that is built. Previously, this would only happen if a link action for the library containing the headers was also built; this specifically means it did not trigger if a library didn't contain source files, as there is no link action for such libraries. That led to no header-only libraries would get their headers parsed, which includes all cc_public_library rules. Adding a flag to introduce this under so we can switch it on independently from the blaze release. Once it is switched on, we can remove the edges in the action graph from the link actions to the header processing actions. -- MOS_MIGRATED_REVID=112782219 --- .../devtools/build/lib/rules/cpp/CcBinary.java | 47 +++++++++++++++------- .../devtools/build/lib/rules/cpp/CcCommon.java | 9 ----- .../build/lib/rules/cpp/CcCompilationOutputs.java | 15 +++++++ .../devtools/build/lib/rules/cpp/CcLibrary.java | 12 ++++-- .../build/lib/rules/cpp/CcLibraryHelper.java | 14 +++---- .../build/lib/rules/cpp/CppConfiguration.java | 7 ++++ .../devtools/build/lib/rules/cpp/CppOptions.java | 10 +++++ .../rules/cpp/CcLibraryConfiguredTargetTest.java | 33 +++++++++++++++ 8 files changed, 112 insertions(+), 35 deletions(-) (limited to 'src') 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 3afa030e1c..4e79ea0363 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 @@ -300,8 +300,17 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { RuleConfiguredTargetBuilder ruleBuilder = new RuleConfiguredTargetBuilder(ruleContext); addTransitiveInfoProviders( - ruleContext, cppConfiguration, common, ruleBuilder, filesToBuild, ccCompilationOutputs, - cppCompilationContext, linkingOutputs, dwoArtifacts, transitiveLipoInfo, fake); + ruleContext, + cppConfiguration, + common, + ruleBuilder, + filesToBuild, + ccCompilationOutputs, + cppCompilationContext, + linkingOutputs, + dwoArtifacts, + transitiveLipoInfo, + fake); Map scannableMap = new LinkedHashMap<>(); if (cppConfiguration.isLipoContextCollector()) { @@ -586,23 +595,33 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { InstrumentedFilesProvider instrumentedFilesProvider = common.getInstrumentedFilesProvider( instrumentedObjectFiles, !TargetUtils.isTestRule(ruleContext.getRule()) && !fake); + NestedSet filesToCompile = ccCompilationOutputs.getFilesToCompile( + cppConfiguration.isLipoContextCollector(), CppHelper.usePic(ruleContext, false)); builder .setFilesToBuild(filesToBuild) .add(CppCompilationContext.class, cppCompilationContext) .add(TransitiveLipoInfoProvider.class, transitiveLipoInfo) - .add(CcExecutionDynamicLibrariesProvider.class, - new CcExecutionDynamicLibrariesProvider(collectExecutionDynamicLibraryArtifacts( - ruleContext, linkingOutputs.getExecutionDynamicLibraries()))) - .add(CcNativeLibraryProvider.class, new CcNativeLibraryProvider( - collectTransitiveCcNativeLibraries(ruleContext, linkingOutputs.getDynamicLibraries()))) + .add( + CcExecutionDynamicLibrariesProvider.class, + new CcExecutionDynamicLibrariesProvider( + collectExecutionDynamicLibraryArtifacts( + ruleContext, linkingOutputs.getExecutionDynamicLibraries()))) + .add( + CcNativeLibraryProvider.class, + new CcNativeLibraryProvider( + collectTransitiveCcNativeLibraries( + ruleContext, linkingOutputs.getDynamicLibraries()))) .add(InstrumentedFilesProvider.class, instrumentedFilesProvider) - .add(CppDebugFileProvider.class, new CppDebugFileProvider( - dwoArtifacts.getDwoArtifacts(), dwoArtifacts.getPicDwoArtifacts())) - .addOutputGroup(OutputGroupProvider.TEMP_FILES, - getTemps(cppConfiguration, ccCompilationOutputs)) - .addOutputGroup(OutputGroupProvider.FILES_TO_COMPILE, - common.getFilesToCompile(ccCompilationOutputs)) - .addOutputGroup(OutputGroupProvider.COMPILATION_PREREQUISITES, + .add( + CppDebugFileProvider.class, + new CppDebugFileProvider( + dwoArtifacts.getDwoArtifacts(), dwoArtifacts.getPicDwoArtifacts())) + .addOutputGroup( + OutputGroupProvider.TEMP_FILES, getTemps(cppConfiguration, ccCompilationOutputs)) + .addOutputGroup( + OutputGroupProvider.FILES_TO_COMPILE, filesToCompile) + .addOutputGroup( + OutputGroupProvider.COMPILATION_PREREQUISITES, CcCommon.collectCompilationPrerequisites(ruleContext, cppCompilationContext)); } 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 2ede292346..49715198c4 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 @@ -27,7 +27,6 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; 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.apple.Platform; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; @@ -460,14 +459,6 @@ public final class CcCommon { CppFileTypes.LINKER_SCRIPT); } - NestedSet getFilesToCompile(CcCompilationOutputs compilationOutputs) { - if (cppConfiguration.isLipoContextCollector()) { - return NestedSetBuilder.emptySet(Order.STABLE_ORDER); - } - return NestedSetBuilder.wrap(Order.STABLE_ORDER, - compilationOutputs.getObjectFiles(CppHelper.usePic(ruleContext, false))); - } - InstrumentedFilesProvider getInstrumentedFilesProvider(Iterable files, boolean withBaselineCoverage) { return cppConfiguration.isLipoContextCollector() diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java index 67162c20d7..aad62f05e7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationOutputs.java @@ -19,6 +19,7 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.collect.nestedset.Order; import java.util.ArrayList; import java.util.LinkedHashSet; @@ -144,6 +145,20 @@ public class CcCompilationOutputs { public List getLipoScannables() { return lipoScannables; } + + /** + * Returns the output files that are considered "copmiled" by this C++ compile action. + */ + NestedSet getFilesToCompile(boolean isLipoContextCollector, boolean usePic) { + if (isLipoContextCollector) { + return NestedSetBuilder.emptySet(Order.STABLE_ORDER); + } + NestedSetBuilder files = NestedSetBuilder.stableOrder(); + files.addAll(getObjectFiles(usePic)); + files.addAll(getHeaderTokenFiles()); + return files.build(); + } + public static final class Builder { private final Set objectFiles = new LinkedHashSet<>(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java index 764b7f4441..411f9ac5e3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java @@ -231,7 +231,7 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { CcLinkingOutputs linkedLibraries = info.getCcLinkingOutputsExcludingPrecompiledLibraries(); NestedSet artifactsToForce = - collectHiddenTopLevelArtifacts(ruleContext, common, info.getCcCompilationOutputs()); + collectHiddenTopLevelArtifacts(ruleContext, info.getCcCompilationOutputs()); NestedSetBuilder filesBuilder = NestedSetBuilder.stableOrder(); filesBuilder.addAll(LinkerInputs.toLibraryArtifacts(linkedLibraries.getStaticLibraries())); @@ -268,11 +268,15 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { } - private static NestedSet collectHiddenTopLevelArtifacts(RuleContext ruleContext, - CcCommon common, CcCompilationOutputs ccCompilationOutputs) { + private static NestedSet collectHiddenTopLevelArtifacts( + RuleContext ruleContext, CcCompilationOutputs ccCompilationOutputs) { // Ensure that we build all the dependencies, otherwise users may get confused. NestedSetBuilder artifactsToForceBuilder = NestedSetBuilder.stableOrder(); - artifactsToForceBuilder.addTransitive(common.getFilesToCompile(ccCompilationOutputs)); + boolean isLipoCollector = + ruleContext.getFragment(CppConfiguration.class).isLipoContextCollector(); + boolean usePic = CppHelper.usePic(ruleContext, false); + artifactsToForceBuilder.addTransitive( + ccCompilationOutputs.getFilesToCompile(isLipoCollector, usePic)); for (OutputGroupProvider dep : ruleContext.getPrerequisites("deps", Mode.TARGET, OutputGroupProvider.class)) { artifactsToForceBuilder.addTransitive( diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index 6fd24c9659..f08baba335 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -744,7 +744,12 @@ public final class CcLibraryHelper { Map> outputGroups = new TreeMap<>(); outputGroups.put(OutputGroupProvider.TEMP_FILES, getTemps(ccOutputs)); if (emitCompileProviders) { - outputGroups.put(OutputGroupProvider.FILES_TO_COMPILE, getFilesToCompile(ccOutputs)); + boolean isLipoCollector = + ruleContext.getFragment(CppConfiguration.class).isLipoContextCollector(); + boolean usePic = CppHelper.usePic(ruleContext, false); + outputGroups.put( + OutputGroupProvider.FILES_TO_COMPILE, + ccOutputs.getFilesToCompile(isLipoCollector, usePic)); outputGroups.put(OutputGroupProvider.COMPILATION_PREREQUISITES, CcCommon.collectCompilationPrerequisites(ruleContext, cppCompilationContext)); } @@ -1003,11 +1008,4 @@ public final class CcLibraryHelper { ? NestedSetBuilder.emptySet(Order.STABLE_ORDER) : compilationOutputs.getTemps(); } - - private NestedSet getFilesToCompile(CcCompilationOutputs compilationOutputs) { - return ruleContext.getFragment(CppConfiguration.class).isLipoContextCollector() - ? NestedSetBuilder.emptySet(Order.STABLE_ORDER) - : NestedSetBuilder.wrap(Order.STABLE_ORDER, - compilationOutputs.getObjectFiles(CppHelper.usePic(ruleContext, false))); - } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index 31aa2f0fee..71045e4fd0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -1526,6 +1526,13 @@ public class CppConfiguration extends BuildConfiguration.Fragment { public boolean skipStaticOutputs() { return cppOptions.skipStaticOutputs; } + + /** + * Returns whether we are processing headers in dependencies of built C++ targets. + */ + public boolean processHeadersInDependencies() { + return cppOptions.processHeadersInDependencies; + } /** * Returns true if Fission is specified for this build and supported by the crosstool. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java index a1015979e7..683e559a46 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java @@ -244,6 +244,16 @@ public class CppOptions extends FragmentOptions { + "network and disk I/O load (and thus, continuous build cycle times) by a lot. " + "NOTE: use of this flag REQUIRES --distinct_host_configuration.") public boolean skipStaticOutputs; + + @Option( + name = "process_headers_in_dependencies", + defaultValue = "false", + category = "semantics", + help = + "When building a target //a:a, process headers in all targets that //a:a depends " + + "on (if header processing is enabled for the toolchain)." + ) + public boolean processHeadersInDependencies; @Option(name = "copt", allowMultiple = true, diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java index a11b7d7c3e..34c2786123 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java @@ -678,6 +678,39 @@ public class CcLibraryConfiguredTargetTest extends BuildViewTestCase { assertThat(getGeneratingAction(getBinArtifact("_objs/x/x/x.pic.o", x))).isNull(); } + @Test + public void testProcessHeadersInDependencies() throws Exception { + AnalysisMock.get() + .ccSupport() + .setupCrosstool(mockToolsConfig, MockCcSupport.HEADER_PROCESSING_FEATURE_CONFIGURATION); + useConfiguration("--features=parse_headers", "--process_headers_in_dependencies"); + ConfiguredTarget x = + scratchConfiguredTarget( + "foo", + "x", + "cc_library(name = 'x', deps = [':y'])", + "cc_library(name = 'y', hdrs = ['y.h'])"); + assertThat(ActionsTestUtil.baseNamesOf(getOutputGroup(x, OutputGroupProvider.HIDDEN_TOP_LEVEL))) + .isEqualTo("y.h.processed"); + } + + @Test + public void testProcessHeadersInCompileOnlyMode() throws Exception { + AnalysisMock.get() + .ccSupport() + .setupCrosstool(mockToolsConfig, MockCcSupport.HEADER_PROCESSING_FEATURE_CONFIGURATION); + useConfiguration( + "--features=parse_headers", "--process_headers_in_dependencies"); + ConfiguredTarget y = + scratchConfiguredTarget( + "foo", + "y", + "cc_library(name = 'x', deps = [':y'])", + "cc_library(name = 'y', hdrs = ['y.h'])"); + assertThat(ActionsTestUtil.baseNamesOf(getOutputGroup(y, OutputGroupProvider.FILES_TO_COMPILE))) + .isEqualTo("y.h.processed"); + } + @Test public void testIncludePathOrder() throws Exception { scratch.file("foo/BUILD", -- cgit v1.2.3