Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make ShowIncludesFilter ignore execroot differences #6931

Closed
wants to merge 1 commit into from
Closed

Make ShowIncludesFilter ignore execroot differences #6931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1122,8 +1122,9 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext)
ShowIncludesFilter showIncludesFilterForStdout;
ShowIncludesFilter showIncludesFilterForStderr;
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
showIncludesFilterForStdout = new ShowIncludesFilter(getSourceFile().getFilename());
showIncludesFilterForStderr = new ShowIncludesFilter(getSourceFile().getFilename());
String workspaceName = actionExecutionContext.getExecRoot().getBaseName();
showIncludesFilterForStdout = new ShowIncludesFilter(getSourceFile().getFilename(), workspaceName);
showIncludesFilterForStderr = new ShowIncludesFilter(getSourceFile().getFilename(), workspaceName);
FileOutErr originalOutErr = actionExecutionContext.getFileOutErr();
FileOutErr tempOutErr = originalOutErr.childOutErr();
spawnContext = actionExecutionContext.withFileOutErr(tempOutErr);
Expand Down
17 changes: 13 additions & 4 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ public class ShowIncludesFilter {

private FilterShowIncludesOutputStream filterShowIncludesOutputStream;
private final String sourceFileName;
private final String workspaceName;

public ShowIncludesFilter(String sourceFileName) {
public ShowIncludesFilter(String sourceFileName, String workspaceName) {
this.sourceFileName = sourceFileName;
this.workspaceName = workspaceName;
}

/**
Expand All @@ -54,10 +56,12 @@ public static class FilterShowIncludesOutputStream extends FilterOutputStream {
private static final int NEWLINE = '\n';
private static final String SHOW_INCLUDES_PREFIX = "Note: including file:";
private final String sourceFileName;
private final String execRootSuffix;

public FilterShowIncludesOutputStream(OutputStream out, String sourceFileName) {
public FilterShowIncludesOutputStream(OutputStream out, String sourceFileName, String workspaceName) {
super(out);
this.sourceFileName = sourceFileName;
this.execRootSuffix = "execroot\\" + workspaceName + "\\";
}

@Override
Expand All @@ -66,7 +70,12 @@ public void write(int b) throws IOException {
if (b == NEWLINE) {
String line = buffer.toString(StandardCharsets.UTF_8.name());
if (line.startsWith(SHOW_INCLUDES_PREFIX)) {
dependencies.add(line.substring(SHOW_INCLUDES_PREFIX.length()).trim());
line = line.substring(SHOW_INCLUDES_PREFIX.length()).trim();
int index = line.indexOf(execRootSuffix);
if (index != -1) {
line = line.substring(index + execRootSuffix.length());
}
dependencies.add(line);
} else if (!line.trim().equals(sourceFileName)) {
buffer.writeTo(out);
}
Expand Down Expand Up @@ -94,7 +103,7 @@ public Collection<String> getDependencies() {

public FilterOutputStream getFilteredOutputStream(OutputStream outputStream) {
filterShowIncludesOutputStream =
new FilterShowIncludesOutputStream(outputStream, sourceFileName);
new FilterShowIncludesOutputStream(outputStream, sourceFileName, workspaceName);
return filterShowIncludesOutputStream;
}

Expand Down
15 changes: 14 additions & 1 deletion src/test/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ 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();
Expand Down Expand Up @@ -92,6 +92,19 @@ public void testMatchAllOfNotePrefix() throws IOException {
assertThat(showIncludesFilter.getDependencies()).contains("bar.h");
}

@Test
public void testMatchAllOfNotePrefixWithAbsolutePath() throws IOException {
// "Note: including file:" is the prefix
filterOutputStream.write(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("bar.h");
}

@Test
public void testMatchSourceFileName() throws IOException {
filterOutputStream.write(getBytes("foo.cpp\n"));
Expand Down