From 53156db1e30fe7197889c3cbc8d1321ff9b85143 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 4 Apr 2019 00:21:45 -0700 Subject: [PATCH] Create an output artifact that lists the enabled features for C++ compilation I'm using this locally to debug some feature stuff and figured it was worth trying to check in. Let me know if I need to write tests and what not for it. RELNOTES: Allow debugging C++ features logic. PiperOrigin-RevId: 241879825 --- .../build/lib/rules/cpp/CcBinary.java | 20 +++++----- .../build/lib/rules/cpp/CcCommon.java | 39 +++++++++++++++++++ .../lib/rules/cpp/CcCompilationHelper.java | 13 ++++--- .../build/lib/rules/cpp/CppConfiguration.java | 4 ++ .../build/lib/rules/cpp/CppOptions.java | 9 +++++ .../cpp/CcLibraryConfiguredTargetTest.java | 10 +++++ src/test/shell/bazel/cc_integration_test.sh | 20 ++++++++++ 7 files changed, 101 insertions(+), 14 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 6db0ac1d01cb4b..0949e6f9ac2f40 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 @@ -70,6 +70,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.List; +import java.util.Map; /** * A ConfiguredTarget for cc_binary rules. @@ -1079,10 +1080,15 @@ private static void addTransitiveInfoProviders( NestedSet headerTokens = CcCompilationHelper.collectHeaderTokens( ruleContext, cppConfiguration, ccCompilationOutputs); - NestedSet filesToCompile = - ccCompilationOutputs.getFilesToCompile( - cppConfiguration.processHeadersInDependencies(), - toolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration)); + + Map> outputGroups = + CcCompilationHelper.buildOutputGroupsForEmittingCompileProviders( + ccCompilationOutputs, + ccCompilationContext, + cppConfiguration, + toolchain, + featureConfiguration, + ruleContext); builder .setFilesToBuild(filesToBuild) @@ -1096,15 +1102,11 @@ private static void addTransitiveInfoProviders( CppDebugFileProvider.class, new CppDebugFileProvider( dwoArtifacts.getDwoArtifacts(), dwoArtifacts.getPicDwoArtifacts())) - .addOutputGroup(OutputGroupInfo.TEMP_FILES, ccCompilationOutputs.getTemps()) - .addOutputGroup(OutputGroupInfo.FILES_TO_COMPILE, filesToCompile) // For CcBinary targets, we only want to ensure that we process headers in dependencies and // thus only add header tokens to HIDDEN_TOP_LEVEL. If we add all HIDDEN_TOP_LEVEL artifacts // from dependent CcLibrary targets, we'd be building .pic.o files in nopic builds. .addOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL, headerTokens) - .addOutputGroup( - OutputGroupInfo.COMPILATION_PREREQUISITES, - CcCommon.collectCompilationPrerequisites(ruleContext, ccCompilationContext)); + .addOutputGroups(outputGroups); CppHelper.maybeAddStaticLinkMarkerProvider(builder, ruleContext); } 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 22c6fdae075bff..447539b84bbf10 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 @@ -25,8 +25,10 @@ import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.FileProvider; import com.google.devtools.build.lib.analysis.MakeVariableSupplier; +import com.google.devtools.build.lib.analysis.OutputGroupInfo; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; +import com.google.devtools.build.lib.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.platform.ToolchainInfo; @@ -1001,4 +1003,41 @@ private static List computeCcFlagsFromFeatureConfig( } return ImmutableList.of(); } + + /** Returns artifacts that help debug the state of C++ features for the given ruleContext. */ + public static Map> createSaveFeatureStateArtifacts( + CppConfiguration cppConfiguration, + FeatureConfiguration featureConfiguration, + RuleContext ruleContext) { + + ImmutableMap.Builder> outputGroupsBuilder = ImmutableMap.builder(); + + if (cppConfiguration.saveFeatureState()) { + Artifact enabledFeaturesFile = + ruleContext.getUniqueDirectoryArtifact("feature_debug", "enabled_features.txt"); + ruleContext.registerAction( + FileWriteAction.create( + ruleContext, + enabledFeaturesFile, + featureConfiguration.getEnabledFeatureNames().toString(), + /* makeExecutable= */ false)); + + Artifact requestedFeaturesFile = + ruleContext.getUniqueDirectoryArtifact("feature_debug", "requested_features.txt"); + ruleContext.registerAction( + FileWriteAction.create( + ruleContext, + requestedFeaturesFile, + featureConfiguration.getRequestedFeatures().toString(), + /* makeExecutable= */ false)); + + outputGroupsBuilder.put( + OutputGroupInfo.DEFAULT, + NestedSetBuilder.stableOrder() + .add(enabledFeaturesFile) + .add(requestedFeaturesFile) + .build()); + } + return outputGroupsBuilder.build(); + } } 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 04035db7780521..e1ddaf1104713e 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 @@ -716,17 +716,20 @@ public static Map> buildOutputGroupsForEmittingCompi CcToolchainProvider ccToolchain, FeatureConfiguration featureConfiguration, RuleContext ruleContext) { - Map> outputGroups = new TreeMap<>(); - outputGroups.put(OutputGroupInfo.TEMP_FILES, ccCompilationOutputs.getTemps()); + ImmutableMap.Builder> outputGroupsBuilder = ImmutableMap.builder(); + outputGroupsBuilder.put(OutputGroupInfo.TEMP_FILES, ccCompilationOutputs.getTemps()); boolean processHeadersInDependencies = cppConfiguration.processHeadersInDependencies(); boolean usePic = ccToolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration); - outputGroups.put( + outputGroupsBuilder.put( OutputGroupInfo.FILES_TO_COMPILE, ccCompilationOutputs.getFilesToCompile(processHeadersInDependencies, usePic)); - outputGroups.put( + outputGroupsBuilder.put( OutputGroupInfo.COMPILATION_PREREQUISITES, CcCommon.collectCompilationPrerequisites(ruleContext, ccCompilationContext)); - return outputGroups; + outputGroupsBuilder.putAll( + CcCommon.createSaveFeatureStateArtifacts( + cppConfiguration, featureConfiguration, ruleContext)); + return outputGroupsBuilder.build(); } @Immutable 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 45da17b6a49cf4..c797ca13161795 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 @@ -657,4 +657,8 @@ boolean isThisHostConfigurationDoNotUseWillBeRemovedFor129045294() { public boolean enableCcToolchainResolution() { return cppOptions.enableCcToolchainResolution; } + + public boolean saveFeatureState() { + return cppOptions.saveFeatureState; + } } 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 cc18e94282d9e0..9322ef4056de83 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 @@ -856,6 +856,15 @@ public Label getFdoPrefetchHintsLabel() { help = "If true, cc rules use toolchain resolution to find the cc_toolchain.") public boolean enableCcToolchainResolution; + @Option( + name = "experimental_save_feature_state", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.OUTPUT_SELECTION, + effectTags = {OptionEffectTag.AFFECTS_OUTPUTS}, + metadataTags = {OptionMetadataTag.EXPERIMENTAL}, + help = "Save the state of enabled and requested feautres as an output of compilation.") + public boolean saveFeatureState; + @Override public FragmentOptions getHost() { CppOptions host = (CppOptions) getDefault(); 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 b288bc1e063462..4b6f5df505714a 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 @@ -261,6 +261,16 @@ public void testFilesToBuildWithInterfaceSharedObjects() throws Exception { .containsExactly(implSharedObjectLink); } + @Test + public void testFilesToBuildWithSaveFeatureState() throws Exception { + useConfiguration("--experimental_save_feature_state"); + ConfiguredTarget hello = getConfiguredTarget("//hello:hello"); + Artifact archive = getBinArtifact("libhello.a", hello); + assertThat(getFilesToBuild(hello)).containsExactly(archive); + assertThat(ActionsTestUtil.baseArtifactNames(getOutputGroup(hello, OutputGroupInfo.DEFAULT))) + .containsAllOf("enabled_features.txt", "requested_features.txt"); + } + @Test public void testEmptyLinkopts() throws Exception { ConfiguredTarget hello = getConfiguredTarget("//hello:hello"); diff --git a/src/test/shell/bazel/cc_integration_test.sh b/src/test/shell/bazel/cc_integration_test.sh index 1b557619150511..c41a8da266f5e4 100755 --- a/src/test/shell/bazel/cc_integration_test.sh +++ b/src/test/shell/bazel/cc_integration_test.sh @@ -257,4 +257,24 @@ EOF bazel run //dynamic_lookup:main || fail "Run of the binary failed." } +function test_save_feature_state() { + mkdir -p ea + cat > ea/BUILD < ea/cc.cc + echo 'void cc1() {}' > ea/cc1.cc + + bazel build --experimental_save_feature_state //ea:cc || fail "expected success" + ls bazel-bin/ea/feature_debug/cc/requested_features.txt || "requested_features.txt not created" + ls bazel-bin/ea/feature_debug/cc/enabled_features.txt || "enabled_features.txt not created" + # This assumes "grep" is supported in any environment bazel is used. + grep "test_feature" bazel-bin/ea/feature_debug/cc/requested_features.txt || "test_feature should have been found in requested_features." +} + run_suite "cc_integration_test"