From 1aa75acc63cc3f663cfb27a6215a98e50a5a8cfc Mon Sep 17 00:00:00 2001
From: Laszlo Csomor <laszlocsomor@google.com>
Date: Fri, 7 Dec 2018 16:21:22 +0100
Subject: [PATCH] C++ compilation: flag for header-validation debug

Add the --[no]experimental_header_validation_debug
flag.

This flag tells Bazel to print extra debugging
information when a C++ compilation action fails
header inclusion validation.

We will enable this flag on BuildKite in hopes of
catching the culprit of https://github.com/bazelbuild/bazel/issues/6847.

After we find the culprit, this commit should be
reverted and the
--experimental_header_validation_debug flag from
BuildKite's configuration should be removed.

See https://github.com/bazelbuild/bazel/issues/6847
---
 .../build/lib/rules/cpp/CppCompileAction.java | 44 ++++++++++---------
 .../build/lib/rules/cpp/CppConfiguration.java |  4 ++
 .../build/lib/rules/cpp/CppOptions.java       | 11 +++++
 3 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
index 27a7c174497850..63d61e9e175fb1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -92,8 +92,6 @@ public class CppCompileAction extends AbstractAction
 
   private static final PathFragment BUILD_PATH_FRAGMENT = PathFragment.create("BUILD");
 
-  private static final boolean VALIDATION_DEBUG_WARN = false;
-
   protected final Artifact outputFile;
   private final Artifact sourceFile;
   private final CppConfiguration cppConfiguration;
@@ -814,27 +812,31 @@ public void validateInclusions(
         errors.add(input.getExecPath().toString());
       }
     }
-    if (VALIDATION_DEBUG_WARN) {
-      synchronized (System.err) {
+    if (cppConfiguration.getHeaderValidationDebug()) {
+      StringBuilder sb = new StringBuilder();
+      if (errors.hasProblems()) {
         if (errors.hasProblems()) {
-          if (errors.hasProblems()) {
-            System.err.println("ERROR: Include(s) were not in declared srcs:");
-          } else {
-            System.err.println("INFO: Include(s) were OK for '" + getSourceFile()
-                + "', declared srcs:");
-          }
-          for (Artifact a : ccCompilationContext.getDeclaredIncludeSrcs()) {
-            System.err.println("  '" + a.toDetailString() + "'");
-          }
-          System.err.println(" or under declared dirs:");
-          for (PathFragment f : Sets.newTreeSet(ccCompilationContext.getDeclaredIncludeDirs())) {
-            System.err.println("  '" + f + "'");
-          }
-          System.err.println(" with prefixes:");
-          for (PathFragment dirpath : ccCompilationContext.getQuoteIncludeDirs()) {
-            System.err.println("  '" + dirpath + "'");
-          }
+          sb.append("ERROR: Include(s) were NOT OK for '");
+        } else {
+          sb.append("INFO: Include(s) were OK for '");
         }
+        sb.append(getSourceFile()).append("', declared srcs:\n");
+        for (Artifact a : ccCompilationContext.getDeclaredIncludeSrcs()) {
+          sb.append("  '").append(a.toDetailString()).append("'\n");
+        }
+        sb.append(" declared dirs:\n");
+        for (PathFragment f : Sets.newTreeSet(ccCompilationContext.getDeclaredIncludeDirs())) {
+          sb.append("  '").append(f).append("'\n");
+        }
+        sb.append(" with prefixes:\n");
+        for (PathFragment dirpath : ccCompilationContext.getQuoteIncludeDirs()) {
+          sb.append("  '").append(dirpath).append("'\n");
+        }
+      }
+
+      String sbString = sb.toString();
+      synchronized (System.err) {
+        System.err.println(sbString);
       }
     }
     errors.assertProblemFree(this, getSourceFile());
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 7e9cda2318f453..ba0542223bdf42 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
@@ -553,6 +553,10 @@ public boolean disableDepsetInUserFlags() {
     return cppOptions.disableDepsetInUserFlags;
   }
 
+  public boolean getHeaderValidationDebug() {
+    return cppOptions.headerValidationDebug;
+  }
+
   public static PathFragment computeDefaultSysroot(String builtInSysroot) {
     if (builtInSysroot.isEmpty()) {
       return null;
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 ed179c987f6fc0..d3de068ea089b6 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
@@ -797,6 +797,17 @@ public Label getFdoPrefetchHintsLabel() {
       help = "If enabled, cpu transformer is not used for CppConfiguration")
   public boolean doNotUseCpuTransformer;
 
+  // TODO(laszlocsomor): revert the commit that added this flag, after we found the root cause of
+  // https://github.com/bazelbuild/bazel/issues/6847
+  @Option(
+      name = "experimental_header_validation_debug",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = {OptionEffectTag.EXECUTION},
+      metadataTags = {OptionMetadataTag.EXPERIMENTAL},
+      help = "If enabled, CppCompileAction prints extra info about failed header validation.")
+  public boolean headerValidationDebug;
+
   @Override
   public FragmentOptions getHost() {
     CppOptions host = (CppOptions) getDefault();