Merge changes If92421af,I2e434f4c,I8735032e

* changes:
  Force dumpvars into dumb terminal mode and move log files
  status table: don't write newlines in non-scrolling terminals
  status table: ANSI code rows and columns are 1-based

cmd/multiproduct_kati/main.go

@@ -158,7 +158,7 @@ type mpContext struct {
 func main() {
 	stdio := terminal.StdioImpl{}
 
-	output := terminal.NewStatusOutput(stdio.Stdout(), "",
+	output := terminal.NewStatusOutput(stdio.Stdout(), "", false,
 		build.OsEnvironment().IsEnvTrue("ANDROID_QUIET_BUILD"))
 
 	log := logger.New(output)
@@ -391,7 +391,7 @@ func buildProduct(mpctx *mpContext, product string) {
 		Thread:  mpctx.Tracer.NewThread(product),
 		Status:  &status.Status{},
 	}}
-	ctx.Status.AddOutput(terminal.NewStatusOutput(ctx.Writer, "",
+	ctx.Status.AddOutput(terminal.NewStatusOutput(ctx.Writer, "", false,
 		build.OsEnvironment().IsEnvTrue("ANDROID_QUIET_BUILD")))
 
 	config := build.NewConfig(ctx, flag.Args()...)

cmd/soong_ui/main.go

@@ -41,6 +41,12 @@ type command struct {
 	// description for the flag (to display when running help)
 	description string
 
+	// Forces the status output into dumb terminal mode.
+	forceDumbOutput bool
+
+	// Sets a prefix string to use for filenames of log files.
+	logsPrefix string
+
 	// Creates the build configuration based on the args and build context.
 	config func(ctx build.Context, args ...string) build.Config
 
@@ -64,17 +70,21 @@ var commands []command = []command{
 		stdio: stdio,
 		run:   make,
 	}, {
-		flag:        "--dumpvar-mode",
-		description: "print the value of the legacy make variable VAR to stdout",
-		config:      dumpVarConfig,
-		stdio:       customStdio,
-		run:         dumpVar,
+		flag:            "--dumpvar-mode",
+		description:     "print the value of the legacy make variable VAR to stdout",
+		forceDumbOutput: true,
+		logsPrefix:      "dumpvars-",
+		config:          dumpVarConfig,
+		stdio:           customStdio,
+		run:             dumpVar,
 	}, {
-		flag:        "--dumpvars-mode",
-		description: "dump the values of one or more legacy make variables, in shell syntax",
-		config:      dumpVarConfig,
-		stdio:       customStdio,
-		run:         dumpVars,
+		flag:            "--dumpvars-mode",
+		description:     "dump the values of one or more legacy make variables, in shell syntax",
+		forceDumbOutput: true,
+		logsPrefix:      "dumpvars-",
+		config:          dumpVarConfig,
+		stdio:           customStdio,
+		run:             dumpVars,
 	}, {
 		flag:        "--build-mode",
 		description: "build modules based on the specified build action",
@@ -113,7 +123,7 @@ func main() {
 		os.Exit(1)
 	}
 
-	output := terminal.NewStatusOutput(c.stdio().Stdout(), os.Getenv("NINJA_STATUS"),
+	output := terminal.NewStatusOutput(c.stdio().Stdout(), os.Getenv("NINJA_STATUS"), c.forceDumbOutput,
 		build.OsEnvironment().IsEnvTrue("ANDROID_QUIET_BUILD"))
 
 	log := logger.New(output)
@@ -157,14 +167,14 @@ func main() {
 	}
 
 	os.MkdirAll(logsDir, 0777)
-	log.SetOutput(filepath.Join(logsDir, "soong.log"))
-	trace.SetOutput(filepath.Join(logsDir, "build.trace"))
-	stat.AddOutput(status.NewVerboseLog(log, filepath.Join(logsDir, "verbose.log")))
-	stat.AddOutput(status.NewErrorLog(log, filepath.Join(logsDir, "error.log")))
-	stat.AddOutput(status.NewProtoErrorLog(log, filepath.Join(logsDir, "build_error")))
+	log.SetOutput(filepath.Join(logsDir, c.logsPrefix+"soong.log"))
+	trace.SetOutput(filepath.Join(logsDir, c.logsPrefix+"build.trace"))
+	stat.AddOutput(status.NewVerboseLog(log, filepath.Join(logsDir, c.logsPrefix+"verbose.log")))
+	stat.AddOutput(status.NewErrorLog(log, filepath.Join(logsDir, c.logsPrefix+"error.log")))
+	stat.AddOutput(status.NewProtoErrorLog(log, filepath.Join(logsDir, c.logsPrefix+"build_error")))
 	stat.AddOutput(status.NewCriticalPath(log))
 
-	defer met.Dump(filepath.Join(logsDir, "soong_metrics"))
+	defer met.Dump(filepath.Join(logsDir, c.logsPrefix+"soong_metrics"))
 
 	if start, ok := os.LookupEnv("TRACE_BEGIN_SOONG"); ok {
 		if !strings.HasSuffix(start, "N") {

ui/terminal/smart_status.go

@@ -189,7 +189,7 @@ func (s *smartStatusOutput) Flush() {
 		fmt.Fprintf(s.writer, ansi.resetScrollingMargins())
 		_, height, _ := termSize(s.writer)
 		// Move the cursor to the top of the now-blank, previously non-scrolling region
-		fmt.Fprintf(s.writer, ansi.setCursor(height-s.tableHeight, 0))
+		fmt.Fprintf(s.writer, ansi.setCursor(height-s.tableHeight, 1))
 		// Turn the cursor back on
 		fmt.Fprintf(s.writer, ansi.showCursor())
 	}
@@ -334,52 +334,44 @@ func (s *smartStatusOutput) actionTable() {
 	scrollingHeight := s.termHeight - s.tableHeight
 
 	// Update the scrolling region in case the height of the terminal changed
-	fmt.Fprint(s.writer, ansi.setScrollingMargins(0, scrollingHeight))
-	// Move the cursor to the first line of the non-scrolling region
-	fmt.Fprint(s.writer, ansi.setCursor(scrollingHeight+1, 0))
+
+	fmt.Fprint(s.writer, ansi.setScrollingMargins(1, scrollingHeight))
 
 	// Write as many status lines as fit in the table
-	var tableLine int
-	var runningAction actionTableEntry
-	for tableLine, runningAction = range s.runningActions {
+	for tableLine := 0; tableLine < s.tableHeight; tableLine++ {
 		if tableLine >= s.tableHeight {
 			break
 		}
+		// Move the cursor to the correct line of the non-scrolling region
+		fmt.Fprint(s.writer, ansi.setCursor(scrollingHeight+1+tableLine, 1))
 
-		seconds := int(time.Since(runningAction.startTime).Round(time.Second).Seconds())
+		if tableLine < len(s.runningActions) {
+			runningAction := s.runningActions[tableLine]
 
-		desc := runningAction.action.Description
-		if desc == "" {
-			desc = runningAction.action.Command
+			seconds := int(time.Since(runningAction.startTime).Round(time.Second).Seconds())
+
+			desc := runningAction.action.Description
+			if desc == "" {
+				desc = runningAction.action.Command
+			}
+
+			color := ""
+			if seconds >= 60 {
+				color = ansi.red() + ansi.bold()
+			} else if seconds >= 30 {
+				color = ansi.yellow() + ansi.bold()
+			}
+
+			durationStr := fmt.Sprintf("   %2d:%02d ", seconds/60, seconds%60)
+			desc = elide(desc, s.termWidth-len(durationStr))
+			durationStr = color + durationStr + ansi.regular()
+			fmt.Fprint(s.writer, durationStr, desc)
 		}
-
-		color := ""
-		if seconds >= 60 {
-			color = ansi.red() + ansi.bold()
-		} else if seconds >= 30 {
-			color = ansi.yellow() + ansi.bold()
-		}
-
-		durationStr := fmt.Sprintf("   %2d:%02d ", seconds/60, seconds%60)
-		desc = elide(desc, s.termWidth-len(durationStr))
-		durationStr = color + durationStr + ansi.regular()
-
-		fmt.Fprint(s.writer, durationStr, desc, ansi.clearToEndOfLine())
-		if tableLine < s.tableHeight-1 {
-			fmt.Fprint(s.writer, "\n")
-		}
-	}
-
-	// Clear any remaining lines in the table
-	for ; tableLine < s.tableHeight; tableLine++ {
 		fmt.Fprint(s.writer, ansi.clearToEndOfLine())
-		if tableLine < s.tableHeight-1 {
-			fmt.Fprint(s.writer, "\n")
-		}
 	}
 
 	// Move the cursor back to the last line of the scrolling region
-	fmt.Fprint(s.writer, ansi.setCursor(scrollingHeight, 0))
+	fmt.Fprint(s.writer, ansi.setCursor(scrollingHeight, 1))
 }
 
 var ansi = ansiImpl{}

ui/terminal/status.go

@@ -26,10 +26,10 @@ import (
 //
 // statusFormat takes nearly all the same options as NINJA_STATUS.
 // %c is currently unsupported.
-func NewStatusOutput(w io.Writer, statusFormat string, quietBuild bool) status.StatusOutput {
+func NewStatusOutput(w io.Writer, statusFormat string, forceDumbOutput, quietBuild bool) status.StatusOutput {
 	formatter := newFormatter(statusFormat, quietBuild)
 
-	if isSmartTerminal(w) {
+	if !forceDumbOutput && isSmartTerminal(w) {
 		return NewSmartStatusOutput(w, formatter)
 	} else {
 		return NewDumbStatusOutput(w, formatter)

ui/terminal/status_test.go

@@ -94,7 +94,7 @@ func TestStatusOutput(t *testing.T) {
 
 			t.Run("smart", func(t *testing.T) {
 				smart := &fakeSmartTerminal{termWidth: 40}
-				stat := NewStatusOutput(smart, "", false)
+				stat := NewStatusOutput(smart, "", false, false)
 				tt.calls(stat)
 				stat.Flush()
 
@@ -105,7 +105,7 @@ func TestStatusOutput(t *testing.T) {
 
 			t.Run("dumb", func(t *testing.T) {
 				dumb := &bytes.Buffer{}
-				stat := NewStatusOutput(dumb, "", false)
+				stat := NewStatusOutput(dumb, "", false, false)
 				tt.calls(stat)
 				stat.Flush()
 
@@ -113,6 +113,17 @@ func TestStatusOutput(t *testing.T) {
 					t.Errorf("want:\n%q\ngot:\n%q", w, g)
 				}
 			})
+
+			t.Run("force dumb", func(t *testing.T) {
+				smart := &fakeSmartTerminal{termWidth: 40}
+				stat := NewStatusOutput(smart, "", true, false)
+				tt.calls(stat)
+				stat.Flush()
+
+				if g, w := smart.String(), tt.dumb; g != w {
+					t.Errorf("want:\n%q\ngot:\n%q", w, g)
+				}
+			})
 		})
 	}
 }
@@ -258,7 +269,7 @@ func TestSmartStatusOutputWidthChange(t *testing.T) {
 	os.Setenv(tableHeightEnVar, "")
 
 	smart := &fakeSmartTerminal{termWidth: 40}
-	stat := NewStatusOutput(smart, "", false)
+	stat := NewStatusOutput(smart, "", false, false)
 	smartStat := stat.(*smartStatusOutput)
 	smartStat.sigwinchHandled = make(chan bool)