Create an output artifact that lists the enabled features for C++ com…

…pilation 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
bazelbuild · Apr 4, 2019 · 53156db · 53156db
1 parent d3ce720
commit 53156db
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 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
@@ -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 <code>cc_binary</code> rules.
@@ -1079,10 +1080,15 @@ private static void addTransitiveInfoProviders(
     NestedSet<Artifact> headerTokens =
         CcCompilationHelper.collectHeaderTokens(
             ruleContext, cppConfiguration, ccCompilationOutputs);
-    NestedSet<Artifact> filesToCompile =
-        ccCompilationOutputs.getFilesToCompile(
-            cppConfiguration.processHeadersInDependencies(),
-            toolchain.usePicForDynamicLibraries(cppConfiguration, featureConfiguration));
+
+    Map<String, NestedSet<Artifact>> 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
@@ -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<String> computeCcFlagsFromFeatureConfig(
     }
     return ImmutableList.of();
   }
+
+  /** Returns artifacts that help debug the state of C++ features for the given ruleContext. */
+  public static Map<String, NestedSet<Artifact>> createSaveFeatureStateArtifacts(
+      CppConfiguration cppConfiguration,
+      FeatureConfiguration featureConfiguration,
+      RuleContext ruleContext) {
+
+    ImmutableMap.Builder<String, NestedSet<Artifact>> 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.<Artifact>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
@@ -716,17 +716,20 @@ public static Map<String, NestedSet<Artifact>> buildOutputGroupsForEmittingCompi
       CcToolchainProvider ccToolchain,
       FeatureConfiguration featureConfiguration,
       RuleContext ruleContext) {
-    Map<String, NestedSet<Artifact>> outputGroups = new TreeMap<>();
-    outputGroups.put(OutputGroupInfo.TEMP_FILES, ccCompilationOutputs.getTemps());
+    ImmutableMap.Builder<String, NestedSet<Artifact>> 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
@@ -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
@@ -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
@@ -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
@@ -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 <<EOF
+cc_library(
+    name = "cc",
+    srcs = ["cc.cc", "cc1.cc"],
+    features = ["test_feature"],
+)
+EOF
+
+  echo 'void cc() {}' > 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"