Revert "Decouple persist and display events (#15529)" (#15705)

<!--- 
Thanks so much for your contribution! If this is your first time
contributing, please ensure that you have read the
[CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md)
documentation.
-->

# Description

<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. -->

This reverts commit ff7a7b4.

Fixes #15668.

changelog/pending/20240315--cli-display--fix-superfluous-newlines-being-written-during-updates.yaml

@@ -0,0 +1,4 @@
+changes:
+- type: fix
+  scope: cli/display
+  description: Fix superfluous newlines being written during updates

pkg/backend/display/jsonmessage.go

@@ -79,7 +79,6 @@ type messageRenderer struct {
 	opts          Options
 	isInteractive bool
 
-	display        *ProgressDisplay
 	terminal       terminal.Terminal
 	terminalWidth  int
 	terminalHeight int
@@ -140,10 +139,6 @@ func (r *messageRenderer) Close() error {
 	return nil
 }
 
-func (r *messageRenderer) initializeDisplay(display *ProgressDisplay) {
-	r.display = display
-}
-
 // Converts the colorization tags in a progress message and then actually writes the progress
 // message to the output stream.  This should be the only place in this file where we actually
 // process colorization tags.
@@ -179,20 +174,21 @@ func (r *messageRenderer) writeSimpleMessage(msg string) {
 	r.colorizeAndWriteProgress(makeMessageProgress(msg))
 }
 
-func (r *messageRenderer) println(line string) {
+func (r *messageRenderer) println(display *ProgressDisplay, line string) {
 	r.writeSimpleMessage(line)
 }
 
-func (r *messageRenderer) tick() {
+func (r *messageRenderer) tick(display *ProgressDisplay) {
 	if r.isInteractive {
-		r.render(false)
+		r.render(display, false)
 	} else {
 		// Update the spinner to let the user know that that work is still happening.
 		r.nonInteractiveSpinner.Tick()
 	}
 }
 
-func (r *messageRenderer) renderRow(id string, colorizedColumns []string, maxColumnLengths []int,
+func (r *messageRenderer) renderRow(display *ProgressDisplay,
+	id string, colorizedColumns []string, maxColumnLengths []int,
 ) {
 	row := renderRow(colorizedColumns, maxColumnLengths)
 	if r.isInteractive {
@@ -216,50 +212,50 @@ func (r *messageRenderer) renderRow(id string, colorizedColumns []string, maxCol
 	}
 }
 
-func (r *messageRenderer) rowUpdated(row Row) {
+func (r *messageRenderer) rowUpdated(display *ProgressDisplay, row Row) {
 	if r.isInteractive {
 		// if we're in a terminal, then refresh everything so that all our columns line up
-		r.render(false)
+		r.render(display, false)
 	} else {
 		// otherwise, just print out this single row.
 		colorizedColumns := row.ColorizedColumns()
-		colorizedColumns[r.display.suffixColumn] += row.ColorizedSuffix()
-		r.renderRow("", colorizedColumns, nil)
+		colorizedColumns[display.suffixColumn] += row.ColorizedSuffix()
+		r.renderRow(display, "", colorizedColumns, nil)
 	}
 }
 
-func (r *messageRenderer) systemMessage(payload engine.StdoutEventPayload) {
+func (r *messageRenderer) systemMessage(display *ProgressDisplay, payload engine.StdoutEventPayload) {
 	if r.isInteractive {
 		// if we're in a terminal, then refresh everything.  The system events will come after
 		// all the normal rows
-		r.render(false)
+		r.render(display, false)
 	} else {
 		// otherwise, in a non-terminal, just print out the actual event.
-		r.writeSimpleMessage(renderStdoutColorEvent(payload, r.display.opts))
+		r.writeSimpleMessage(renderStdoutColorEvent(payload, display.opts))
 	}
 }
 
-func (r *messageRenderer) done() {
+func (r *messageRenderer) done(display *ProgressDisplay) {
 	if r.isInteractive {
-		r.render(false)
+		r.render(display, false)
 	}
 }
 
-func (r *messageRenderer) render(done bool) {
-	if !r.isInteractive || r.display.headerRow == nil {
+func (r *messageRenderer) render(display *ProgressDisplay, done bool) {
+	if !r.isInteractive || display.headerRow == nil {
 		return
 	}
 
 	// make sure our stored dimension info is up to date
 	r.updateTerminalDimensions()
 
-	rootNodes := r.display.generateTreeNodes()
-	rootNodes = r.display.filterOutUnnecessaryNodesAndSetDisplayTimes(rootNodes)
+	rootNodes := display.generateTreeNodes()
+	rootNodes = display.filterOutUnnecessaryNodesAndSetDisplayTimes(rootNodes)
 	sortNodes(rootNodes)
-	r.display.addIndentations(rootNodes, true /*isRoot*/, "")
+	display.addIndentations(rootNodes, true /*isRoot*/, "")
 
 	maxSuffixLength := 0
-	for _, v := range r.display.suffixesArray {
+	for _, v := range display.suffixesArray {
 		runeCount := utf8.RuneCountInString(v)
 		if runeCount > maxSuffixLength {
 			maxSuffixLength = runeCount
@@ -268,18 +264,18 @@ func (r *messageRenderer) render(done bool) {
 
 	var rows [][]string
 	var maxColumnLengths []int
-	r.display.convertNodesToRows(rootNodes, maxSuffixLength, &rows, &maxColumnLengths)
+	display.convertNodesToRows(rootNodes, maxSuffixLength, &rows, &maxColumnLengths)
 
 	removeInfoColumnIfUnneeded(rows)
 
 	for i, row := range rows {
-		r.renderRow(strconv.Itoa(i), row, maxColumnLengths)
+		r.renderRow(display, strconv.Itoa(i), row, maxColumnLengths)
 	}
 
 	systemID := len(rows)
 
 	printedHeader := false
-	for _, payload := range r.display.systemEventPayloads {
+	for _, payload := range display.systemEventPayloads {
 		msg := payload.Color.Colorize(payload.Message)
 		lines := splitIntoDisplayableLines(msg)
 
@@ -307,7 +303,7 @@ func (r *messageRenderer) render(done bool) {
 	}
 
 	if done {
-		r.println("")
+		r.println(display, "")
 	}
 }
 

pkg/backend/display/progress.go

@@ -22,7 +22,6 @@ import (
 	"runtime"
 	"sort"
 	"strings"
-	"sync"
 	"time"
 	"unicode"
 
@@ -66,19 +65,15 @@ type DiagInfo struct {
 type progressRenderer interface {
 	io.Closer
 
-	initializeDisplay(display *ProgressDisplay)
-	tick()
-	rowUpdated(row Row)
-	systemMessage(payload engine.StdoutEventPayload)
-	done()
-	println(line string)
+	tick(display *ProgressDisplay)
+	rowUpdated(display *ProgressDisplay, row Row)
+	systemMessage(display *ProgressDisplay, payload engine.StdoutEventPayload)
+	done(display *ProgressDisplay)
+	println(display *ProgressDisplay, line string)
 }
 
 // ProgressDisplay organizes all the information needed for a dynamically updated "progress" view of an update.
 type ProgressDisplay struct {
-	// mutex is used to synchronize access to eventUrnToResourceRow, which is accessed by the treeRenderer
-	m sync.RWMutex
-
 	opts Options
 
 	renderer progressRenderer
@@ -139,6 +134,9 @@ type ProgressDisplay struct {
 	// the list of suffixes to rotate through
 	suffixesArray []string
 
+	// Maps used so we can generate short IDs for resource urns.
+	urnToID map[resource.URN]string
+
 	// Structure that tracks the time taken to perform an action on a resource.
 	opStopwatch opStopwatch
 
@@ -237,10 +235,10 @@ func ShowProgressEvents(op string, action apitype.UpdateKind, stack tokens.Stack
 		eventUrnToResourceRow: make(map[resource.URN]ResourceRow),
 		suffixColumn:          int(statusColumn),
 		suffixesArray:         []string{"", ".", "..", "..."},
+		urnToID:               make(map[resource.URN]string),
 		displayOrderCounter:   1,
 		opStopwatch:           newOpStopwatch(),
 	}
-	renderer.initializeDisplay(display)
 
 	ticker := time.NewTicker(1 * time.Second)
 	if opts.deterministicOutput {
@@ -255,7 +253,7 @@ func ShowProgressEvents(op string, action apitype.UpdateKind, stack tokens.Stack
 }
 
 func (display *ProgressDisplay) println(line string) {
-	display.renderer.println(line)
+	display.renderer.println(display, line)
 }
 
 type treeNode struct {
@@ -312,11 +310,6 @@ func (display *ProgressDisplay) getOrCreateTreeNode(
 }
 
 func (display *ProgressDisplay) generateTreeNodes() []*treeNode {
-	// We take the reader lock here because this is called from the renderer and reads from
-	// the eventUrnToResourceRow map
-	display.m.RLock()
-	defer display.m.RUnlock()
-
 	result := []*treeNode{}
 
 	result = append(result, &treeNode{
@@ -447,10 +440,6 @@ func removeInfoColumnIfUnneeded(rows [][]string) {
 // Specifically, this will update the status messages for any resources, and will also then
 // print out all final diagnostics. and finally will print out the summary.
 func (display *ProgressDisplay) processEndSteps() {
-	// Take the read lock here because we are reading from the eventUrnToResourceRow map
-	display.m.RLock()
-	defer display.m.RUnlock()
-
 	// Figure out the rows that are currently in progress.
 	var inProgressRows []ResourceRow
 	if !display.isTerminal {
@@ -469,15 +458,15 @@ func (display *ProgressDisplay) processEndSteps() {
 	// since the display was marked 'done'.
 	if !display.isTerminal {
 		for _, v := range inProgressRows {
-			display.renderer.rowUpdated(v)
+			display.renderer.rowUpdated(display, v)
 		}
 	}
 
 	// Now refresh everything.  This ensures that we go back and remove things like the diagnostic
 	// messages from a status message (since we're going to print them all) below.  Note, this will
 	// only do something in a terminal.  This is what we want, because if we're not in a terminal we
 	// don't really want to reprint any finished items we've already printed.
-	display.renderer.done()
+	display.renderer.done(display)
 
 	// Render the policies section; this will print all policy packs that ran plus any specific
 	// policies that led to violations or remediations. This comes before diagnostics since policy
@@ -818,14 +807,10 @@ func (display *ProgressDisplay) processTick() {
 	// often timeout a process if output is not seen in a while.
 	display.currentTick++
 
-	display.renderer.tick()
+	display.renderer.tick(display)
 }
 
 func (display *ProgressDisplay) getRowForURN(urn resource.URN, metadata *engine.StepEventMetadata) ResourceRow {
-	// Take the write lock here because this can write the eventUrnToResourceRow map
-	display.m.Lock()
-	defer display.m.Unlock()
-
 	// If there's already a row for this URN, return it.
 	row, has := display.eventUrnToResourceRow[urn]
 	if has {
@@ -1008,26 +993,19 @@ func (display *ProgressDisplay) processNormalEvent(event engine.Event) {
 		contract.Failf("Unhandled event type '%s'", event.Type)
 	}
 
-	display.renderer.rowUpdated(row)
+	display.renderer.rowUpdated(display, row)
 }
 
 func (display *ProgressDisplay) handleSystemEvent(payload engine.StdoutEventPayload) {
-	// We need too take the writer lock here because ensureHeaderAndStackRows expects to be
-	// called under the write lock
-	display.m.Lock()
-	defer display.m.Unlock()
-
 	// Make sure we have a header to display
 	display.ensureHeaderAndStackRows()
 
 	display.systemEventPayloads = append(display.systemEventPayloads, payload)
 
-	display.renderer.systemMessage(payload)
+	display.renderer.systemMessage(display, payload)
 }
 
 func (display *ProgressDisplay) ensureHeaderAndStackRows() {
-	contract.Assertf(!display.m.TryLock(), "ProgressDisplay.ensureHeaderAndStackRows MUST be called "+
-		"under the write lock")
 	if display.headerRow == nil {
 		// about to make our first status message.  make sure we present the header line first.
 		display.headerRow = &headerRowData{display: display}