feat: allow for stdout to be buffered on each command (#2335)

* feat: add preRun func to version to restore stdout

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

* test: add test to capture version in output

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>

* change stdout buffering to log to be opt-in per command

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

* fix tests

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>

---------

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Co-authored-by: Alex Goodman <wagoodman@users.noreply.github.com>

cmd/syft/cli/cli.go

@@ -23,8 +23,8 @@ import (
 // It also constructs the syft attest command and the syft version command.
 // `RunE` is the earliest that the complete application configuration can be loaded.
 func Application(id clio.Identification) clio.Application {
-	app, rootCmd := create(id, os.Stdout)
+	app, _ := create(id, os.Stdout)
-	return ui.StdoutLoggingApplication(app, rootCmd)
+	return app
 }
 
 // Command returns the root command for the syft CLI application. This is useful for embedding the entire syft CLI

cmd/syft/cli/commands/attest.go

@@ -13,6 +13,7 @@ import (
 	"github.com/anchore/clio"
 	"github.com/anchore/stereoscope/pkg/image"
 	"github.com/anchore/syft/cmd/syft/cli/options"
+	"github.com/anchore/syft/cmd/syft/internal/ui"
 	"github.com/anchore/syft/internal"
 	"github.com/anchore/syft/internal/bus"
 	"github.com/anchore/syft/internal/file"
@@ -78,6 +79,9 @@ func Attest(app clio.Application) *cobra.Command {
 		Args:    validatePackagesArgs,
 		PreRunE: applicationUpdateCheck(id, &opts.UpdateCheck),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			restoreStdout := ui.CaptureStdoutToTraceLog()
+			defer restoreStdout()
+
 			return runAttest(id, opts, args[0])
 		},
 	}, opts)

cmd/syft/cli/commands/convert.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/anchore/clio"
 	"github.com/anchore/syft/cmd/syft/cli/options"
+	"github.com/anchore/syft/cmd/syft/internal/ui"
 	"github.com/anchore/syft/internal"
 	"github.com/anchore/syft/internal/log"
 	"github.com/anchore/syft/syft/format"
@@ -47,6 +48,9 @@ func Convert(app clio.Application) *cobra.Command {
 		Args:    validateConvertArgs,
 		PreRunE: applicationUpdateCheck(id, &opts.UpdateCheck),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			restoreStdout := ui.CaptureStdoutToTraceLog()
+			defer restoreStdout()
+
 			return RunConvert(opts, args[0])
 		},
 	}, opts)

cmd/syft/cli/commands/packages.go

@@ -10,6 +10,7 @@ import (
 	"github.com/anchore/stereoscope/pkg/image"
 	"github.com/anchore/syft/cmd/syft/cli/eventloop"
 	"github.com/anchore/syft/cmd/syft/cli/options"
+	"github.com/anchore/syft/cmd/syft/internal/ui"
 	"github.com/anchore/syft/internal"
 	"github.com/anchore/syft/internal/file"
 	"github.com/anchore/syft/internal/log"
@@ -84,6 +85,9 @@ func Packages(app clio.Application) *cobra.Command {
 		Args:    validatePackagesArgs,
 		PreRunE: applicationUpdateCheck(id, &opts.UpdateCheck),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			restoreStdout := ui.CaptureStdoutToTraceLog()
+			defer restoreStdout()
+
 			return runPackages(id, opts, args[0])
 		},
 	}, opts)

cmd/syft/cli/commands/poweruser.go

@@ -12,6 +12,7 @@ import (
 	"github.com/anchore/stereoscope/pkg/image"
 	"github.com/anchore/syft/cmd/syft/cli/eventloop"
 	"github.com/anchore/syft/cmd/syft/cli/options"
+	"github.com/anchore/syft/cmd/syft/internal/ui"
 	"github.com/anchore/syft/internal"
 	"github.com/anchore/syft/syft/artifact"
 	"github.com/anchore/syft/syft/format/syftjson"
@@ -58,6 +59,9 @@ func PowerUser(app clio.Application) *cobra.Command {
 		Hidden:  true,
 		PreRunE: applicationUpdateCheck(id, &opts.UpdateCheck),
 		RunE: func(cmd *cobra.Command, args []string) error {
+			restoreStdout := ui.CaptureStdoutToTraceLog()
+			defer restoreStdout()
+
 			return runPowerUser(id, opts, args[0])
 		},
 	}, opts)

cmd/syft/internal/ui/capture.go

@@ -8,13 +8,19 @@ import (
 	"github.com/anchore/syft/internal/log"
 )
 
-// capture replaces the provided *os.File and redirects output to the provided writer. The return value is a function,
+const defaultStdoutLogBufferSize = 1024
-// which is used to stop the current capturing of output and restore the original file.
+
+// CaptureStdoutToTraceLog replaces stdout and redirects output to the log as trace lines. The return value is a
+// function, which is used to stop the current capturing of output and restore the original file.
 // Example:
 //
-//	restore := capture(&os.Stderr, writer)
+//	restore := CaptureStdoutToTraceLog()
-//	// here, stderr will be captured and redirected to the provided writer
+//	// here, stdout will be captured and redirected to the provided writer
-//	restore() // block until the output has all been sent to the writer and restore the original stderr
+//	restore() // block until the output has all been sent to the writer and restore the original stdout
+func CaptureStdoutToTraceLog() (close func()) {
+	return capture(&os.Stdout, newLogWriter(), defaultStdoutLogBufferSize)
+}
+
 func capture(target **os.File, writer io.Writer, bufSize int) (close func()) {
 	original := *target
 

cmd/syft/internal/ui/log_writer.go

@@ -25,7 +25,7 @@ func (l *logWriter) Write(data []byte) (n int, err error) {
 	s, err := l.r.ReadString('\n')
 	s = strings.TrimRight(s, "\n")
 	for s != "" {
-		log.Trace(s)
+		log.Trace("[unexpected stdout] " + s)
 		n += len(s)
 		if err != nil {
 			break

cmd/syft/internal/ui/log_writer_test.go

@@ -23,14 +23,14 @@ func Test_logWriter(t *testing.T) {
 
 	_, _ = w.Write([]byte("a\nvalue"))
 
-	expected := []any{"a", "value"}
+	expected := []any{"[unexpected stdout] a", "[unexpected stdout] value"}
 	require.Equal(t, expected, bl.values)
 
 	bl.values = nil
 	_, _ = w.Write([]byte("some"))
 	_, _ = w.Write([]byte("thing"))
 
-	expected = []any{"some", "thing"}
+	expected = []any{"[unexpected stdout] some", "[unexpected stdout] thing"}
 	require.Equal(t, expected, bl.values)
 }
 

cmd/syft/internal/ui/stdout_logging_application.go

@@ -1,63 +0,0 @@
-package ui
-
-import (
-	"os"
-
-	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
-
-	"github.com/anchore/clio"
-)
-
-const defaultStdoutLogBufferSize = 1024
-
-// StdoutLoggingApplication wraps the provided app in a clio.Application, which captures non-report data
-// written to os.Stdout and instead logs it to the internal logger. It also modifies the rootCmd help function
-// to restore os.Stdout in order for Cobra to properly print help to stdout
-func StdoutLoggingApplication(app clio.Application, rootCmd *cobra.Command) clio.Application {
-	return &stdoutLoggingApplication{
-		delegate: app,
-		rootCmd:  rootCmd,
-	}
-}
-
-// stdoutLoggingApplication is a clio.Application, which captures data written to os.Stdout prior to the report
-// being output and instead sends non-report output to the internal logger
-type stdoutLoggingApplication struct {
-	delegate clio.Application
-	rootCmd  *cobra.Command
-}
-
-func (s *stdoutLoggingApplication) ID() clio.Identification {
-	return s.delegate.ID()
-}
-
-func (s *stdoutLoggingApplication) AddFlags(flags *pflag.FlagSet, cfgs ...any) {
-	s.delegate.AddFlags(flags, cfgs...)
-}
-
-func (s *stdoutLoggingApplication) SetupCommand(cmd *cobra.Command, cfgs ...any) *cobra.Command {
-	return s.delegate.SetupCommand(cmd, cfgs...)
-}
-
-func (s *stdoutLoggingApplication) SetupRootCommand(cmd *cobra.Command, cfgs ...any) *cobra.Command {
-	return s.delegate.SetupRootCommand(cmd, cfgs...)
-}
-
-func (s *stdoutLoggingApplication) Run() {
-	// capture everything written to stdout that is not report output
-	restoreStdout := capture(&os.Stdout, newLogWriter(), defaultStdoutLogBufferSize)
-	defer restoreStdout()
-
-	// need to restore stdout for cobra to properly output help text to the user on stdout
-	baseHelpFunc := s.rootCmd.HelpFunc()
-	defer s.rootCmd.SetHelpFunc(baseHelpFunc)
-	s.rootCmd.SetHelpFunc(func(command *cobra.Command, strings []string) {
-		restoreStdout()
-		baseHelpFunc(command, strings)
-	})
-
-	s.delegate.Run()
-}
-
-var _ clio.Application = (*stdoutLoggingApplication)(nil)

go.mod

@@ -76,8 +76,6 @@ require (
 	modernc.org/sqlite v1.27.0
 )
 
-require github.com/spf13/pflag v1.0.5
-
 require (
 	dario.cat/mergo v1.0.0 // indirect
 	github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 // indirect
@@ -185,6 +183,7 @@ require (
 	github.com/skeema/knownhosts v1.2.0 // indirect
 	github.com/spf13/cast v1.5.1 // indirect
 	github.com/spf13/jwalterweatherman v1.1.0 // indirect
+	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/spf13/viper v1.16.0 // indirect
 	github.com/subosito/gotenv v1.4.2 // indirect
 	github.com/sylabs/sif/v2 v2.11.5 // indirect

test/cli/version_cmd_test.go

@@ -0,0 +1,29 @@
+package cli
+
+import (
+	"testing"
+)
+
+func TestVersionCmdPrintsToStdout(t *testing.T) {
+	tests := []struct {
+		name       string
+		env        map[string]string
+		assertions []traitAssertion
+	}{
+		{
+			name: "version command prints to stdout",
+			assertions: []traitAssertion{
+				assertInOutput("Version:"),
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			pkgCmd, pkgsStdout, pkgsStderr := runSyft(t, test.env, "version")
+			for _, traitFn := range test.assertions {
+				traitFn(t, pkgsStdout, pkgsStderr, pkgCmd.ProcessState.ExitCode())
+			}
+		})
+	}
+}