Fix error output handling on Windows
This broke in 33f7b7755db0ed5cad1b686b7616183927c72ab1, as the new code
path was not handling this case correctly.
Fixes #8402.
PiperOrigin-RevId: 249201968
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 3aeff9b..b199335 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
@@ -83,7 +83,6 @@
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
-import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
@@ -1695,44 +1694,17 @@
// executed spawn.
dotDContents = getDotDContents(spawnResults.get(0));
} catch (ExecException e) {
- if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
- closeSuppressed(e, spawnExecutionContext.getFileOutErr());
- }
+ copyTempOutErrToActionOutErr();
throw e.toActionExecutionException(
"C++ compilation of rule '" + getOwner().getLabel() + "'",
actionExecutionContext.getVerboseFailures(),
CppCompileAction.this);
} catch (InterruptedException e) {
- if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
- closeSuppressed(e, spawnExecutionContext.getFileOutErr());
- }
+ copyTempOutErrToActionOutErr();
throw e;
}
- // If parse_showincludes feature is enabled, instead of parsing dotD file we parse the
- // output of cl.exe caused by /showIncludes option.
- if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
- try {
- FileOutErr tempOutErr = spawnExecutionContext.getFileOutErr();
- FileOutErr outErr = actionExecutionContext.getFileOutErr();
- tempOutErr.close();
- if (tempOutErr.hasRecordedStdout()) {
- try (InputStream in = tempOutErr.getOutputPath().getInputStream()) {
- ByteStreams.copy(
- in,
- showIncludesFilterForStdout.getFilteredOutputStream(outErr.getOutputStream()));
- }
- }
- if (tempOutErr.hasRecordedStderr()) {
- try (InputStream in = tempOutErr.getErrorPath().getInputStream()) {
- ByteStreams.copy(
- in, showIncludesFilterForStderr.getFilteredOutputStream(outErr.getErrorStream()));
- }
- }
- } catch (IOException e) {
- throw printIOExceptionAndConvertToActionExecutionException(actionExecutionContext, e);
- }
- }
+ copyTempOutErrToActionOutErr();
ensureCoverageNotesFilesExist(actionExecutionContext);
@@ -1784,11 +1756,30 @@
return ActionContinuationOrResult.of(ActionResult.create(spawnResults));
}
- private void closeSuppressed(Exception e, Closeable closeable) {
- try {
- closeable.close();
- } catch (IOException e2) {
- e.addSuppressed(e2);
+ private void copyTempOutErrToActionOutErr() throws ActionExecutionException {
+ // If parse_showincludes feature is enabled, instead of parsing dotD file we parse the
+ // output of cl.exe caused by /showIncludes option.
+ if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
+ try {
+ FileOutErr tempOutErr = spawnExecutionContext.getFileOutErr();
+ FileOutErr outErr = actionExecutionContext.getFileOutErr();
+ tempOutErr.close();
+ if (tempOutErr.hasRecordedStdout()) {
+ try (InputStream in = tempOutErr.getOutputPath().getInputStream()) {
+ ByteStreams.copy(
+ in,
+ showIncludesFilterForStdout.getFilteredOutputStream(outErr.getOutputStream()));
+ }
+ }
+ if (tempOutErr.hasRecordedStderr()) {
+ try (InputStream in = tempOutErr.getErrorPath().getInputStream()) {
+ ByteStreams.copy(
+ in, showIncludesFilterForStderr.getFilteredOutputStream(outErr.getErrorStream()));
+ }
+ }
+ } catch (IOException e) {
+ throw printIOExceptionAndConvertToActionExecutionException(actionExecutionContext, e);
+ }
}
}
}
diff --git a/src/test/py/bazel/bazel_windows_cpp_test.py b/src/test/py/bazel/bazel_windows_cpp_test.py
index ee20860..a7a97de 100644
--- a/src/test/py/bazel/bazel_windows_cpp_test.py
+++ b/src/test/py/bazel/bazel_windows_cpp_test.py
@@ -504,5 +504,23 @@
])
self.AssertExitCode(exit_code, 0, stderr)
+ def testCppErrorShouldBeVisible(self):
+ self.ScratchFile('WORKSPACE')
+ self.ScratchFile('BUILD', [
+ 'cc_binary(',
+ ' name = "bad",',
+ ' srcs = ["bad.cc"],',
+ ')',
+ ])
+ self.ScratchFile('bad.cc', [
+ 'int main(int argc, char** argv) {',
+ ' this_is_an_error();',
+ '}',
+ ])
+ exit_code, stdout, stderr = self.RunBazel(['build', '//:bad'])
+ self.AssertExitCode(exit_code, 1, stderr)
+ self.assertIn('this_is_an_error', ''.join(stdout))
+
+
if __name__ == '__main__':
unittest.main()
