From 24c980b6d4adef456cf4b4f84ac3b8ec4580b172 Mon Sep 17 00:00:00 2001 From: Googler Date: Wed, 10 Feb 2021 08:06:29 -0800 Subject: [PATCH] Make ShowIncludesFilter compatible with the sibling repository layout by grabbing everything under the execroot base and prepend the collected paths with "..\". PiperOrigin-RevId: 356735700 --- .../build/lib/rules/cpp/ShowIncludesFilter.java | 14 ++++++++++---- .../lib/rules/cpp/ShowIncludesFilterTest.java | 4 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) 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 b3a43383a4de1f..5f457fa4358152 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,8 +148,10 @@ public static class FilterShowIncludesOutputStream extends FilterOutputStream { StandardCharsets.UTF_8) // Spanish ); private final String sourceFileName; - private static final Pattern EXECROOT_HEADER_PATTERN = - Pattern.compile(".*execroot\\\\[^\\\\]*\\\\(?.*)"); + // 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\\\\(?.*)"); public FilterShowIncludesOutputStream(OutputStream out, String sourceFileName) { super(out); @@ -165,9 +167,13 @@ public void write(int b) throws IOException { for (String prefix : SHOW_INCLUDES_PREFIXES) { if (line.startsWith(prefix)) { line = line.substring(prefix.length()).trim(); - Matcher m = EXECROOT_HEADER_PATTERN.matcher(line); + Matcher m = EXECROOT_BASE_HEADER_PATTERN.matcher(line); if (m.matches()) { - line = m.group("headerPath"); + // Prefix the matched header path with "..\". This way, external repo header paths are + // resolved to "\..\\", and main repo file paths are + // resolved to "\..\
\", which is nicely normalized to + // "\". + line = "..\\" + m.group("headerPath"); } dependencies.add(line); prefixMatched = true; 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 1d7239d0ba4d59..6bfaf66058ac1b 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 @@ -95,7 +95,7 @@ public void testMatchAllOfNotePrefix() throws IOException { @Test // Regression tests for https://github.com/bazelbuild/bazel/issues/9172 - public void testFindHeaderFromAbsolutePathUnderExecroot() throws IOException { + public void testFindHeaderFromAbsolutePathUnderExecrootBase() throws IOException { // "Note: including file:" is the prefix filterOutputStream.write( getBytes("Note: including file: C:\\tmp\\xxxx\\execroot\\__main__\\foo\\bar\\bar.h")); @@ -105,7 +105,7 @@ public void testFindHeaderFromAbsolutePathUnderExecroot() throws IOException { 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("foo\\bar\\bar.h"); + assertThat(showIncludesFilter.getDependencies()).contains("..\\__main__\\foo\\bar\\bar.h"); } @Test