Windows: Ignore workspace name differences while doing header checking

Fixes #9172

Closes #9188.

PiperOrigin-RevId: 264146358

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java

@@ -1241,11 +1241,8 @@ public ActionContinuationOrResult beginExecution(ActionExecutionContext actionEx
     ShowIncludesFilter showIncludesFilterForStdout;
     ShowIncludesFilter showIncludesFilterForStderr;
     if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
-      String workspaceName = actionExecutionContext.getExecRoot().getBaseName();
-      showIncludesFilterForStdout =
-          new ShowIncludesFilter(getSourceFile().getFilename(), workspaceName);
-      showIncludesFilterForStderr =
-          new ShowIncludesFilter(getSourceFile().getFilename(), workspaceName);
+      showIncludesFilterForStdout = new ShowIncludesFilter(getSourceFile().getFilename());
+      showIncludesFilterForStderr = new ShowIncludesFilter(getSourceFile().getFilename());
       FileOutErr originalOutErr = actionExecutionContext.getFileOutErr();
       FileOutErr tempOutErr = originalOutErr.childOutErr();
       spawnContext = actionExecutionContext.withFileOutErr(tempOutErr);

src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java

@@ -25,7 +25,8 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * A Class for filtering the output of /showIncludes from MSVC compiler.
@@ -40,11 +41,9 @@ public class ShowIncludesFilter {
 
   private FilterShowIncludesOutputStream filterShowIncludesOutputStream;
   private final String sourceFileName;
-  private final String workspaceName;
 
-  public ShowIncludesFilter(String sourceFileName, String workspaceName) {
+  public ShowIncludesFilter(String sourceFileName) {
     this.sourceFileName = sourceFileName;
-    this.workspaceName = workspaceName;
   }
 
   /**
@@ -58,7 +57,7 @@ public static class FilterShowIncludesOutputStream extends FilterOutputStream {
     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 =
+    private static final ImmutableList<String> SHOW_INCLUDES_PREFIXES =
         ImmutableList.of(
             new String(
                 new byte[] {
@@ -149,13 +148,12 @@ public static class FilterShowIncludesOutputStream extends FilterOutputStream {
                 StandardCharsets.UTF_8) // Spanish
             );
     private final String sourceFileName;
-    private final String execRootSuffix;
+    private static final Pattern EXECROOT_HEADER_PATTERN =
+        Pattern.compile(".*execroot\\\\[^\\\\]*\\\\(?<headerPath>.*)");
 
-    public FilterShowIncludesOutputStream(
-        OutputStream out, String sourceFileName, String workspaceName) {
+    public FilterShowIncludesOutputStream(OutputStream out, String sourceFileName) {
       super(out);
       this.sourceFileName = sourceFileName;
-      this.execRootSuffix = "execroot\\" + workspaceName + "\\";
     }
 
     @Override
@@ -167,9 +165,9 @@ public void write(int b) throws IOException {
         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());
+            Matcher m = EXECROOT_HEADER_PATTERN.matcher(line);
+            if (m.matches()) {
+              line = m.group("headerPath");
             }
             dependencies.add(line);
             prefixMatched = true;
@@ -214,7 +212,7 @@ public Collection<String> getDependencies() {
 
   public FilterOutputStream getFilteredOutputStream(OutputStream outputStream) {
     filterShowIncludesOutputStream =
-        new FilterShowIncludesOutputStream(outputStream, sourceFileName, workspaceName);
+        new FilterShowIncludesOutputStream(outputStream, sourceFileName);
     return filterShowIncludesOutputStream;
   }
 

src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java

@@ -38,7 +38,7 @@ public class ShowIncludesFilterTest {
 
   @Before
   public void setUpOutputStreams() throws IOException {
-    showIncludesFilter = new ShowIncludesFilter("foo.cpp", "__main__");
+    showIncludesFilter = new ShowIncludesFilter("foo.cpp");
     output = new ByteArrayOutputStream();
     filterOutputStream = showIncludesFilter.getFilteredOutputStream(output);
     fs = new InMemoryFileSystem();
@@ -93,17 +93,31 @@ public void testMatchAllOfNotePrefix() throws IOException {
   }
 
   @Test
-  public void testMatchAllOfNotePrefixWithAbsolutePath() throws IOException {
+  // Regression tests for https://github.com/bazelbuild/bazel/issues/9172
+  public void testFindHeaderFromAbsolutePathUnderExecroot() throws IOException {
     // "Note: including file:" is the prefix
     filterOutputStream.write(
-        getBytes("Note: including file: C:\\tmp\\xxxx\\execroot\\__main__\\bar.h"));
+        getBytes("Note: including file: C:\\tmp\\xxxx\\execroot\\__main__\\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("bar.h");
+    assertThat(showIncludesFilter.getDependencies()).contains("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");
   }
 
   @Test

src/test/py/bazel/bazel_windows_cpp_test.py

@@ -639,6 +639,28 @@ def testBuildWithClangClByToolchainResolution(self):
     self.AssertExitCode(exit_code, 0, stderr)
     self.assertIn('clang-cl.exe', ''.join(stderr))
 
+  def createSimpleCppWorkspace(self, name):
+    work_dir = self.ScratchDir(name)
+    self.ScratchFile(name + '/WORKSPACE', ['workspace(name = "%s")' % name])
+    self.ScratchFile(
+        name + '/BUILD',
+        ['cc_library(name = "lib", srcs = ["lib.cc"], hdrs = ["lib.h"])'])
+    self.ScratchFile(name + '/lib.h', ['void hello();'])
+    self.ScratchFile(name + '/lib.cc', ['#include "lib.h"', 'void hello() {}'])
+    return work_dir
+
+  # Regression test for https://github.com/bazelbuild/bazel/issues/9172
+  def testCacheBetweenWorkspaceWithDifferentNames(self):
+    cache_dir = self.ScratchDir('cache')
+    dir_a = self.createSimpleCppWorkspace('A')
+    dir_b = self.createSimpleCppWorkspace('B')
+    exit_code, _, stderr = self.RunBazel(
+        ['build', '--disk_cache=' + cache_dir, ':lib'], cwd=dir_a)
+    self.AssertExitCode(exit_code, 0, stderr)
+    exit_code, _, stderr = self.RunBazel(
+        ['build', '--disk_cache=' + cache_dir, ':lib'], cwd=dir_b)
+    self.AssertExitCode(exit_code, 0, stderr)
+
 
 if __name__ == '__main__':
   unittest.main()

src/test/py/bazel/test_base.py

@@ -285,7 +285,7 @@ def CopyFile(self, src_path, dst_path, executable=False):
       os.chmod(abspath, stat.S_IRWXU)
     return abspath
 
-  def RunBazel(self, args, env_remove=None, env_add=None):
+  def RunBazel(self, args, env_remove=None, env_add=None, cwd=None):
     """Runs "bazel <args>", waits for it to exit.
 
     Args:
@@ -294,14 +294,16 @@ def RunBazel(self, args, env_remove=None, env_add=None):
         to Bazel
       env_add: {string: string}; optional; environment variables to pass to
         Bazel, won't be removed by env_remove.
+      cwd: [string]; the working directory of Bazel, will be self._test_cwd if
+        not specified.
     Returns:
       (int, [string], [string]) tuple: exit code, stdout lines, stderr lines
     """
     return self.RunProgram([
         self.Rlocation('io_bazel/src/bazel'),
         '--bazelrc=' + self._test_bazelrc,
         '--nomaster_bazelrc',
-    ] + args, env_remove, env_add)
+    ] + args, env_remove, env_add, False, cwd)
 
   def StartRemoteWorker(self):
     """Runs a "local remote worker" to run remote builds and tests on.
@@ -370,7 +372,12 @@ def StopRemoteWorker(self):
       print('--------------------------')
       print('\n'.join(stderr_lines))
 
-  def RunProgram(self, args, env_remove=None, env_add=None, shell=False):
+  def RunProgram(self,
+                 args,
+                 env_remove=None,
+                 env_add=None,
+                 shell=False,
+                 cwd=None):
     """Runs a program (args[0]), waits for it to exit.
 
     Args:
@@ -381,6 +388,8 @@ def RunProgram(self, args, env_remove=None, env_add=None, shell=False):
         the program, won't be removed by env_remove.
       shell: {bool: bool}; optional; whether to use the shell as the program
         to execute
+      cwd: [string]; the current working dirctory, will be self._test_cwd if not
+        specified.
     Returns:
       (int, [string], [string]) tuple: exit code, stdout lines, stderr lines
     """
@@ -390,7 +399,7 @@ def RunProgram(self, args, env_remove=None, env_add=None, shell=False):
             args,
             stdout=stdout,
             stderr=stderr,
-            cwd=self._test_cwd,
+            cwd=(cwd if cwd else self._test_cwd),
             env=self._EnvMap(env_remove, env_add),
             shell=shell)
         exit_code = proc.wait()