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 3045127154a6a1..3bd149bad55d5e 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 @@ -1710,6 +1710,22 @@ private NestedSet discoverInputsFromShowIncludesFilters( throws ActionExecutionException { Collection stdoutDeps = showIncludesFilterForStdout.getDependencies(execRoot); Collection stderrDeps = showIncludesFilterForStderr.getDependencies(execRoot); + if ((stdoutDeps.isEmpty() + && showIncludesFilterForStdout.sawPotentialUnsupportedShowIncludesLine()) + || (stderrDeps.isEmpty() + && showIncludesFilterForStderr.sawPotentialUnsupportedShowIncludesLine())) { + // /showIncludes parsing didn't result in any headers being found (unusual) *and* also + // encountered a line that looked like /showIncludes output in an unsupported encoding. + String message = + "While parsing the C++ compiler output for information about included headers, Bazel " + + "failed to find any headers but encountered a line that appears to be " + + "/showIncludes output in an unsupported encoding. This can result in incorrect " + + "incremental builds. If you are using the default Windows MSVC toolchain that " + + "ships with Bazel, ensure that the English language pack for Visual Studio is " + + "installed and then execute 'bazel clean --expunge'."; + DetailedExitCode code = createDetailedExitCode(message, Code.FIND_USED_HEADERS_IO_EXCEPTION); + throw new ActionExecutionException(message, this, /* catastrophe= */ false, code); + } return HeaderDiscovery.discoverInputsFromDependencies( this, getSourceFile(), diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java index 5f457fa4358152..53bf5eda6bf237 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java @@ -148,10 +148,19 @@ public static class FilterShowIncludesOutputStream extends FilterOutputStream { StandardCharsets.UTF_8) // Spanish ); private final String sourceFileName; + private boolean sawPotentialUnsupportedShowIncludesLine; // Grab everything under the execroot base so that external repository header files are covered // in the sibling repository layout. private static final Pattern EXECROOT_BASE_HEADER_PATTERN = Pattern.compile(".*execroot\\\\(?.*)"); + // Match a line of the form "fooo: bar: C:\some\path\file.h". As this is relatively generic, + // we require the line to include an absolute path with drive letter. If remote workers rewrite + // the path to a relative one, we won't match it, but it is unlikely that such setups use an + // unsupported encoding. We also exclude any matches that contain numbers: MSVC warnings and + // errors always contain numbers, but the /showIncludes output doesn't in any encoding since all + // codepages are ASCII-compatible. + private static final Pattern POTENTIAL_UNSUPPORTED_SHOW_INCLUDES_LINE = + Pattern.compile("[^:0-9]+:\\s+[^:0-9]+:\\s+[A-Za-z]:\\\\.*"); public FilterShowIncludesOutputStream(OutputStream out, String sourceFileName) { super(out); @@ -182,6 +191,15 @@ public void write(int b) throws IOException { } // cl.exe also prints out the file name unconditionally, we need to also filter it out. if (!prefixMatched && !line.trim().equals(sourceFileName)) { + // When the toolchain definition failed to force an English locale, /showIncludes lines + // can use non-UTF8 encodings, which the checks above fail to detect. As this results in + // incorrect incremental builds, we emit a warning if the raw byte sequence comprising the + // line looks like it could be a /showIncludes line. + if (POTENTIAL_UNSUPPORTED_SHOW_INCLUDES_LINE + .matcher(buffer.toString(StandardCharsets.ISO_8859_1).trim()) + .matches()) { + sawPotentialUnsupportedShowIncludesLine = true; + } buffer.writeTo(out); } buffer.reset(); @@ -214,6 +232,10 @@ public void flush() throws IOException { public Collection getDependencies() { return this.dependencies; } + + public boolean sawPotentialUnsupportedShowIncludesLine() { + return sawPotentialUnsupportedShowIncludesLine; + } } public FilterOutputStream getFilteredOutputStream(OutputStream outputStream) { @@ -232,6 +254,11 @@ public Collection getDependencies(Path root) { return Collections.unmodifiableCollection(dependenciesInPath); } + public boolean sawPotentialUnsupportedShowIncludesLine() { + return filterShowIncludesOutputStream != null + && filterShowIncludesOutputStream.sawPotentialUnsupportedShowIncludesLine(); + } + @VisibleForTesting Collection getDependencies() { return filterShowIncludesOutputStream.getDependencies(); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java index 6bfaf66058ac1b..c2e78b7cc57557 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java @@ -55,6 +55,7 @@ public void testNotMatch() throws IOException { // Normal output message with newline filterOutputStream.write(getBytes("I am compiling\n")); assertThat(output.toString()).isEqualTo("I am compiling\n"); + assertThat(showIncludesFilter.sawPotentialUnsupportedShowIncludesLine()).isFalse(); } @Test @@ -65,6 +66,7 @@ public void testNotMatchThenFlushing() throws IOException { filterOutputStream.flush(); // flush to output should succeed assertThat(output.toString()).isEqualTo("Still compiling"); + assertThat(showIncludesFilter.sawPotentialUnsupportedShowIncludesLine()).isFalse(); } @Test @@ -78,6 +80,7 @@ public void testMatchPartOfNotePrefix() throws IOException { filterOutputStream.write(getBytes("other info")); filterOutputStream.flush(); assertThat(output.toString()).isEqualTo("Note: other info"); + assertThat(showIncludesFilter.sawPotentialUnsupportedShowIncludesLine()).isFalse(); } @Test @@ -91,6 +94,7 @@ public void testMatchAllOfNotePrefix() throws IOException { // 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.sawPotentialUnsupportedShowIncludesLine()).isFalse(); } @Test @@ -106,6 +110,7 @@ public void testFindHeaderFromAbsolutePathUnderExecrootBase() throws IOException // 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"); + assertThat(showIncludesFilter.sawPotentialUnsupportedShowIncludesLine()).isFalse(); } @Test @@ -119,6 +124,7 @@ public void testFindHeaderFromAbsolutePathOutsideExecroot() throws IOException { // 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.sawPotentialUnsupportedShowIncludesLine()).isFalse(); } @Test @@ -127,6 +133,7 @@ public void testMatchSourceFileName() throws IOException { // It's a match, output should be filtered, no dependency found. assertThat(output.toString()).isEmpty(); assertThat(showIncludesFilter.getDependencies()).isEmpty(); + assertThat(showIncludesFilter.sawPotentialUnsupportedShowIncludesLine()).isFalse(); } @Test @@ -138,5 +145,21 @@ public void testMatchPartOfSourceFileName() throws IOException { filterOutputStream.write(getBytes(".h")); filterOutputStream.flush(); assertThat(output.toString()).isEqualTo("foo.h"); + assertThat(showIncludesFilter.sawPotentialUnsupportedShowIncludesLine()).isFalse(); + } + + @Test + public void testSawPotentialUnsupportedShowIncludesLine() throws IOException { + // MSVC output with French non-UTF-8 locale. + filterOutputStream.write(getBytes("Remarque")); + filterOutputStream.write(0xFF); + filterOutputStream.write(getBytes(": inclusion du fichier")); + filterOutputStream.write(0xFF); + filterOutputStream.write(getBytes(": C:\\\\foo.h\n")); + filterOutputStream.flush(); + + assertThat(output.toString()).isNotEmpty(); + assertThat(showIncludesFilter.getDependencies()).isEmpty(); + assertThat(showIncludesFilter.sawPotentialUnsupportedShowIncludesLine()).isTrue(); } }