Make ShowIncludesFilter ignore execroot differences

    Fixes bazelbuild/bazel#6847

    This change is for making including scanning work on Windows for builds with remote caching or remote execution enabled.

    After this change, the ShowIncludesFilter will look for the first `execroot\<workspace_name>` in the output header file paths, then it considers `C:\...\execroot\<workspace_name>` as the execroot path. Because execroot path could be different if remote cache is hit, we ignore it and only add the relative path as dependencies.

    I'm quite unwilling to make this change, because parsing `execroot\\<workspace_name>` for execroot is not guaranteed to work always. But considering the only case this could go wrong is when people use an output base that already contains `execroot\\<workspace_name>`, which I think should never happen.

    Closes #6931.

    Change-Id: Ife2cb91c75f1b5b297851400e672db2b35ff09e0
    PiperOrigin-RevId: 225553627

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java

@@ -236,7 +236,7 @@ public class CppCompileAction extends AbstractAction
       @Nullable Artifact grepIncludes) {
     super(
         owner,
-        createInputsBuilder(mandatoryInputs, inputsForInvalidation).build(),
+        NestedSetBuilder.fromNestedSet(mandatoryInputs).addAll(inputsForInvalidation).build(),
         CollectionUtils.asSetWithoutNulls(
             outputFile,
             dotdFile == null ? null : dotdFile.artifact(),
@@ -274,12 +274,14 @@ public class CppCompileAction extends AbstractAction
     this.actionName = actionName;
     this.featureConfiguration = featureConfiguration;
     this.needsDotdInputPruning =
-        cppSemantics.needsDotdInputPruning() && !sourceFile.isFileType(CppFileTypes.CPP_MODULE);
+        !shareable
+            && cppSemantics.needsDotdInputPruning()
+            && !sourceFile.isFileType(CppFileTypes.CPP_MODULE);
     this.needsIncludeValidation = cppSemantics.needsIncludeValidation();
     this.includeProcessing = cppSemantics.getIncludeProcessing();
     this.actionClassId = actionClassId;
     this.builtInIncludeDirectories = builtInIncludeDirectories;
-    this.additionalInputs = null;
+    this.additionalInputs = discoversInputs() ? null : ImmutableList.of();
     this.usedModules = null;
     this.topLevelModules = null;
     this.overwrittenVariables = null;
@@ -345,7 +347,9 @@ public Iterable<Artifact> getAdditionalInputs() {
 
   /** Clears the discovered {@link #additionalInputs}. */
   public void clearAdditionalInputs() {
-    additionalInputs = null;
+    if (discoversInputs()) {
+      additionalInputs = null;
+    }
   }
 
   @Override
@@ -937,22 +941,16 @@ private static boolean isDeclaredIn(
     }
   }
 
-  /**
-   * Recalculates this action's live input collection, including sources, middlemen.
-   *
-   * <p>Can only be called if {@link #discoversInputs}, and must be called after execution in that
-   * case.
-   */
+  /** Recalculates this action's live input collection, including sources, middlemen. */
   @VisibleForTesting // productionVisibility = Visibility.PRIVATE
   @ThreadCompatible
-  final void updateActionInputs(NestedSet<Artifact> discoveredInputs) {
-    Preconditions.checkState(
-        discoversInputs(), "Can't call if not discovering inputs: %s %s", discoveredInputs, this);
+  public final void updateActionInputs(NestedSet<Artifact> discoveredInputs) {
+    NestedSetBuilder<Artifact> inputs = NestedSetBuilder.stableOrder();
     try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.ACTION_UPDATE, describe())) {
-      super.updateInputs(
-          createInputsBuilder(mandatoryInputs, inputsForInvalidation)
-              .addTransitive(discoveredInputs)
-              .build());
+      inputs.addTransitive(mandatoryInputs);
+      inputs.addAll(inputsForInvalidation);
+      inputs.addTransitive(discoveredInputs);
+      super.updateInputs(inputs.build());
     }
   }
 
@@ -1116,7 +1114,7 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext)
     setModuleFileFlags();
     CppCompileActionContext.Reply reply;
 
-    if (!shouldScanDotdFiles()) {
+    if (discoversInputs() && !shouldScanDotdFiles()) {
       updateActionInputs(NestedSetBuilder.wrap(Order.STABLE_ORDER, additionalInputs));
     }
 
@@ -1211,17 +1209,9 @@ public void close() throws IOException {
     }
     reply = null; // Clear in-memory .d files early.
 
-    if (discoversInputs()) {
-      // Post-execute "include scanning", which modifies the action inputs to match what the compile
-      // action actually used by incorporating the results of .d file parsing.
-      updateActionInputs(discoveredInputs);
-    } else {
-      Preconditions.checkState(
-          discoveredInputs.isEmpty(),
-          "Discovered inputs without discovering inputs? %s %s",
-          discoveredInputs,
-          this);
-    }
+    // Post-execute "include scanning", which modifies the action inputs to match what the compile
+    // action actually used by incorporating the results of .d file parsing.
+    updateActionInputs(discoveredInputs);
 
     // hdrs_check: This cannot be switched off for C++ build actions,
     // because doing so would allow for incorrect builds.
@@ -1497,13 +1487,6 @@ private static ActionLookupData lookupDataFromModule(
             module));
   }
 
-  private static NestedSetBuilder<Artifact> createInputsBuilder(
-      NestedSet<Artifact> mandatoryInputs, Iterable<Artifact> inputsForInvalidation) {
-    return NestedSetBuilder.<Artifact>stableOrder()
-        .addTransitive(mandatoryInputs)
-        .addAll(inputsForInvalidation);
-  }
-
   /**
    * A reference to a .d file. There are two modes:
    *

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java

@@ -15,7 +15,6 @@
 package com.google.devtools.build.lib.rules.cpp;
 
 import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.vfs.Path;
 import java.io.ByteArrayOutputStream;
 import java.io.FilterOutputStream;
@@ -25,7 +24,6 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.List;
 
 /**
  * A Class for filtering the output of /showIncludes from MSVC compiler.
@@ -56,98 +54,7 @@ public static class FilterShowIncludesOutputStream extends FilterOutputStream {
     private final ByteArrayOutputStream buffer = new ByteArrayOutputStream(4096);
     private final Collection<String> dependencies = new ArrayList<>();
     private static final int NEWLINE = '\n';
-    // "Note: including file:" in 14 languages,
-    // cl.exe will print different prefix according to the locale configured for MSVC.
-    private static final List<String> SHOW_INCLUDES_PREFIXES =
-        ImmutableList.of(
-            new String(
-                new byte[] {
-                  78, 111, 116, 101, 58, 32, 105, 110, 99, 108, 117, 100, 105, 110, 103, 32, 102,
-                  105, 108, 101, 58
-                },
-                StandardCharsets.UTF_8), // English
-            new String(
-                new byte[] {
-                  -26, -77, -88, -26, -124, -113, 58, 32, -27, -116, -123, -27, -112, -85, -26, -86,
-                  -108, -26, -95, -120, 58
-                },
-                StandardCharsets.UTF_8), // Traditional Chinese
-            new String(
-                new byte[] {
-                  80, 111, 122, 110, -61, -95, 109, 107, 97, 58, 32, 86, -60, -115, 101, 116, 110,
-                  -60, -101, 32, 115, 111, 117, 98, 111, 114, 117, 58
-                },
-                StandardCharsets.UTF_8), // Czech
-            new String(
-                new byte[] {
-                  72, 105, 110, 119, 101, 105, 115, 58, 32, 69, 105, 110, 108, 101, 115, 101, 110,
-                  32, 100, 101, 114, 32, 68, 97, 116, 101, 105, 58
-                },
-                StandardCharsets.UTF_8), // German
-            new String(
-                new byte[] {
-                  82, 101, 109, 97, 114, 113, 117, 101, -62, -96, 58, 32, 105, 110, 99, 108, 117,
-                  115, 105, 111, 110, 32, 100, 117, 32, 102, 105, 99, 104, 105, 101, 114, -62, -96,
-                  58
-                },
-                StandardCharsets.UTF_8), // French
-            new String(
-                new byte[] {
-                  78, 111, 116, 97, 58, 32, 102, 105, 108, 101, 32, 105, 110, 99, 108, 117, 115, 111
-                },
-                StandardCharsets.UTF_8), // Italian
-            new String(
-                new byte[] {
-                  -29, -125, -95, -29, -125, -94, 58, 32, -29, -126, -92, -29, -125, -77, -29, -126,
-                  -81, -29, -125, -85, -29, -125, -68, -29, -125, -119, 32, -29, -125, -107, -29,
-                  -126, -95, -29, -126, -92, -29, -125, -85, 58
-                },
-                StandardCharsets.UTF_8), // Janpanese
-            new String(
-                new byte[] {
-                  -20, -80, -72, -22, -77, -96, 58, 32, -19, -113, -84, -19, -107, -88, 32, -19,
-                  -116, -116, -20, -99, -68, 58
-                },
-                StandardCharsets.UTF_8), // Korean
-            new String(
-                new byte[] {
-                  85, 119, 97, 103, 97, 58, 32, 119, 32, 116, 121, 109, 32, 112, 108, 105, 107, 117,
-                  58
-                },
-                StandardCharsets.UTF_8), // Polish
-            new String(
-                new byte[] {
-                  79, 98, 115, 101, 114, 118, 97, -61, -89, -61, -93, 111, 58, 32, 105, 110, 99,
-                  108, 117, 105, 110, 100, 111, 32, 97, 114, 113, 117, 105, 118, 111, 58
-                },
-                StandardCharsets.UTF_8), // Portuguese
-            new String(
-                new byte[] {
-                  -48, -97, -47, -128, -48, -72, -48, -68, -48, -75, -47, -121, -48, -80, -48, -67,
-                  -48, -72, -48, -75, 58, 32, -48, -78, -48, -70, -48, -69, -47, -114, -47, -121,
-                  -48, -75, -48, -67, -48, -72, -48, -75, 32, -47, -124, -48, -80, -48, -71, -48,
-                  -69, -48, -80, 58
-                },
-                StandardCharsets.UTF_8), // Russian
-            new String(
-                new byte[] {
-                  78, 111, 116, 58, 32, 101, 107, 108, 101, 110, 101, 110, 32, 100, 111, 115, 121,
-                  97, 58
-                },
-                StandardCharsets.UTF_8), // Turkish
-            new String(
-                new byte[] {
-                  -26, -77, -88, -26, -124, -113, 58, 32, -27, -116, -123, -27, -112, -85, -26,
-                  -106, -121, -28, -69, -74, 58
-                },
-                StandardCharsets.UTF_8), // Simplified Chinese
-            new String(
-                new byte[] {
-                  78, 111, 116, 97, 58, 32, 105, 110, 99, 108, 117, 115, 105, -61, -77, 110, 32,
-                  100, 101, 108, 32, 97, 114, 99, 104, 105, 118, 111, 58
-                },
-                StandardCharsets.UTF_8) // Spanish
-            );
+    private static final String SHOW_INCLUDES_PREFIX = "Note: including file:";
     private final String sourceFileName;
     private final String execRootSuffix;
 
@@ -163,21 +70,14 @@ public void write(int b) throws IOException {
       buffer.write(b);
       if (b == NEWLINE) {
         String line = buffer.toString(StandardCharsets.UTF_8.name());
-        boolean prefixMatched = false;
-        for (String prefix : SHOW_INCLUDES_PREFIXES) {
-          if (line.startsWith(prefix)) {
-            line = line.substring(prefix.length()).trim();
-            int index = line.indexOf(execRootSuffix);
-            if (index != -1) {
-              line = line.substring(index + execRootSuffix.length());
-            }
-            dependencies.add(line);
-            prefixMatched = true;
-            break;
+        if (line.startsWith(SHOW_INCLUDES_PREFIX)) {
+          line = line.substring(SHOW_INCLUDES_PREFIX.length()).trim();
+          int index = line.indexOf(execRootSuffix);
+          if (index != -1) {
+            line = line.substring(index + execRootSuffix.length());
           }
-        }
-        // cl.exe also prints out the file name unconditionally, we need to also filter it out.
-        if (!prefixMatched && !line.trim().equals(sourceFileName)) {
+          dependencies.add(line);
+        } else if (!line.trim().equals(sourceFileName)) {
           buffer.writeTo(out);
         }
         buffer.reset();
@@ -187,19 +87,9 @@ public void write(int b) throws IOException {
     @Override
     public void flush() throws IOException {
       String line = buffer.toString(StandardCharsets.UTF_8.name());
-
-      // If this line starts or could start with a prefix.
-      boolean startingWithAnyPrefix = false;
-      for (String prefix : SHOW_INCLUDES_PREFIXES) {
-        if (line.startsWith(prefix) || prefix.startsWith(line)) {
-          startingWithAnyPrefix = true;
-          break;
-        }
-      }
-
-      if (!startingWithAnyPrefix
-          // If this line starts or could start with the source file name.
+      if (!line.startsWith(SHOW_INCLUDES_PREFIX)
           && !line.startsWith(sourceFileName)
+          && !SHOW_INCLUDES_PREFIX.startsWith(line)
           && !sourceFileName.startsWith(line)) {
         buffer.writeTo(out);
         buffer.reset();

dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java

@@ -17,7 +17,6 @@
 import static com.google.common.truth.Truth.assertThat;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
-import com.google.devtools.build.lib.vfs.DigestHashFunction;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
 import java.io.ByteArrayOutputStream;
@@ -39,10 +38,10 @@ public class ShowIncludesFilterTest {
 
   @Before
   public void setUpOutputStreams() throws IOException {
-    showIncludesFilter = new ShowIncludesFilter("foo.cpp");
+    showIncludesFilter = new ShowIncludesFilter("foo.cpp", "__main__");
     output = new ByteArrayOutputStream();
     filterOutputStream = showIncludesFilter.getFilteredOutputStream(output);
-    fs = new InMemoryFileSystem(DigestHashFunction.SHA256);
+    fs = new InMemoryFileSystem();
     fs.getPath("/out").createDirectory();
   }
 
@@ -94,31 +93,17 @@ public void testMatchAllOfNotePrefix() throws IOException {
   }
 
   @Test
-  // Regression tests for https://github.com/bazelbuild/bazel/issues/9172
-  public void testFindHeaderFromAbsolutePathUnderExecrootBase() throws IOException {
+  public void testMatchAllOfNotePrefixWithAbsolutePath() throws IOException {
     // "Note: including file:" is the prefix
     filterOutputStream.write(
-        getBytes("Note: including file: C:\\tmp\\xxxx\\execroot\\__main__\\foo\\bar\\bar.h"));
+        getBytes("Note: including file: C:\\tmp\\xxxx\\execroot\\__main__\\bar.h"));
     filterOutputStream.flush();
     // flush to output should not work, waiting for newline
     assertThat(output.toString()).isEmpty();
     filterOutputStream.write(getBytes("\n"));
     // It's a match, output should be filtered, dependency on bar.h should be found.
     assertThat(output.toString()).isEmpty();
-    assertThat(showIncludesFilter.getDependencies()).contains("..\\__main__\\foo\\bar\\bar.h");
-  }
-
-  @Test
-  public void testFindHeaderFromAbsolutePathOutsideExecroot() throws IOException {
-    // "Note: including file:" is the prefix
-    filterOutputStream.write(getBytes("Note: including file: C:\\system\\foo\\bar\\bar.h"));
-    filterOutputStream.flush();
-    // flush to output should not work, waiting for newline
-    assertThat(output.toString()).isEmpty();
-    filterOutputStream.write(getBytes("\n"));
-    // It's a match, output should be filtered, dependency on bar.h should be found.
-    assertThat(output.toString()).isEmpty();
-    assertThat(showIncludesFilter.getDependencies()).contains("C:\\system\\foo\\bar\\bar.h");
+    assertThat(showIncludesFilter.getDependencies()).contains("bar.h");
   }
 
   @Test