Log: report outputs of failed actions by default

Some actions are allowed to fail, typically tests. By reporting the
output of failed such actions early, the user can already have a
look at those artifacts, typically a test log, while the build is
still going on.

2 files changed, 44 insertions, 12 deletions

diff --git a/src/buildtool/execution_engine/executor/executor.hpp b/src/buildtool/execution_engine/executor/executor.hpp
index 89d92570..f2f1b880 100644
--- a/src/buildtool/execution_engine/executor/executor.hpp
+++ b/src/buildtool/execution_engine/executor/executor.hpp
@@ -591,17 +591,15 @@ class ExecutorImpl {
         progress->TaskTracker().Stop(action->Content().Id());
 
         PrintInfo(logger, action, response);
+        bool action_failed = false;
         bool should_fail_outputs = false;
         for (auto const& [local_path, node] : action->Dependencies()) {
             should_fail_outputs |= node->Content().Info()->failed;
         }
         if (response->ExitCode() != 0) {
             if (action->MayFail()) {
-                logger.Emit(LogLevel::Warning,
-                            "{} (exit code {})",
-                            *(action->MayFail()),
-                            response->ExitCode());
                 should_fail_outputs = true;
+                action_failed = true;
             }
             else {
                 logger.Emit(LogLevel::Error,
@@ -625,23 +623,48 @@ class ExecutorImpl {
             not CheckOutputsExist(*artifacts.value(),
                                   action->OutputDirPaths(),
                                   action->Content().Cwd())) {
-            logger.Emit(LogLevel::Error, [&] {
-                std::string message{
-                    "action executed with missing outputs.\n"
-                    " Action outputs should be the following artifacts:"};
+            logger.Emit(LogLevel::Error, [&]() {
+                std::ostringstream message{};
+                if (action_failed) {
+                    message << *(action->MayFail()) << " (exit code "
+                            << response->ExitCode() << ")\nMoreover ";
+                }
+                message << "action executed with missing outputs.\nAction "
+                           "outputs should be the following artifacts:";
                 for (auto const& output : action->OutputFilePaths()) {
-                    message += "\n  - file: " + output;
+                    message << "\n  - file: " << output;
                 }
                 for (auto const& output : action->OutputDirPaths()) {
-                    message += "\n  - dir: " + output;
+                    message << "\n  - dir: " << output;
                 }
-                return message;
+                return message.str();
             });
             PrintError(logger, action, progress);
             return false;
         }
 
         SaveObjectInfo(*artifacts.value(), action, should_fail_outputs);
+        if (action_failed) {
+            logger.Emit(LogLevel::Warning, [&]() {
+                std::ostringstream message{};
+                auto base = action->Content().Cwd();
+                message << *(action->MayFail()) << " (exit code "
+                        << response->ExitCode() << "); outputs:";
+                for (auto const& [name, node] : action->OutputFiles()) {
+                    message << "\n - " << nlohmann::json(name).dump() << " "
+                            << artifacts.value()
+                                   ->at(RebasePathStringRelativeTo(base, name))
+                                   .ToString();
+                }
+                for (auto const& [name, node] : action->OutputDirs()) {
+                    message << "\n - " << nlohmann::json(name).dump() << " "
+                            << artifacts.value()
+                                   ->at(RebasePathStringRelativeTo(base, name))
+                                   .ToString();
+                }
+                return message.str();
+            });
+        }
 
         return true;
     }
diff --git a/test/end-to-end/cli/log-limit.sh b/test/end-to-end/cli/log-limit.sh
index cfb41498..4b209432 100755
--- a/test/end-to-end/cli/log-limit.sh
+++ b/test/end-to-end/cli/log-limit.sh
@@ -91,6 +91,10 @@ cat > TARGETS <<'EOF'
 }
 EOF
 
+
+echo FAIL > "${TEST_TMPDIR}/result"
+FAIL_HASH=$("${JUST}" add-to-cas --local-build-root "${LBR}" "${TEST_TMPDIR}/result")
+
 "${JUST}" build --local-build-root "${LBR}" -f file.log \
           -L '["env", "PATH='"${PATH}"'"]' \
           --restrict-stderr-log-limit 1 2>console.log || :
@@ -120,7 +124,9 @@ grep TeStRuN console.log && exit 1 || :
 echo
 grep TeStFaIlUrE file.log
 grep TeStFaIlUrE console.log
-
+# Also, their outputs should be reported
+grep "RESULT.*${FAIL_HASH}" file.log
+grep "RESULT.*${FAIL_HASH}" console.log
 
 echo
 echo
@@ -211,6 +217,9 @@ grep TeStRuN console.log && exit 1 || :
 echo
 grep TeStFaIlUrE file.log
 grep TeStFaIlUrE console.log
+# Also, their outputs should be reported
+grep "RESULT.*${FAIL_HASH}" file.log
+grep "RESULT.*${FAIL_HASH}" console.log
 
 echo
 echo OK