From f7119b57ac6d7ffade29d931b94c200766f7ebd9 Mon Sep 17 00:00:00 2001 From: cherokeeMeta Date: Thu, 11 Jul 2024 16:52:18 -0700 Subject: [PATCH] Highlight build failures in soong output Summary: When build failures occur, it can be hard to find where the error message begins. We now highlight "FAILURE" in red to make it easier to see what exactly has failed in the output. Test: execute automated tests with m nothing --no-skip-soong-tests, can also manually test with a build failure and observe red text in the output Change-Id: Iad3b94cc1c385f0afdebdf12c44843db04ff2bdc Signed-off-by: Cherokee Toole --- ui/terminal/format.go | 30 ++++++++++++++++++++---------- ui/terminal/status.go | 5 +++-- ui/terminal/status_test.go | 10 +++++----- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/ui/terminal/format.go b/ui/terminal/format.go index 241a1ddf7..01f8b0d13 100644 --- a/ui/terminal/format.go +++ b/ui/terminal/format.go @@ -23,26 +23,28 @@ import ( ) type formatter struct { - format string - quiet bool - start time.Time + colorize bool + format string + quiet bool + start time.Time } // newFormatter returns a formatter for formatting output to // the terminal in a format similar to Ninja. // format takes nearly all the same options as NINJA_STATUS. // %c is currently unsupported. -func newFormatter(format string, quiet bool) formatter { +func newFormatter(colorize bool, format string, quiet bool) formatter { return formatter{ - format: format, - quiet: quiet, - start: time.Now(), + colorize: colorize, + format: format, + quiet: quiet, + start: time.Now(), } } func (s formatter) message(level status.MsgLevel, message string) string { if level >= status.ErrorLvl { - return fmt.Sprintf("FAILED: %s", message) + return fmt.Sprintf("%s %s", s.failedString(), message) } else if level > status.StatusLvl { return fmt.Sprintf("%s%s", level.Prefix(), message) } else if level == status.StatusLvl { @@ -127,9 +129,9 @@ func (s formatter) result(result status.ActionResult) string { if result.Error != nil { targets := strings.Join(result.Outputs, " ") if s.quiet || result.Command == "" { - ret = fmt.Sprintf("FAILED: %s\n%s", targets, result.Output) + ret = fmt.Sprintf("%s %s\n%s", s.failedString(), targets, result.Output) } else { - ret = fmt.Sprintf("FAILED: %s\n%s\n%s", targets, result.Command, result.Output) + ret = fmt.Sprintf("%s %s\n%s\n%s", s.failedString(), targets, result.Command, result.Output) } } else if result.Output != "" { ret = result.Output @@ -141,3 +143,11 @@ func (s formatter) result(result status.ActionResult) string { return ret } + +func (s formatter) failedString() string { + failed := "FAILED:" + if s.colorize { + failed = ansi.red() + ansi.bold() + failed + ansi.regular() + } + return failed +} diff --git a/ui/terminal/status.go b/ui/terminal/status.go index 2ad174fee..92f299405 100644 --- a/ui/terminal/status.go +++ b/ui/terminal/status.go @@ -27,9 +27,10 @@ import ( // statusFormat takes nearly all the same options as NINJA_STATUS. // %c is currently unsupported. func NewStatusOutput(w io.Writer, statusFormat string, forceSimpleOutput, quietBuild, forceKeepANSI bool) status.StatusOutput { - formatter := newFormatter(statusFormat, quietBuild) + canUseSmartFormatting := !forceSimpleOutput && isSmartTerminal(w) + formatter := newFormatter(canUseSmartFormatting, statusFormat, quietBuild) - if !forceSimpleOutput && isSmartTerminal(w) { + if canUseSmartFormatting { return NewSmartStatusOutput(w, formatter) } else { return NewSimpleStatusOutput(w, formatter, forceKeepANSI) diff --git a/ui/terminal/status_test.go b/ui/terminal/status_test.go index 8dd180967..991eca04e 100644 --- a/ui/terminal/status_test.go +++ b/ui/terminal/status_test.go @@ -58,7 +58,7 @@ func TestStatusOutput(t *testing.T) { { name: "action with error", calls: actionsWithError, - smart: "\r\x1b[1m[ 0% 0/3] action1\x1b[0m\x1b[K\r\x1b[1m[ 33% 1/3] action1\x1b[0m\x1b[K\r\x1b[1m[ 33% 1/3] action2\x1b[0m\x1b[K\r\x1b[1m[ 66% 2/3] action2\x1b[0m\x1b[K\nFAILED: f1 f2\ntouch f1 f2\nerror1\nerror2\n\r\x1b[1m[ 66% 2/3] action3\x1b[0m\x1b[K\r\x1b[1m[100% 3/3] action3\x1b[0m\x1b[K\n", + smart: "\r\x1b[1m[ 0% 0/3] action1\x1b[0m\x1b[K\r\x1b[1m[ 33% 1/3] action1\x1b[0m\x1b[K\r\x1b[1m[ 33% 1/3] action2\x1b[0m\x1b[K\r\x1b[1m[ 66% 2/3] action2\x1b[0m\x1b[K\n\x1b[31m\x1b[1mFAILED:\x1b[0m f1 f2\ntouch f1 f2\nerror1\nerror2\n\r\x1b[1m[ 66% 2/3] action3\x1b[0m\x1b[K\r\x1b[1m[100% 3/3] action3\x1b[0m\x1b[K\n", simple: "[ 33% 1/3] action1\n[ 66% 2/3] action2\nFAILED: f1 f2\ntouch f1 f2\nerror1\nerror2\n[100% 3/3] action3\n", }, { @@ -70,7 +70,7 @@ func TestStatusOutput(t *testing.T) { { name: "messages", calls: actionsWithMessages, - smart: "\r\x1b[1m[ 0% 0/2] action1\x1b[0m\x1b[K\r\x1b[1m[ 50% 1/2] action1\x1b[0m\x1b[K\r\x1b[1mstatus\x1b[0m\x1b[K\r\x1b[Kprint\nFAILED: error\n\r\x1b[1m[ 50% 1/2] action2\x1b[0m\x1b[K\r\x1b[1m[100% 2/2] action2\x1b[0m\x1b[K\n", + smart: "\r\x1b[1m[ 0% 0/2] action1\x1b[0m\x1b[K\r\x1b[1m[ 50% 1/2] action1\x1b[0m\x1b[K\r\x1b[1mstatus\x1b[0m\x1b[K\r\x1b[Kprint\n\x1b[31m\x1b[1mFAILED:\x1b[0m error\n\r\x1b[1m[ 50% 1/2] action2\x1b[0m\x1b[K\r\x1b[1m[100% 2/2] action2\x1b[0m\x1b[K\n", simple: "[ 50% 1/2] action1\nstatus\nprint\nFAILED: error\n[100% 2/2] action2\n", }, { @@ -362,7 +362,7 @@ func TestSmartStatusHideAfterFailure(t *testing.T) { stat.Flush() - w := "\r\x1b[1m[ 0% 0/2] action1\x1b[0m\x1b[K\r\x1b[1m[ 0% 0/2] action2\x1b[0m\x1b[K\r\x1b[1m[ 50% 1/2] action1\x1b[0m\x1b[K\nFAILED: \nOutput1\n\r\x1b[1m[100% 2/2] action2\x1b[0m\x1b[K\nThere was 1 action that completed after the action that failed. See verbose.log.gz for its output.\n" + w := "\r\x1b[1m[ 0% 0/2] action1\x1b[0m\x1b[K\r\x1b[1m[ 0% 0/2] action2\x1b[0m\x1b[K\r\x1b[1m[ 50% 1/2] action1\x1b[0m\x1b[K\n\x1b[31m\x1b[1mFAILED:\x1b[0m \nOutput1\n\r\x1b[1m[100% 2/2] action2\x1b[0m\x1b[K\nThere was 1 action that completed after the action that failed. See verbose.log.gz for its output.\n" if g := smart.String(); g != w { t.Errorf("want:\n%q\ngot:\n%q", w, g) @@ -407,7 +407,7 @@ func TestSmartStatusHideAfterFailurePlural(t *testing.T) { stat.Flush() - w := "\r\x1b[1m[ 0% 0/2] action1\x1b[0m\x1b[K\r\x1b[1m[ 0% 0/2] action2\x1b[0m\x1b[K\r\x1b[1m[ 0% 0/2] action3\x1b[0m\x1b[K\r\x1b[1m[ 50% 1/2] action1\x1b[0m\x1b[K\nFAILED: \nOutput1\n\r\x1b[1m[100% 2/2] action2\x1b[0m\x1b[K\r\x1b[1m[150% 3/2] action3\x1b[0m\x1b[K\nThere were 2 actions that completed after the action that failed. See verbose.log.gz for their output.\n" + w := "\r\x1b[1m[ 0% 0/2] action1\x1b[0m\x1b[K\r\x1b[1m[ 0% 0/2] action2\x1b[0m\x1b[K\r\x1b[1m[ 0% 0/2] action3\x1b[0m\x1b[K\r\x1b[1m[ 50% 1/2] action1\x1b[0m\x1b[K\n\x1b[31m\x1b[1mFAILED:\x1b[0m \nOutput1\n\r\x1b[1m[100% 2/2] action2\x1b[0m\x1b[K\r\x1b[1m[150% 3/2] action3\x1b[0m\x1b[K\nThere were 2 actions that completed after the action that failed. See verbose.log.gz for their output.\n" if g := smart.String(); g != w { t.Errorf("want:\n%q\ngot:\n%q", w, g) @@ -445,7 +445,7 @@ func TestSmartStatusDontHideErrorAfterFailure(t *testing.T) { stat.Flush() - w := "\r\x1b[1m[ 0% 0/2] action1\x1b[0m\x1b[K\r\x1b[1m[ 0% 0/2] action2\x1b[0m\x1b[K\r\x1b[1m[ 50% 1/2] action1\x1b[0m\x1b[K\nFAILED: \nOutput1\n\r\x1b[1m[100% 2/2] action2\x1b[0m\x1b[K\nFAILED: \nOutput2\n" + w := "\r\x1b[1m[ 0% 0/2] action1\x1b[0m\x1b[K\r\x1b[1m[ 0% 0/2] action2\x1b[0m\x1b[K\r\x1b[1m[ 50% 1/2] action1\x1b[0m\x1b[K\n\x1b[31m\x1b[1mFAILED:\x1b[0m \nOutput1\n\r\x1b[1m[100% 2/2] action2\x1b[0m\x1b[K\n\x1b[31m\x1b[1mFAILED:\x1b[0m \nOutput2\n" if g := smart.String(); g != w { t.Errorf("want:\n%q\ngot:\n%q", w, g)