From dde49cb687796e38f01e649c5f9dd515db8a2f69 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sat, 8 Jun 2019 21:59:42 -0700 Subject: [PATCH 1/6] Add tests for status output Test: status_test.go Change-Id: If3febd8fdacb0e70716d0520a41c982bd6474720 --- ui/terminal/Android.bp | 1 + ui/terminal/status_test.go | 297 +++++++++++++++++++++++++++++++++++++ ui/terminal/util.go | 9 ++ 3 files changed, 307 insertions(+) create mode 100644 ui/terminal/status_test.go diff --git a/ui/terminal/Android.bp b/ui/terminal/Android.bp index 7104a5047..cf6cf0a30 100644 --- a/ui/terminal/Android.bp +++ b/ui/terminal/Android.bp @@ -22,6 +22,7 @@ bootstrap_go_package { "util.go", ], testSrcs: [ + "status_test.go", "util_test.go", ], darwin: { diff --git a/ui/terminal/status_test.go b/ui/terminal/status_test.go new file mode 100644 index 000000000..e032c1f1e --- /dev/null +++ b/ui/terminal/status_test.go @@ -0,0 +1,297 @@ +// Copyright 2018 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package terminal + +import ( + "bytes" + "fmt" + "testing" + + "android/soong/ui/status" +) + +func TestStatusOutput(t *testing.T) { + tests := []struct { + name string + calls func(stat status.StatusOutput) + smart string + dumb string + }{ + { + name: "two actions", + calls: twoActions, + smart: "\r[ 0% 0/2] action1\x1b[K\r[ 50% 1/2] action1\x1b[K\r[ 50% 1/2] action2\x1b[K\r[100% 2/2] action2\x1b[K\n", + dumb: "[ 50% 1/2] action1\n[100% 2/2] action2\n", + }, + { + name: "two parallel actions", + calls: twoParallelActions, + smart: "\r[ 0% 0/2] action1\x1b[K\r[ 0% 0/2] action2\x1b[K\r[ 50% 1/2] action1\x1b[K\r[100% 2/2] action2\x1b[K\n", + dumb: "[ 50% 1/2] action1\n[100% 2/2] action2\n", + }, + { + name: "action with output", + calls: actionsWithOutput, + smart: "\r[ 0% 0/3] action1\x1b[K\r[ 33% 1/3] action1\x1b[K\r[ 33% 1/3] action2\x1b[K\r[ 66% 2/3] action2\x1b[K\noutput1\noutput2\n\r[ 66% 2/3] action3\x1b[K\r[100% 3/3] action3\x1b[K\n", + dumb: "[ 33% 1/3] action1\n[ 66% 2/3] action2\noutput1\noutput2\n[100% 3/3] action3\n", + }, + { + name: "action with output without newline", + calls: actionsWithOutputWithoutNewline, + smart: "\r[ 0% 0/3] action1\x1b[K\r[ 33% 1/3] action1\x1b[K\r[ 33% 1/3] action2\x1b[K\r[ 66% 2/3] action2\x1b[K\noutput1\noutput2\n\r[ 66% 2/3] action3\x1b[K\r[100% 3/3] action3\x1b[K\n", + dumb: "[ 33% 1/3] action1\n[ 66% 2/3] action2\noutput1\noutput2\n[100% 3/3] action3\n", + }, + { + name: "action with error", + calls: actionsWithError, + smart: "\r[ 0% 0/3] action1\x1b[K\r[ 33% 1/3] action1\x1b[K\r[ 33% 1/3] action2\x1b[K\r[ 66% 2/3] action2\x1b[K\nFAILED: f1 f2\ntouch f1 f2\nerror1\nerror2\n\r[ 66% 2/3] action3\x1b[K\r[100% 3/3] action3\x1b[K\n", + dumb: "[ 33% 1/3] action1\n[ 66% 2/3] action2\nFAILED: f1 f2\ntouch f1 f2\nerror1\nerror2\n[100% 3/3] action3\n", + }, + { + name: "action with empty description", + calls: actionWithEmptyDescription, + smart: "\r[ 0% 0/1] command1\x1b[K\r[100% 1/1] command1\x1b[K\n", + dumb: "[100% 1/1] command1\n", + }, + { + name: "messages", + calls: actionsWithMessages, + smart: "\r[ 0% 0/2] action1\x1b[K\r[ 50% 1/2] action1\x1b[K\rstatus\x1b[K\r\x1b[Kprint\nFAILED: error\n\r[ 50% 1/2] action2\x1b[K\r[100% 2/2] action2\x1b[K\n", + dumb: "[ 50% 1/2] action1\nstatus\nprint\nFAILED: error\n[100% 2/2] action2\n", + }, + { + name: "action with long description", + calls: actionWithLongDescription, + smart: "\r[ 0% 0/2] action with very long descrip\x1b[K\r[ 50% 1/2] action with very long descrip\x1b[K\n", + dumb: "[ 50% 1/2] action with very long description to test eliding\n", + }, + { + name: "action with output with ansi codes", + calls: actionWithOuptutWithAnsiCodes, + smart: "\r[ 0% 0/1] action1\x1b[K\r[100% 1/1] action1\x1b[K\n\x1b[31mcolor\x1b[0m\n", + dumb: "[100% 1/1] action1\ncolor\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Run("smart", func(t *testing.T) { + smart := &fakeSmartTerminal{termWidth: 40} + stdio := customStdio{ + stdin: nil, + stdout: smart, + stderr: nil, + } + + writer := NewWriter(stdio) + stat := NewStatusOutput(writer, "", false) + tt.calls(stat) + stat.Flush() + writer.Finish() + + if g, w := smart.String(), tt.smart; g != w { + t.Errorf("want:\n%q\ngot:\n%q", w, g) + } + }) + + t.Run("dumb", func(t *testing.T) { + dumb := &bytes.Buffer{} + stdio := customStdio{ + stdin: nil, + stdout: dumb, + stderr: nil, + } + + writer := NewWriter(stdio) + stat := NewStatusOutput(writer, "", false) + tt.calls(stat) + stat.Flush() + writer.Finish() + + if g, w := dumb.String(), tt.dumb; g != w { + t.Errorf("want:\n%q\ngot:\n%q", w, g) + } + }) + }) + } +} + +type runner struct { + counts status.Counts + stat status.StatusOutput +} + +func newRunner(stat status.StatusOutput, totalActions int) *runner { + return &runner{ + counts: status.Counts{TotalActions: totalActions}, + stat: stat, + } +} + +func (r *runner) startAction(action *status.Action) { + r.counts.StartedActions++ + r.counts.RunningActions++ + r.stat.StartAction(action, r.counts) +} + +func (r *runner) finishAction(result status.ActionResult) { + r.counts.FinishedActions++ + r.counts.RunningActions-- + r.stat.FinishAction(result, r.counts) +} + +func (r *runner) finishAndStartAction(result status.ActionResult, action *status.Action) { + r.counts.FinishedActions++ + r.stat.FinishAction(result, r.counts) + + r.counts.StartedActions++ + r.stat.StartAction(action, r.counts) +} + +var ( + action1 = &status.Action{Description: "action1"} + result1 = status.ActionResult{Action: action1} + action2 = &status.Action{Description: "action2"} + result2 = status.ActionResult{Action: action2} + action3 = &status.Action{Description: "action3"} + result3 = status.ActionResult{Action: action3} +) + +func twoActions(stat status.StatusOutput) { + runner := newRunner(stat, 2) + runner.startAction(action1) + runner.finishAction(result1) + runner.startAction(action2) + runner.finishAction(result2) +} + +func twoParallelActions(stat status.StatusOutput) { + runner := newRunner(stat, 2) + runner.startAction(action1) + runner.startAction(action2) + runner.finishAction(result1) + runner.finishAction(result2) +} + +func actionsWithOutput(stat status.StatusOutput) { + result2WithOutput := status.ActionResult{Action: action2, Output: "output1\noutput2\n"} + + runner := newRunner(stat, 3) + runner.startAction(action1) + runner.finishAction(result1) + runner.startAction(action2) + runner.finishAction(result2WithOutput) + runner.startAction(action3) + runner.finishAction(result3) +} + +func actionsWithOutputWithoutNewline(stat status.StatusOutput) { + result2WithOutputWithoutNewline := status.ActionResult{Action: action2, Output: "output1\noutput2"} + + runner := newRunner(stat, 3) + runner.startAction(action1) + runner.finishAction(result1) + runner.startAction(action2) + runner.finishAction(result2WithOutputWithoutNewline) + runner.startAction(action3) + runner.finishAction(result3) +} + +func actionsWithError(stat status.StatusOutput) { + action2WithError := &status.Action{Description: "action2", Outputs: []string{"f1", "f2"}, Command: "touch f1 f2"} + result2WithError := status.ActionResult{Action: action2WithError, Output: "error1\nerror2\n", Error: fmt.Errorf("error1")} + + runner := newRunner(stat, 3) + runner.startAction(action1) + runner.finishAction(result1) + runner.startAction(action2WithError) + runner.finishAction(result2WithError) + runner.startAction(action3) + runner.finishAction(result3) +} + +func actionWithEmptyDescription(stat status.StatusOutput) { + action1 := &status.Action{Command: "command1"} + result1 := status.ActionResult{Action: action1} + + runner := newRunner(stat, 1) + runner.startAction(action1) + runner.finishAction(result1) +} + +func actionsWithMessages(stat status.StatusOutput) { + runner := newRunner(stat, 2) + + runner.startAction(action1) + runner.finishAction(result1) + + stat.Message(status.VerboseLvl, "verbose") + stat.Message(status.StatusLvl, "status") + stat.Message(status.PrintLvl, "print") + stat.Message(status.ErrorLvl, "error") + + runner.startAction(action2) + runner.finishAction(result2) +} + +func actionWithLongDescription(stat status.StatusOutput) { + action1 := &status.Action{Description: "action with very long description to test eliding"} + result1 := status.ActionResult{Action: action1} + + runner := newRunner(stat, 2) + + runner.startAction(action1) + + runner.finishAction(result1) +} + +func actionWithOuptutWithAnsiCodes(stat status.StatusOutput) { + result1WithOutputWithAnsiCodes := status.ActionResult{Action: action1, Output: "\x1b[31mcolor\x1b[0m"} + + runner := newRunner(stat, 1) + runner.startAction(action1) + runner.finishAction(result1WithOutputWithAnsiCodes) +} + +func TestSmartStatusOutputWidthChange(t *testing.T) { + smart := &fakeSmartTerminal{termWidth: 40} + + stdio := customStdio{ + stdin: nil, + stdout: smart, + stderr: nil, + } + + writer := NewWriter(stdio) + stat := NewStatusOutput(writer, "", false) + + runner := newRunner(stat, 2) + + action := &status.Action{Description: "action with very long description to test eliding"} + result := status.ActionResult{Action: action} + + runner.startAction(action) + smart.termWidth = 30 + runner.finishAction(result) + + stat.Flush() + writer.Finish() + + w := "\r[ 0% 0/2] action with very long descrip\x1b[K\r[ 50% 1/2] action with very lo\x1b[K\n" + + if g := smart.String(); g != w { + t.Errorf("want:\n%q\ngot:\n%q", w, g) + } +} diff --git a/ui/terminal/util.go b/ui/terminal/util.go index a85a517b9..4309809c7 100644 --- a/ui/terminal/util.go +++ b/ui/terminal/util.go @@ -29,6 +29,8 @@ func isTerminal(w io.Writer) bool { ioctlGetTermios, uintptr(unsafe.Pointer(&termios)), 0, 0, 0) return err == 0 + } else if _, ok := w.(*fakeSmartTerminal); ok { + return true } return false } @@ -43,6 +45,8 @@ func termWidth(w io.Writer) (int, bool) { syscall.TIOCGWINSZ, uintptr(unsafe.Pointer(&winsize)), 0, 0, 0) return int(winsize.ws_column), err == 0 + } else if f, ok := w.(*fakeSmartTerminal); ok { + return f.termWidth, true } return 0, false } @@ -99,3 +103,8 @@ func stripAnsiEscapes(input []byte) []byte { return input } + +type fakeSmartTerminal struct { + bytes.Buffer + termWidth int +} From ce525350f419b9781410d2a2e734929ff13fe54a Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sat, 8 Jun 2019 18:58:00 -0700 Subject: [PATCH 2/6] Move smart and dumb terminals into separate implementations Support for smart and dumb terminals are implemented in writer.go, which makes dumb terminals much more complicated than necessary. Move smart and dumb terminals into two separate implementations of StatusOutput, with common code moved into a shared formatter class. Test: not yet Change-Id: I59bbdae479f138b46cd0f03092720a3303e8f0fe --- ui/terminal/Android.bp | 3 + ui/terminal/dumb_status.go | 65 ++++++++++++++++ ui/terminal/format.go | 123 ++++++++++++++++++++++++++++++ ui/terminal/smart_status.go | 148 ++++++++++++++++++++++++++++++++++++ ui/terminal/status.go | 118 +--------------------------- ui/terminal/util.go | 2 +- ui/terminal/writer.go | 129 +++---------------------------- 7 files changed, 355 insertions(+), 233 deletions(-) create mode 100644 ui/terminal/dumb_status.go create mode 100644 ui/terminal/format.go create mode 100644 ui/terminal/smart_status.go diff --git a/ui/terminal/Android.bp b/ui/terminal/Android.bp index cf6cf0a30..683e3e39a 100644 --- a/ui/terminal/Android.bp +++ b/ui/terminal/Android.bp @@ -17,6 +17,9 @@ bootstrap_go_package { pkgPath: "android/soong/ui/terminal", deps: ["soong-ui-status"], srcs: [ + "dumb_status.go", + "format.go", + "smart_status.go", "status.go", "writer.go", "util.go", diff --git a/ui/terminal/dumb_status.go b/ui/terminal/dumb_status.go new file mode 100644 index 000000000..f2fcba79d --- /dev/null +++ b/ui/terminal/dumb_status.go @@ -0,0 +1,65 @@ +// Copyright 2019 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package terminal + +import ( + "fmt" + + "android/soong/ui/status" +) + +type dumbStatusOutput struct { + writer Writer + formatter formatter +} + +// NewDumbStatusOutput returns a StatusOutput that represents the +// current build status similarly to Ninja's built-in terminal +// output. +func NewDumbStatusOutput(w Writer, formatter formatter) status.StatusOutput { + return &dumbStatusOutput{ + writer: w, + formatter: formatter, + } +} + +func (s *dumbStatusOutput) Message(level status.MsgLevel, message string) { + if level >= status.StatusLvl { + fmt.Fprintln(s.writer, s.formatter.message(level, message)) + } +} + +func (s *dumbStatusOutput) StartAction(action *status.Action, counts status.Counts) { +} + +func (s *dumbStatusOutput) FinishAction(result status.ActionResult, counts status.Counts) { + str := result.Description + if str == "" { + str = result.Command + } + + progress := s.formatter.progress(counts) + str + + output := s.formatter.result(result) + output = string(stripAnsiEscapes([]byte(output))) + + if output != "" { + fmt.Fprint(s.writer, progress, "\n", output) + } else { + fmt.Fprintln(s.writer, progress) + } +} + +func (s *dumbStatusOutput) Flush() {} diff --git a/ui/terminal/format.go b/ui/terminal/format.go new file mode 100644 index 000000000..4205bdc22 --- /dev/null +++ b/ui/terminal/format.go @@ -0,0 +1,123 @@ +// Copyright 2019 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package terminal + +import ( + "fmt" + "strings" + "time" + + "android/soong/ui/status" +) + +type formatter struct { + 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 { + return formatter{ + 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) + } else if level > status.StatusLvl { + return fmt.Sprintf("%s%s", level.Prefix(), message) + } else if level == status.StatusLvl { + return message + } + return "" +} + +func (s formatter) progress(counts status.Counts) string { + if s.format == "" { + return fmt.Sprintf("[%3d%% %d/%d] ", 100*counts.FinishedActions/counts.TotalActions, counts.FinishedActions, counts.TotalActions) + } + + buf := &strings.Builder{} + for i := 0; i < len(s.format); i++ { + c := s.format[i] + if c != '%' { + buf.WriteByte(c) + continue + } + + i = i + 1 + if i == len(s.format) { + buf.WriteByte(c) + break + } + + c = s.format[i] + switch c { + case '%': + buf.WriteByte(c) + case 's': + fmt.Fprintf(buf, "%d", counts.StartedActions) + case 't': + fmt.Fprintf(buf, "%d", counts.TotalActions) + case 'r': + fmt.Fprintf(buf, "%d", counts.RunningActions) + case 'u': + fmt.Fprintf(buf, "%d", counts.TotalActions-counts.StartedActions) + case 'f': + fmt.Fprintf(buf, "%d", counts.FinishedActions) + case 'o': + fmt.Fprintf(buf, "%.1f", float64(counts.FinishedActions)/time.Since(s.start).Seconds()) + case 'c': + // TODO: implement? + buf.WriteRune('?') + case 'p': + fmt.Fprintf(buf, "%3d%%", 100*counts.FinishedActions/counts.TotalActions) + case 'e': + fmt.Fprintf(buf, "%.3f", time.Since(s.start).Seconds()) + default: + buf.WriteString("unknown placeholder '") + buf.WriteByte(c) + buf.WriteString("'") + } + } + return buf.String() +} + +func (s formatter) result(result status.ActionResult) string { + var ret 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) + } else { + ret = fmt.Sprintf("FAILED: %s\n%s\n%s", targets, result.Command, result.Output) + } + } else if result.Output != "" { + ret = result.Output + } + + if len(ret) > 0 && ret[len(ret)-1] != '\n' { + ret += "\n" + } + + return ret +} diff --git a/ui/terminal/smart_status.go b/ui/terminal/smart_status.go new file mode 100644 index 000000000..5edc21a1a --- /dev/null +++ b/ui/terminal/smart_status.go @@ -0,0 +1,148 @@ +// Copyright 2019 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package terminal + +import ( + "fmt" + "strings" + "sync" + + "android/soong/ui/status" +) + +type smartStatusOutput struct { + writer Writer + formatter formatter + + lock sync.Mutex + + haveBlankLine bool +} + +// NewSmartStatusOutput returns a StatusOutput that represents the +// current build status similarly to Ninja's built-in terminal +// output. +func NewSmartStatusOutput(w Writer, formatter formatter) status.StatusOutput { + return &smartStatusOutput{ + writer: w, + formatter: formatter, + + haveBlankLine: true, + } +} + +func (s *smartStatusOutput) Message(level status.MsgLevel, message string) { + if level < status.StatusLvl { + return + } + + str := s.formatter.message(level, message) + + s.lock.Lock() + defer s.lock.Unlock() + + if level > status.StatusLvl { + s.print(str) + } else { + s.statusLine(str) + } +} + +func (s *smartStatusOutput) StartAction(action *status.Action, counts status.Counts) { + str := action.Description + if str == "" { + str = action.Command + } + + progress := s.formatter.progress(counts) + + s.lock.Lock() + defer s.lock.Unlock() + + s.statusLine(progress + str) +} + +func (s *smartStatusOutput) FinishAction(result status.ActionResult, counts status.Counts) { + str := result.Description + if str == "" { + str = result.Command + } + + progress := s.formatter.progress(counts) + str + + output := s.formatter.result(result) + + s.lock.Lock() + defer s.lock.Unlock() + + if output != "" { + s.statusLine(progress) + s.requestLine() + s.print(output) + } else { + s.statusLine(progress) + } +} + +func (s *smartStatusOutput) Flush() { + s.lock.Lock() + defer s.lock.Unlock() + + s.requestLine() +} + +func (s *smartStatusOutput) requestLine() { + if !s.haveBlankLine { + fmt.Fprintln(s.writer) + s.haveBlankLine = true + } +} + +func (s *smartStatusOutput) print(str string) { + if !s.haveBlankLine { + fmt.Fprint(s.writer, "\r", "\x1b[K") + s.haveBlankLine = true + } + fmt.Fprint(s.writer, str) + if len(str) == 0 || str[len(str)-1] != '\n' { + fmt.Fprint(s.writer, "\n") + } +} + +func (s *smartStatusOutput) statusLine(str string) { + idx := strings.IndexRune(str, '\n') + if idx != -1 { + str = str[0:idx] + } + + // Limit line width to the terminal width, otherwise we'll wrap onto + // another line and we won't delete the previous line. + // + // Run this on every line in case the window has been resized while + // we're printing. This could be optimized to only re-run when we get + // SIGWINCH if it ever becomes too time consuming. + if max, ok := s.writer.termWidth(); ok { + if len(str) > max { + // TODO: Just do a max. Ninja elides the middle, but that's + // more complicated and these lines aren't that important. + str = str[:max] + } + } + + // Move to the beginning on the line, print the output, then clear + // the rest of the line. + fmt.Fprint(s.writer, "\r", str, "\x1b[K") + s.haveBlankLine = false +} diff --git a/ui/terminal/status.go b/ui/terminal/status.go index 2445c5b2c..481c511a0 100644 --- a/ui/terminal/status.go +++ b/ui/terminal/status.go @@ -15,21 +15,9 @@ package terminal import ( - "fmt" - "strings" - "time" - "android/soong/ui/status" ) -type statusOutput struct { - writer Writer - format string - - start time.Time - quiet bool -} - // NewStatusOutput returns a StatusOutput that represents the // current build status similarly to Ninja's built-in terminal // output. @@ -37,109 +25,11 @@ type statusOutput struct { // statusFormat takes nearly all the same options as NINJA_STATUS. // %c is currently unsupported. func NewStatusOutput(w Writer, statusFormat string, quietBuild bool) status.StatusOutput { - return &statusOutput{ - writer: w, - format: statusFormat, + formatter := newFormatter(statusFormat, quietBuild) - start: time.Now(), - quiet: quietBuild, - } -} - -func (s *statusOutput) Message(level status.MsgLevel, message string) { - if level >= status.ErrorLvl { - s.writer.Print(fmt.Sprintf("FAILED: %s", message)) - } else if level > status.StatusLvl { - s.writer.Print(fmt.Sprintf("%s%s", level.Prefix(), message)) - } else if level == status.StatusLvl { - s.writer.StatusLine(message) - } -} - -func (s *statusOutput) StartAction(action *status.Action, counts status.Counts) { - if !s.writer.isSmartTerminal() { - return - } - - str := action.Description - if str == "" { - str = action.Command - } - - s.writer.StatusLine(s.progress(counts) + str) -} - -func (s *statusOutput) FinishAction(result status.ActionResult, counts status.Counts) { - str := result.Description - if str == "" { - str = result.Command - } - - progress := s.progress(counts) + str - - if result.Error != nil { - targets := strings.Join(result.Outputs, " ") - if s.quiet || result.Command == "" { - s.writer.StatusAndMessage(progress, fmt.Sprintf("FAILED: %s\n%s", targets, result.Output)) - } else { - s.writer.StatusAndMessage(progress, fmt.Sprintf("FAILED: %s\n%s\n%s", targets, result.Command, result.Output)) - } - } else if result.Output != "" { - s.writer.StatusAndMessage(progress, result.Output) + if w.isSmartTerminal() { + return NewSmartStatusOutput(w, formatter) } else { - s.writer.StatusLine(progress) + return NewDumbStatusOutput(w, formatter) } } - -func (s *statusOutput) Flush() {} - -func (s *statusOutput) progress(counts status.Counts) string { - if s.format == "" { - return fmt.Sprintf("[%3d%% %d/%d] ", 100*counts.FinishedActions/counts.TotalActions, counts.FinishedActions, counts.TotalActions) - } - - buf := &strings.Builder{} - for i := 0; i < len(s.format); i++ { - c := s.format[i] - if c != '%' { - buf.WriteByte(c) - continue - } - - i = i + 1 - if i == len(s.format) { - buf.WriteByte(c) - break - } - - c = s.format[i] - switch c { - case '%': - buf.WriteByte(c) - case 's': - fmt.Fprintf(buf, "%d", counts.StartedActions) - case 't': - fmt.Fprintf(buf, "%d", counts.TotalActions) - case 'r': - fmt.Fprintf(buf, "%d", counts.RunningActions) - case 'u': - fmt.Fprintf(buf, "%d", counts.TotalActions-counts.StartedActions) - case 'f': - fmt.Fprintf(buf, "%d", counts.FinishedActions) - case 'o': - fmt.Fprintf(buf, "%.1f", float64(counts.FinishedActions)/time.Since(s.start).Seconds()) - case 'c': - // TODO: implement? - buf.WriteRune('?') - case 'p': - fmt.Fprintf(buf, "%3d%%", 100*counts.FinishedActions/counts.TotalActions) - case 'e': - fmt.Fprintf(buf, "%.3f", time.Since(s.start).Seconds()) - default: - buf.WriteString("unknown placeholder '") - buf.WriteByte(c) - buf.WriteString("'") - } - } - return buf.String() -} diff --git a/ui/terminal/util.go b/ui/terminal/util.go index 4309809c7..3a11b79bb 100644 --- a/ui/terminal/util.go +++ b/ui/terminal/util.go @@ -22,7 +22,7 @@ import ( "unsafe" ) -func isTerminal(w io.Writer) bool { +func isSmartTerminal(w io.Writer) bool { if f, ok := w.(*os.File); ok { var termios syscall.Termios _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, f.Fd(), diff --git a/ui/terminal/writer.go b/ui/terminal/writer.go index ebe4b2aad..26e0e342f 100644 --- a/ui/terminal/writer.go +++ b/ui/terminal/writer.go @@ -21,8 +21,6 @@ import ( "fmt" "io" "os" - "strings" - "sync" ) // Writer provides an interface to write temporary and permanent messages to @@ -39,22 +37,6 @@ type Writer interface { // On a dumb terminal, the status messages will be kept. Print(str string) - // Status prints the first line of the string to the terminal, - // overwriting any previous status line. Strings longer than the width - // of the terminal will be cut off. - // - // On a dumb terminal, previous status messages will remain, and the - // entire first line of the string will be printed. - StatusLine(str string) - - // StatusAndMessage prints the first line of status to the terminal, - // similarly to StatusLine(), then prints the full msg below that. The - // status line is retained. - // - // There is guaranteed to be no other output in between the status and - // message. - StatusAndMessage(status, msg string) - // Finish ensures that the output ends with a newline (preserving any // current status line that is current displayed). // @@ -69,6 +51,7 @@ type Writer interface { Write(p []byte) (n int, err error) isSmartTerminal() bool + termWidth() (int, bool) } // NewWriter creates a new Writer based on the stdio and the TERM @@ -76,124 +59,34 @@ type Writer interface { func NewWriter(stdio StdioInterface) Writer { w := &writerImpl{ stdio: stdio, - - haveBlankLine: true, } - if term, ok := os.LookupEnv("TERM"); ok && term != "dumb" { - w.smartTerminal = isTerminal(stdio.Stdout()) - } - w.stripEscapes = !w.smartTerminal - return w } type writerImpl struct { stdio StdioInterface - - haveBlankLine bool - - // Protecting the above, we assume that smartTerminal and stripEscapes - // does not change after initial setup. - lock sync.Mutex - - smartTerminal bool - stripEscapes bool -} - -func (w *writerImpl) isSmartTerminal() bool { - return w.smartTerminal -} - -func (w *writerImpl) requestLine() { - if !w.haveBlankLine { - fmt.Fprintln(w.stdio.Stdout()) - w.haveBlankLine = true - } } func (w *writerImpl) Print(str string) { - if w.stripEscapes { - str = string(stripAnsiEscapes([]byte(str))) - } - - w.lock.Lock() - defer w.lock.Unlock() - w.print(str) -} - -func (w *writerImpl) print(str string) { - if !w.haveBlankLine { - fmt.Fprint(w.stdio.Stdout(), "\r", "\x1b[K") - w.haveBlankLine = true - } fmt.Fprint(w.stdio.Stdout(), str) if len(str) == 0 || str[len(str)-1] != '\n' { fmt.Fprint(w.stdio.Stdout(), "\n") } } -func (w *writerImpl) StatusLine(str string) { - w.lock.Lock() - defer w.lock.Unlock() - - w.statusLine(str) -} - -func (w *writerImpl) statusLine(str string) { - if !w.smartTerminal { - fmt.Fprintln(w.stdio.Stdout(), str) - return - } - - idx := strings.IndexRune(str, '\n') - if idx != -1 { - str = str[0:idx] - } - - // Limit line width to the terminal width, otherwise we'll wrap onto - // another line and we won't delete the previous line. - // - // Run this on every line in case the window has been resized while - // we're printing. This could be optimized to only re-run when we get - // SIGWINCH if it ever becomes too time consuming. - if max, ok := termWidth(w.stdio.Stdout()); ok { - if len(str) > max { - // TODO: Just do a max. Ninja elides the middle, but that's - // more complicated and these lines aren't that important. - str = str[:max] - } - } - - // Move to the beginning on the line, print the output, then clear - // the rest of the line. - fmt.Fprint(w.stdio.Stdout(), "\r", str, "\x1b[K") - w.haveBlankLine = false -} - -func (w *writerImpl) StatusAndMessage(status, msg string) { - if w.stripEscapes { - msg = string(stripAnsiEscapes([]byte(msg))) - } - - w.lock.Lock() - defer w.lock.Unlock() - - w.statusLine(status) - w.requestLine() - w.print(msg) -} - -func (w *writerImpl) Finish() { - w.lock.Lock() - defer w.lock.Unlock() - - w.requestLine() -} +func (w *writerImpl) Finish() {} func (w *writerImpl) Write(p []byte) (n int, err error) { - w.Print(string(p)) - return len(p), nil + return w.stdio.Stdout().Write(p) +} + +func (w *writerImpl) isSmartTerminal() bool { + return isSmartTerminal(w.stdio.Stdout()) +} + +func (w *writerImpl) termWidth() (int, bool) { + return termWidth(w.stdio.Stdout()) } // StdioInterface represents a set of stdin/stdout/stderr Reader/Writers From 097ed2a37cca619a9be0e109a108f3d474b576d1 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sat, 8 Jun 2019 21:48:58 -0700 Subject: [PATCH 3/6] Remove terminal.Writer terminal.Writer is now just a wrapper around stdio.Stdout() without any useful functionality. Replace it with stdio.Stdout() as an io.Writer. Test: ui/terminal/status_test.go Change-Id: I5bc5476afdca950b505642f0135a3af9d37fbe24 --- cmd/multiproduct_kati/main.go | 13 ++-- cmd/soong_ui/main.go | 23 +++---- ui/build/config_test.go | 3 +- ui/build/context.go | 4 +- ui/build/dumpvars.go | 2 +- ui/terminal/Android.bp | 2 +- ui/terminal/dumb_status.go | 5 +- ui/terminal/smart_status.go | 7 +- ui/terminal/status.go | 6 +- ui/terminal/status_test.go | 31 +-------- ui/terminal/stdio.go | 55 +++++++++++++++ ui/terminal/writer.go | 122 ---------------------------------- 12 files changed, 90 insertions(+), 183 deletions(-) create mode 100644 ui/terminal/stdio.go delete mode 100644 ui/terminal/writer.go diff --git a/cmd/multiproduct_kati/main.go b/cmd/multiproduct_kati/main.go index 330c5dd27..c976dcb45 100644 --- a/cmd/multiproduct_kati/main.go +++ b/cmd/multiproduct_kati/main.go @@ -156,10 +156,9 @@ type mpContext struct { } func main() { - writer := terminal.NewWriter(terminal.StdioImpl{}) - defer writer.Finish() + stdio := terminal.StdioImpl{} - log := logger.New(writer) + log := logger.New(stdio.Stdout()) defer log.Cleanup() flag.Parse() @@ -172,7 +171,7 @@ func main() { stat := &status.Status{} defer stat.Finish() - stat.AddOutput(terminal.NewStatusOutput(writer, "", + stat.AddOutput(terminal.NewStatusOutput(stdio.Stdout(), "", build.OsEnvironment().IsEnvTrue("ANDROID_QUIET_BUILD"))) var failures failureCount @@ -188,7 +187,7 @@ func main() { Context: ctx, Logger: log, Tracer: trace, - Writer: writer, + Writer: stdio.Stdout(), Status: stat, }} @@ -341,7 +340,7 @@ func main() { } else if failures > 1 { log.Fatalf("%d failures", failures) } else { - writer.Print("Success") + fmt.Fprintln(stdio.Stdout(), "Success") } } @@ -386,7 +385,7 @@ func buildProduct(mpctx *mpContext, product string) { Context: mpctx.Context, Logger: log, Tracer: mpctx.Tracer, - Writer: terminal.NewWriter(terminal.NewCustomStdio(nil, f, f)), + Writer: f, Thread: mpctx.Tracer.NewThread(product), Status: &status.Status{}, }} diff --git a/cmd/soong_ui/main.go b/cmd/soong_ui/main.go index 5f9bd016e..58d8d345e 100644 --- a/cmd/soong_ui/main.go +++ b/cmd/soong_ui/main.go @@ -109,10 +109,7 @@ func main() { os.Exit(1) } - writer := terminal.NewWriter(c.stdio()) - defer writer.Finish() - - log := logger.New(writer) + log := logger.New(c.stdio().Stdout()) defer log.Cleanup() ctx, cancel := context.WithCancel(context.Background()) @@ -125,7 +122,7 @@ func main() { stat := &status.Status{} defer stat.Finish() - stat.AddOutput(terminal.NewStatusOutput(writer, os.Getenv("NINJA_STATUS"), + stat.AddOutput(terminal.NewStatusOutput(c.stdio().Stdout(), os.Getenv("NINJA_STATUS"), build.OsEnvironment().IsEnvTrue("ANDROID_QUIET_BUILD"))) stat.AddOutput(trace.StatusTracer()) @@ -140,7 +137,7 @@ func main() { Logger: log, Metrics: met, Tracer: trace, - Writer: writer, + Writer: c.stdio().Stdout(), Status: stat, }} @@ -312,13 +309,13 @@ func dumpVarConfig(ctx build.Context, args ...string) build.Config { func make(ctx build.Context, config build.Config, _ []string, logsDir string) { if config.IsVerbose() { writer := ctx.Writer - writer.Print("! The argument `showcommands` is no longer supported.") - writer.Print("! Instead, the verbose log is always written to a compressed file in the output dir:") - writer.Print("!") - writer.Print(fmt.Sprintf("! gzip -cd %s/verbose.log.gz | less -R", logsDir)) - writer.Print("!") - writer.Print("! Older versions are saved in verbose.log.#.gz files") - writer.Print("") + fmt.Fprintln(writer, "! The argument `showcommands` is no longer supported.") + fmt.Fprintln(writer, "! Instead, the verbose log is always written to a compressed file in the output dir:") + fmt.Fprintln(writer, "!") + fmt.Fprintf(writer, "! gzip -cd %s/verbose.log.gz | less -R\n", logsDir) + fmt.Fprintln(writer, "!") + fmt.Fprintln(writer, "! Older versions are saved in verbose.log.#.gz files") + fmt.Fprintln(writer, "") time.Sleep(5 * time.Second) } diff --git a/ui/build/config_test.go b/ui/build/config_test.go index 242e3afb0..1d23fec57 100644 --- a/ui/build/config_test.go +++ b/ui/build/config_test.go @@ -22,14 +22,13 @@ import ( "testing" "android/soong/ui/logger" - "android/soong/ui/terminal" ) func testContext() Context { return Context{&ContextImpl{ Context: context.Background(), Logger: logger.New(&bytes.Buffer{}), - Writer: terminal.NewWriter(terminal.NewCustomStdio(&bytes.Buffer{}, &bytes.Buffer{}, &bytes.Buffer{})), + Writer: &bytes.Buffer{}, }} } diff --git a/ui/build/context.go b/ui/build/context.go index 249e89822..7ff98ef78 100644 --- a/ui/build/context.go +++ b/ui/build/context.go @@ -16,12 +16,12 @@ package build import ( "context" + "io" "android/soong/ui/logger" "android/soong/ui/metrics" "android/soong/ui/metrics/metrics_proto" "android/soong/ui/status" - "android/soong/ui/terminal" "android/soong/ui/tracer" ) @@ -35,7 +35,7 @@ type ContextImpl struct { Metrics *metrics.Metrics - Writer terminal.Writer + Writer io.Writer Status *status.Status Thread tracer.Thread diff --git a/ui/build/dumpvars.go b/ui/build/dumpvars.go index 4335667d1..266130f04 100644 --- a/ui/build/dumpvars.go +++ b/ui/build/dumpvars.go @@ -249,7 +249,7 @@ func runMakeProductConfig(ctx Context, config Config) { env := config.Environment() // Print the banner like make does if !env.IsEnvTrue("ANDROID_QUIET_BUILD") { - ctx.Writer.Print(Banner(make_vars)) + fmt.Fprintln(ctx.Writer, Banner(make_vars)) } // Populate the environment diff --git a/ui/terminal/Android.bp b/ui/terminal/Android.bp index 683e3e39a..b533b0d30 100644 --- a/ui/terminal/Android.bp +++ b/ui/terminal/Android.bp @@ -21,7 +21,7 @@ bootstrap_go_package { "format.go", "smart_status.go", "status.go", - "writer.go", + "stdio.go", "util.go", ], testSrcs: [ diff --git a/ui/terminal/dumb_status.go b/ui/terminal/dumb_status.go index f2fcba79d..08ef3733a 100644 --- a/ui/terminal/dumb_status.go +++ b/ui/terminal/dumb_status.go @@ -16,19 +16,20 @@ package terminal import ( "fmt" + "io" "android/soong/ui/status" ) type dumbStatusOutput struct { - writer Writer + writer io.Writer formatter formatter } // NewDumbStatusOutput returns a StatusOutput that represents the // current build status similarly to Ninja's built-in terminal // output. -func NewDumbStatusOutput(w Writer, formatter formatter) status.StatusOutput { +func NewDumbStatusOutput(w io.Writer, formatter formatter) status.StatusOutput { return &dumbStatusOutput{ writer: w, formatter: formatter, diff --git a/ui/terminal/smart_status.go b/ui/terminal/smart_status.go index 5edc21a1a..a52fdc2b5 100644 --- a/ui/terminal/smart_status.go +++ b/ui/terminal/smart_status.go @@ -16,6 +16,7 @@ package terminal import ( "fmt" + "io" "strings" "sync" @@ -23,7 +24,7 @@ import ( ) type smartStatusOutput struct { - writer Writer + writer io.Writer formatter formatter lock sync.Mutex @@ -34,7 +35,7 @@ type smartStatusOutput struct { // NewSmartStatusOutput returns a StatusOutput that represents the // current build status similarly to Ninja's built-in terminal // output. -func NewSmartStatusOutput(w Writer, formatter formatter) status.StatusOutput { +func NewSmartStatusOutput(w io.Writer, formatter formatter) status.StatusOutput { return &smartStatusOutput{ writer: w, formatter: formatter, @@ -133,7 +134,7 @@ func (s *smartStatusOutput) statusLine(str string) { // Run this on every line in case the window has been resized while // we're printing. This could be optimized to only re-run when we get // SIGWINCH if it ever becomes too time consuming. - if max, ok := s.writer.termWidth(); ok { + if max, ok := termWidth(s.writer); ok { if len(str) > max { // TODO: Just do a max. Ninja elides the middle, but that's // more complicated and these lines aren't that important. diff --git a/ui/terminal/status.go b/ui/terminal/status.go index 481c511a0..69a2a0929 100644 --- a/ui/terminal/status.go +++ b/ui/terminal/status.go @@ -15,6 +15,8 @@ package terminal import ( + "io" + "android/soong/ui/status" ) @@ -24,10 +26,10 @@ import ( // // statusFormat takes nearly all the same options as NINJA_STATUS. // %c is currently unsupported. -func NewStatusOutput(w Writer, statusFormat string, quietBuild bool) status.StatusOutput { +func NewStatusOutput(w io.Writer, statusFormat string, quietBuild bool) status.StatusOutput { formatter := newFormatter(statusFormat, quietBuild) - if w.isSmartTerminal() { + if isSmartTerminal(w) { return NewSmartStatusOutput(w, formatter) } else { return NewDumbStatusOutput(w, formatter) diff --git a/ui/terminal/status_test.go b/ui/terminal/status_test.go index e032c1f1e..c81e837b0 100644 --- a/ui/terminal/status_test.go +++ b/ui/terminal/status_test.go @@ -89,17 +89,9 @@ func TestStatusOutput(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Run("smart", func(t *testing.T) { smart := &fakeSmartTerminal{termWidth: 40} - stdio := customStdio{ - stdin: nil, - stdout: smart, - stderr: nil, - } - - writer := NewWriter(stdio) - stat := NewStatusOutput(writer, "", false) + stat := NewStatusOutput(smart, "", false) tt.calls(stat) stat.Flush() - writer.Finish() if g, w := smart.String(), tt.smart; g != w { t.Errorf("want:\n%q\ngot:\n%q", w, g) @@ -108,17 +100,9 @@ func TestStatusOutput(t *testing.T) { t.Run("dumb", func(t *testing.T) { dumb := &bytes.Buffer{} - stdio := customStdio{ - stdin: nil, - stdout: dumb, - stderr: nil, - } - - writer := NewWriter(stdio) - stat := NewStatusOutput(writer, "", false) + stat := NewStatusOutput(dumb, "", false) tt.calls(stat) stat.Flush() - writer.Finish() if g, w := dumb.String(), tt.dumb; g != w { t.Errorf("want:\n%q\ngot:\n%q", w, g) @@ -267,15 +251,7 @@ func actionWithOuptutWithAnsiCodes(stat status.StatusOutput) { func TestSmartStatusOutputWidthChange(t *testing.T) { smart := &fakeSmartTerminal{termWidth: 40} - - stdio := customStdio{ - stdin: nil, - stdout: smart, - stderr: nil, - } - - writer := NewWriter(stdio) - stat := NewStatusOutput(writer, "", false) + stat := NewStatusOutput(smart, "", false) runner := newRunner(stat, 2) @@ -287,7 +263,6 @@ func TestSmartStatusOutputWidthChange(t *testing.T) { runner.finishAction(result) stat.Flush() - writer.Finish() w := "\r[ 0% 0/2] action with very long descrip\x1b[K\r[ 50% 1/2] action with very lo\x1b[K\n" diff --git a/ui/terminal/stdio.go b/ui/terminal/stdio.go new file mode 100644 index 000000000..dec296312 --- /dev/null +++ b/ui/terminal/stdio.go @@ -0,0 +1,55 @@ +// Copyright 2018 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package terminal provides a set of interfaces that can be used to interact +// with the terminal (including falling back when the terminal is detected to +// be a redirect or other dumb terminal) +package terminal + +import ( + "io" + "os" +) + +// StdioInterface represents a set of stdin/stdout/stderr Reader/Writers +type StdioInterface interface { + Stdin() io.Reader + Stdout() io.Writer + Stderr() io.Writer +} + +// StdioImpl uses the OS stdin/stdout/stderr to implement StdioInterface +type StdioImpl struct{} + +func (StdioImpl) Stdin() io.Reader { return os.Stdin } +func (StdioImpl) Stdout() io.Writer { return os.Stdout } +func (StdioImpl) Stderr() io.Writer { return os.Stderr } + +var _ StdioInterface = StdioImpl{} + +type customStdio struct { + stdin io.Reader + stdout io.Writer + stderr io.Writer +} + +func NewCustomStdio(stdin io.Reader, stdout, stderr io.Writer) StdioInterface { + return customStdio{stdin, stdout, stderr} +} + +func (c customStdio) Stdin() io.Reader { return c.stdin } +func (c customStdio) Stdout() io.Writer { return c.stdout } +func (c customStdio) Stderr() io.Writer { return c.stderr } + +var _ StdioInterface = customStdio{} diff --git a/ui/terminal/writer.go b/ui/terminal/writer.go deleted file mode 100644 index 26e0e342f..000000000 --- a/ui/terminal/writer.go +++ /dev/null @@ -1,122 +0,0 @@ -// Copyright 2018 Google Inc. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package terminal provides a set of interfaces that can be used to interact -// with the terminal (including falling back when the terminal is detected to -// be a redirect or other dumb terminal) -package terminal - -import ( - "fmt" - "io" - "os" -) - -// Writer provides an interface to write temporary and permanent messages to -// the terminal. -// -// The terminal is considered to be a dumb terminal if TERM==dumb, or if a -// terminal isn't detected on stdout/stderr (generally because it's a pipe or -// file). Dumb terminals will strip out all ANSI escape sequences, including -// colors. -type Writer interface { - // Print prints the string to the terminal, overwriting any current - // status being displayed. - // - // On a dumb terminal, the status messages will be kept. - Print(str string) - - // Finish ensures that the output ends with a newline (preserving any - // current status line that is current displayed). - // - // This does nothing on dumb terminals. - Finish() - - // Write implements the io.Writer interface. This is primarily so that - // the logger can use this interface to print to stderr without - // breaking the other semantics of this interface. - // - // Try to use any of the other functions if possible. - Write(p []byte) (n int, err error) - - isSmartTerminal() bool - termWidth() (int, bool) -} - -// NewWriter creates a new Writer based on the stdio and the TERM -// environment variable. -func NewWriter(stdio StdioInterface) Writer { - w := &writerImpl{ - stdio: stdio, - } - - return w -} - -type writerImpl struct { - stdio StdioInterface -} - -func (w *writerImpl) Print(str string) { - fmt.Fprint(w.stdio.Stdout(), str) - if len(str) == 0 || str[len(str)-1] != '\n' { - fmt.Fprint(w.stdio.Stdout(), "\n") - } -} - -func (w *writerImpl) Finish() {} - -func (w *writerImpl) Write(p []byte) (n int, err error) { - return w.stdio.Stdout().Write(p) -} - -func (w *writerImpl) isSmartTerminal() bool { - return isSmartTerminal(w.stdio.Stdout()) -} - -func (w *writerImpl) termWidth() (int, bool) { - return termWidth(w.stdio.Stdout()) -} - -// StdioInterface represents a set of stdin/stdout/stderr Reader/Writers -type StdioInterface interface { - Stdin() io.Reader - Stdout() io.Writer - Stderr() io.Writer -} - -// StdioImpl uses the OS stdin/stdout/stderr to implement StdioInterface -type StdioImpl struct{} - -func (StdioImpl) Stdin() io.Reader { return os.Stdin } -func (StdioImpl) Stdout() io.Writer { return os.Stdout } -func (StdioImpl) Stderr() io.Writer { return os.Stderr } - -var _ StdioInterface = StdioImpl{} - -type customStdio struct { - stdin io.Reader - stdout io.Writer - stderr io.Writer -} - -func NewCustomStdio(stdin io.Reader, stdout, stderr io.Writer) StdioInterface { - return customStdio{stdin, stdout, stderr} -} - -func (c customStdio) Stdin() io.Reader { return c.stdin } -func (c customStdio) Stdout() io.Writer { return c.stdout } -func (c customStdio) Stderr() io.Writer { return c.stderr } - -var _ StdioInterface = customStdio{} From e0df1a36b265ce271a9a723a09ecf892ffefe36f Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Sun, 9 Jun 2019 19:40:08 -0700 Subject: [PATCH 4/6] Move all output through StatusOutput Write log output through StatusOutput so that the status implementation can synchronize it with its own output. Test: status_test.go Change-Id: I917bdeeea4759a12b6b4aa6d6d86ee18a2771723 --- cmd/multiproduct_kati/main.go | 17 ++++++++++++----- cmd/soong_ui/main.go | 10 ++++++---- ui/status/log.go | 10 ++++++++++ ui/status/status.go | 3 +++ ui/status/status_test.go | 5 +++++ ui/terminal/dumb_status.go | 5 +++++ ui/terminal/smart_status.go | 7 +++++++ ui/tracer/status.go | 5 +++++ 8 files changed, 53 insertions(+), 9 deletions(-) diff --git a/cmd/multiproduct_kati/main.go b/cmd/multiproduct_kati/main.go index c976dcb45..1171a6521 100644 --- a/cmd/multiproduct_kati/main.go +++ b/cmd/multiproduct_kati/main.go @@ -158,7 +158,10 @@ type mpContext struct { func main() { stdio := terminal.StdioImpl{} - log := logger.New(stdio.Stdout()) + output := terminal.NewStatusOutput(stdio.Stdout(), "", + build.OsEnvironment().IsEnvTrue("ANDROID_QUIET_BUILD")) + + log := logger.New(output) defer log.Cleanup() flag.Parse() @@ -171,8 +174,7 @@ func main() { stat := &status.Status{} defer stat.Finish() - stat.AddOutput(terminal.NewStatusOutput(stdio.Stdout(), "", - build.OsEnvironment().IsEnvTrue("ANDROID_QUIET_BUILD"))) + stat.AddOutput(output) var failures failureCount stat.AddOutput(&failures) @@ -187,7 +189,7 @@ func main() { Context: ctx, Logger: log, Tracer: trace, - Writer: stdio.Stdout(), + Writer: output, Status: stat, }} @@ -340,7 +342,7 @@ func main() { } else if failures > 1 { log.Fatalf("%d failures", failures) } else { - fmt.Fprintln(stdio.Stdout(), "Success") + fmt.Fprintln(output, "Success") } } @@ -465,3 +467,8 @@ func (f *failureCount) Message(level status.MsgLevel, message string) { } func (f *failureCount) Flush() {} + +func (f *failureCount) Write(p []byte) (int, error) { + // discard writes + return len(p), nil +} diff --git a/cmd/soong_ui/main.go b/cmd/soong_ui/main.go index 58d8d345e..f5276c335 100644 --- a/cmd/soong_ui/main.go +++ b/cmd/soong_ui/main.go @@ -109,7 +109,10 @@ func main() { os.Exit(1) } - log := logger.New(c.stdio().Stdout()) + output := terminal.NewStatusOutput(c.stdio().Stdout(), os.Getenv("NINJA_STATUS"), + build.OsEnvironment().IsEnvTrue("ANDROID_QUIET_BUILD")) + + log := logger.New(output) defer log.Cleanup() ctx, cancel := context.WithCancel(context.Background()) @@ -122,8 +125,7 @@ func main() { stat := &status.Status{} defer stat.Finish() - stat.AddOutput(terminal.NewStatusOutput(c.stdio().Stdout(), os.Getenv("NINJA_STATUS"), - build.OsEnvironment().IsEnvTrue("ANDROID_QUIET_BUILD"))) + stat.AddOutput(output) stat.AddOutput(trace.StatusTracer()) build.SetupSignals(log, cancel, func() { @@ -137,7 +139,7 @@ func main() { Logger: log, Metrics: met, Tracer: trace, - Writer: c.stdio().Stdout(), + Writer: output, Status: stat, }} diff --git a/ui/status/log.go b/ui/status/log.go index 921aa4401..7badac73c 100644 --- a/ui/status/log.go +++ b/ui/status/log.go @@ -71,6 +71,11 @@ func (v *verboseLog) Message(level MsgLevel, message string) { fmt.Fprintf(v.w, "%s%s\n", level.Prefix(), message) } +func (v *verboseLog) Write(p []byte) (int, error) { + fmt.Fprint(v.w, string(p)) + return len(p), nil +} + type errorLog struct { w io.WriteCloser @@ -134,3 +139,8 @@ func (e *errorLog) Message(level MsgLevel, message string) { fmt.Fprintf(e.w, "error: %s\n", message) } + +func (e *errorLog) Write(p []byte) (int, error) { + fmt.Fprint(e.w, string(p)) + return len(p), nil +} diff --git a/ui/status/status.go b/ui/status/status.go index 46ec72e80..3d8cd7a2c 100644 --- a/ui/status/status.go +++ b/ui/status/status.go @@ -173,6 +173,9 @@ type StatusOutput interface { // Flush is called when your outputs should be flushed / closed. No // output is expected after this call. Flush() + + // Write lets StatusOutput implement io.Writer + Write(p []byte) (n int, err error) } // Status is the multiplexer / accumulator between ToolStatus instances (via diff --git a/ui/status/status_test.go b/ui/status/status_test.go index e62785f43..949458222 100644 --- a/ui/status/status_test.go +++ b/ui/status/status_test.go @@ -27,6 +27,11 @@ func (c *counterOutput) FinishAction(result ActionResult, counts Counts) { func (c counterOutput) Message(level MsgLevel, msg string) {} func (c counterOutput) Flush() {} +func (c counterOutput) Write(p []byte) (int, error) { + // Discard writes + return len(p), nil +} + func (c counterOutput) Expect(t *testing.T, counts Counts) { if Counts(c) == counts { return diff --git a/ui/terminal/dumb_status.go b/ui/terminal/dumb_status.go index 08ef3733a..201770fac 100644 --- a/ui/terminal/dumb_status.go +++ b/ui/terminal/dumb_status.go @@ -64,3 +64,8 @@ func (s *dumbStatusOutput) FinishAction(result status.ActionResult, counts statu } func (s *dumbStatusOutput) Flush() {} + +func (s *dumbStatusOutput) Write(p []byte) (int, error) { + fmt.Fprint(s.writer, string(p)) + return len(p), nil +} diff --git a/ui/terminal/smart_status.go b/ui/terminal/smart_status.go index a52fdc2b5..9a4931c01 100644 --- a/ui/terminal/smart_status.go +++ b/ui/terminal/smart_status.go @@ -104,6 +104,13 @@ func (s *smartStatusOutput) Flush() { s.requestLine() } +func (s *smartStatusOutput) Write(p []byte) (int, error) { + s.lock.Lock() + defer s.lock.Unlock() + s.print(string(p)) + return len(p), nil +} + func (s *smartStatusOutput) requestLine() { if !s.haveBlankLine { fmt.Fprintln(s.writer) diff --git a/ui/tracer/status.go b/ui/tracer/status.go index af50e2d4d..c83125514 100644 --- a/ui/tracer/status.go +++ b/ui/tracer/status.go @@ -85,3 +85,8 @@ func (s *statusOutput) FinishAction(result status.ActionResult, counts status.Co func (s *statusOutput) Flush() {} func (s *statusOutput) Message(level status.MsgLevel, message string) {} + +func (s *statusOutput) Write(p []byte) (int, error) { + // Discard writes + return len(p), nil +} From 00bdfd8476b031fb862cbfef1f122d7bff1e0acd Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 11 Jun 2019 11:16:23 -0700 Subject: [PATCH 5/6] Make status line bold Bolding the status line provides differentiation between output of each command. Test: status_test.go Change-Id: I9d46761e69c5af0a0aa86c7921e121cfd2a3fc82 --- ui/terminal/smart_status.go | 8 +++++--- ui/terminal/status_test.go | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/ui/terminal/smart_status.go b/ui/terminal/smart_status.go index 9a4931c01..8fa9effcc 100644 --- a/ui/terminal/smart_status.go +++ b/ui/terminal/smart_status.go @@ -149,8 +149,10 @@ func (s *smartStatusOutput) statusLine(str string) { } } - // Move to the beginning on the line, print the output, then clear - // the rest of the line. - fmt.Fprint(s.writer, "\r", str, "\x1b[K") + // Move to the beginning on the line, turn on bold, print the output, + // turn off bold, then clear the rest of the line. + start := "\r\x1b[1m" + end := "\x1b[0m\x1b[K" + fmt.Fprint(s.writer, start, str, end) s.haveBlankLine = false } diff --git a/ui/terminal/status_test.go b/ui/terminal/status_test.go index c81e837b0..a87a7f07d 100644 --- a/ui/terminal/status_test.go +++ b/ui/terminal/status_test.go @@ -32,55 +32,55 @@ func TestStatusOutput(t *testing.T) { { name: "two actions", calls: twoActions, - smart: "\r[ 0% 0/2] action1\x1b[K\r[ 50% 1/2] action1\x1b[K\r[ 50% 1/2] action2\x1b[K\r[100% 2/2] action2\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[1m[ 50% 1/2] action2\x1b[0m\x1b[K\r\x1b[1m[100% 2/2] action2\x1b[0m\x1b[K\n", dumb: "[ 50% 1/2] action1\n[100% 2/2] action2\n", }, { name: "two parallel actions", calls: twoParallelActions, - smart: "\r[ 0% 0/2] action1\x1b[K\r[ 0% 0/2] action2\x1b[K\r[ 50% 1/2] action1\x1b[K\r[100% 2/2] action2\x1b[K\n", + smart: "\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\r\x1b[1m[100% 2/2] action2\x1b[0m\x1b[K\n", dumb: "[ 50% 1/2] action1\n[100% 2/2] action2\n", }, { name: "action with output", calls: actionsWithOutput, - smart: "\r[ 0% 0/3] action1\x1b[K\r[ 33% 1/3] action1\x1b[K\r[ 33% 1/3] action2\x1b[K\r[ 66% 2/3] action2\x1b[K\noutput1\noutput2\n\r[ 66% 2/3] action3\x1b[K\r[100% 3/3] action3\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\noutput1\noutput2\n\r\x1b[1m[ 66% 2/3] action3\x1b[0m\x1b[K\r\x1b[1m[100% 3/3] action3\x1b[0m\x1b[K\n", dumb: "[ 33% 1/3] action1\n[ 66% 2/3] action2\noutput1\noutput2\n[100% 3/3] action3\n", }, { name: "action with output without newline", calls: actionsWithOutputWithoutNewline, - smart: "\r[ 0% 0/3] action1\x1b[K\r[ 33% 1/3] action1\x1b[K\r[ 33% 1/3] action2\x1b[K\r[ 66% 2/3] action2\x1b[K\noutput1\noutput2\n\r[ 66% 2/3] action3\x1b[K\r[100% 3/3] action3\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\noutput1\noutput2\n\r\x1b[1m[ 66% 2/3] action3\x1b[0m\x1b[K\r\x1b[1m[100% 3/3] action3\x1b[0m\x1b[K\n", dumb: "[ 33% 1/3] action1\n[ 66% 2/3] action2\noutput1\noutput2\n[100% 3/3] action3\n", }, { name: "action with error", calls: actionsWithError, - smart: "\r[ 0% 0/3] action1\x1b[K\r[ 33% 1/3] action1\x1b[K\r[ 33% 1/3] action2\x1b[K\r[ 66% 2/3] action2\x1b[K\nFAILED: f1 f2\ntouch f1 f2\nerror1\nerror2\n\r[ 66% 2/3] action3\x1b[K\r[100% 3/3] action3\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\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", dumb: "[ 33% 1/3] action1\n[ 66% 2/3] action2\nFAILED: f1 f2\ntouch f1 f2\nerror1\nerror2\n[100% 3/3] action3\n", }, { name: "action with empty description", calls: actionWithEmptyDescription, - smart: "\r[ 0% 0/1] command1\x1b[K\r[100% 1/1] command1\x1b[K\n", + smart: "\r\x1b[1m[ 0% 0/1] command1\x1b[0m\x1b[K\r\x1b[1m[100% 1/1] command1\x1b[0m\x1b[K\n", dumb: "[100% 1/1] command1\n", }, { name: "messages", calls: actionsWithMessages, - smart: "\r[ 0% 0/2] action1\x1b[K\r[ 50% 1/2] action1\x1b[K\rstatus\x1b[K\r\x1b[Kprint\nFAILED: error\n\r[ 50% 1/2] action2\x1b[K\r[100% 2/2] action2\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\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", dumb: "[ 50% 1/2] action1\nstatus\nprint\nFAILED: error\n[100% 2/2] action2\n", }, { name: "action with long description", calls: actionWithLongDescription, - smart: "\r[ 0% 0/2] action with very long descrip\x1b[K\r[ 50% 1/2] action with very long descrip\x1b[K\n", + smart: "\r\x1b[1m[ 0% 0/2] action with very long descrip\x1b[0m\x1b[K\r\x1b[1m[ 50% 1/2] action with very long descrip\x1b[0m\x1b[K\n", dumb: "[ 50% 1/2] action with very long description to test eliding\n", }, { name: "action with output with ansi codes", calls: actionWithOuptutWithAnsiCodes, - smart: "\r[ 0% 0/1] action1\x1b[K\r[100% 1/1] action1\x1b[K\n\x1b[31mcolor\x1b[0m\n", + smart: "\r\x1b[1m[ 0% 0/1] action1\x1b[0m\x1b[K\r\x1b[1m[100% 1/1] action1\x1b[0m\x1b[K\n\x1b[31mcolor\x1b[0m\n", dumb: "[100% 1/1] action1\ncolor\n", }, } @@ -264,7 +264,7 @@ func TestSmartStatusOutputWidthChange(t *testing.T) { stat.Flush() - w := "\r[ 0% 0/2] action with very long descrip\x1b[K\r[ 50% 1/2] action with very lo\x1b[K\n" + w := "\r\x1b[1m[ 0% 0/2] action with very long descrip\x1b[0m\x1b[K\r\x1b[1m[ 50% 1/2] action with very lo\x1b[0m\x1b[K\n" if g := smart.String(); g != w { t.Errorf("want:\n%q\ngot:\n%q", w, g) From 49036be4076851c4ef2003339ffd1bf022dfc844 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 11 Jun 2019 23:01:36 -0700 Subject: [PATCH 6/6] Use SIGWINCH to update terminal size Instead of reading the terminal size on every status update, register for SIGWINCH to read and store the size when it changes. Test: status_test.go Change-Id: I555ad21a31a2c924ab0ca681e0c8f00df42a370a --- ui/terminal/smart_status.go | 62 ++++++++++++++++++++++++++++++------- ui/terminal/status_test.go | 3 ++ 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/ui/terminal/smart_status.go b/ui/terminal/smart_status.go index 8fa9effcc..999a2d0f1 100644 --- a/ui/terminal/smart_status.go +++ b/ui/terminal/smart_status.go @@ -17,8 +17,11 @@ package terminal import ( "fmt" "io" + "os" + "os/signal" "strings" "sync" + "syscall" "android/soong/ui/status" ) @@ -30,18 +33,29 @@ type smartStatusOutput struct { lock sync.Mutex haveBlankLine bool + + termWidth int + sigwinch chan os.Signal } // NewSmartStatusOutput returns a StatusOutput that represents the // current build status similarly to Ninja's built-in terminal // output. func NewSmartStatusOutput(w io.Writer, formatter formatter) status.StatusOutput { - return &smartStatusOutput{ + s := &smartStatusOutput{ writer: w, formatter: formatter, haveBlankLine: true, + + sigwinch: make(chan os.Signal), } + + s.updateTermSize() + + s.startSigwinch() + + return s } func (s *smartStatusOutput) Message(level status.MsgLevel, message string) { @@ -101,6 +115,8 @@ func (s *smartStatusOutput) Flush() { s.lock.Lock() defer s.lock.Unlock() + s.stopSigwinch() + s.requestLine() } @@ -137,16 +153,8 @@ func (s *smartStatusOutput) statusLine(str string) { // Limit line width to the terminal width, otherwise we'll wrap onto // another line and we won't delete the previous line. - // - // Run this on every line in case the window has been resized while - // we're printing. This could be optimized to only re-run when we get - // SIGWINCH if it ever becomes too time consuming. - if max, ok := termWidth(s.writer); ok { - if len(str) > max { - // TODO: Just do a max. Ninja elides the middle, but that's - // more complicated and these lines aren't that important. - str = str[:max] - } + if s.termWidth > 0 { + str = s.elide(str) } // Move to the beginning on the line, turn on bold, print the output, @@ -156,3 +164,35 @@ func (s *smartStatusOutput) statusLine(str string) { fmt.Fprint(s.writer, start, str, end) s.haveBlankLine = false } + +func (s *smartStatusOutput) elide(str string) string { + if len(str) > s.termWidth { + // TODO: Just do a max. Ninja elides the middle, but that's + // more complicated and these lines aren't that important. + str = str[:s.termWidth] + } + + return str +} + +func (s *smartStatusOutput) startSigwinch() { + signal.Notify(s.sigwinch, syscall.SIGWINCH) + go func() { + for _ = range s.sigwinch { + s.lock.Lock() + s.updateTermSize() + s.lock.Unlock() + } + }() +} + +func (s *smartStatusOutput) stopSigwinch() { + signal.Stop(s.sigwinch) + close(s.sigwinch) +} + +func (s *smartStatusOutput) updateTermSize() { + if w, ok := termWidth(s.writer); ok { + s.termWidth = w + } +} diff --git a/ui/terminal/status_test.go b/ui/terminal/status_test.go index a87a7f07d..fc9315b96 100644 --- a/ui/terminal/status_test.go +++ b/ui/terminal/status_test.go @@ -17,6 +17,7 @@ package terminal import ( "bytes" "fmt" + "syscall" "testing" "android/soong/ui/status" @@ -260,6 +261,8 @@ func TestSmartStatusOutputWidthChange(t *testing.T) { runner.startAction(action) smart.termWidth = 30 + // Fake a SIGWINCH + stat.(*smartStatusOutput).sigwinch <- syscall.SIGWINCH runner.finishAction(result) stat.Flush()