refactor: Handle stderr similar to stdout (#1427)

Signed-off-by: Terry Howe <[email protected]>
oras-project · Jun 26, 2024 · 46de891 · 46de891
1 parent f8519a9
commit 46de891
Show file tree

Hide file tree

Showing 12 changed files with 25 additions and 23 deletions.
diff --git a/cmd/oras/internal/display/handler_test.go b/cmd/oras/internal/display/handler_test.go
@@ -24,23 +24,23 @@ import (
 )
 
 func TestNewPushHandler(t *testing.T) {
-	printer := output.NewPrinter(os.Stdout, false)
+	printer := output.NewPrinter(os.Stdout, os.Stderr, false)
 	_, _, err := NewPushHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout)
 	if err != nil {
 		t.Errorf("NewPushHandler() error = %v, want nil", err)
 	}
 }
 
 func TestNewAttachHandler(t *testing.T) {
-	printer := output.NewPrinter(os.Stdout, false)
+	printer := output.NewPrinter(os.Stdout, os.Stderr, false)
 	_, _, err := NewAttachHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout)
 	if err != nil {
 		t.Errorf("NewAttachHandler() error = %v, want nil", err)
 	}
 }
 
 func TestNewPullHandler(t *testing.T) {
-	printer := output.NewPrinter(os.Stdout, false)
+	printer := output.NewPrinter(os.Stdout, os.Stderr, false)
 	_, _, err := NewPullHandler(printer, option.Format{Type: option.FormatTypeText.Name}, "", os.Stdout)
 	if err != nil {
 		t.Errorf("NewPullHandler() error = %v, want nil", err)

diff --git a/cmd/oras/internal/display/metadata/text/push_test.go b/cmd/oras/internal/display/metadata/text/push_test.go
@@ -19,6 +19,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"os"
 	"testing"
 
 	"github.com/opencontainers/go-digest"
@@ -64,7 +65,7 @@ func TestPushHandler_OnCompleted(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			printer := output.NewPrinter(tt.out, false)
+			printer := output.NewPrinter(tt.out, os.Stderr, false)
 			p := &PushHandler{
 				printer: printer,
 			}

diff --git a/cmd/oras/internal/option/common.go b/cmd/oras/internal/option/common.go
@@ -44,7 +44,7 @@ func (opts *Common) ApplyFlags(fs *pflag.FlagSet) {
 
 // Parse gets target options from user input.
 func (opts *Common) Parse(cmd *cobra.Command) error {
-	opts.Printer = output.NewPrinter(cmd.OutOrStdout(), opts.Verbose)
+	opts.Printer = output.NewPrinter(cmd.OutOrStdout(), cmd.OutOrStderr(), opts.Verbose)
 	// use STDERR as TTY output since STDOUT is reserved for pipeable output
 	return opts.parseTTY(os.Stderr)
 }

diff --git a/cmd/oras/internal/option/remote.go b/cmd/oras/internal/option/remote.go
@@ -176,9 +176,9 @@ func (opts *Remote) Parse(cmd *cobra.Command) error {
 // optional cmd prompt.
 func (opts *Remote) readSecret(cmd *cobra.Command) (err error) {
 	if cmd.Flags().Changed(identityTokenFlag) {
-		fmt.Fprintln(os.Stderr, "WARNING! Using --identity-token via the CLI is insecure. Use --identity-token-stdin.")
+		fmt.Fprintln(cmd.ErrOrStderr(), "WARNING! Using --identity-token via the CLI is insecure. Use --identity-token-stdin.")
 	} else if cmd.Flags().Changed(passwordFlag) {
-		fmt.Fprintln(os.Stderr, "WARNING! Using --password via the CLI is insecure. Use --password-stdin.")
+		fmt.Fprintln(cmd.ErrOrStderr(), "WARNING! Using --password via the CLI is insecure. Use --password-stdin.")
 	} else if opts.secretFromStdin {
 		// Prompt for credential
 		secret, err := io.ReadAll(os.Stdin)

diff --git a/cmd/oras/internal/output/print.go b/cmd/oras/internal/output/print.go
@@ -19,7 +19,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"os"
 	"sync"
 
 	"oras.land/oras/internal/descriptor"
@@ -34,13 +33,14 @@ type PrintFunc func(ocispec.Descriptor) error
 // Printer prints for status handlers.
 type Printer struct {
 	out     io.Writer
+	err     io.Writer
 	verbose bool
 	lock    sync.Mutex
 }
 
 // NewPrinter creates a new Printer.
-func NewPrinter(out io.Writer, verbose bool) *Printer {
-	return &Printer{out: out, verbose: verbose}
+func NewPrinter(out io.Writer, err io.Writer, verbose bool) *Printer {
+	return &Printer{out: out, err: err, verbose: verbose}
 }
 
 // Write implements the io.Writer interface.
@@ -57,7 +57,7 @@ func (p *Printer) Println(a ...any) error {
 	_, err := fmt.Fprintln(p.out, a...)
 	if err != nil {
 		err = fmt.Errorf("display output error: %w", err)
-		_, _ = fmt.Fprint(os.Stderr, err)
+		_, _ = fmt.Fprint(p.err, err)
 	}
 	// Errors are handled above, so return nil
 	return nil
@@ -70,7 +70,7 @@ func (p *Printer) Printf(format string, a ...any) error {
 	_, err := fmt.Fprintf(p.out, format, a...)
 	if err != nil {
 		err = fmt.Errorf("display output error: %w", err)
-		_, _ = fmt.Fprint(os.Stderr, err)
+		_, _ = fmt.Fprint(p.err, err)
 	}
 	// Errors are handled above, so return nil
 	return nil

diff --git a/cmd/oras/internal/output/print_test.go b/cmd/oras/internal/output/print_test.go
@@ -17,6 +17,7 @@ package output
 
 import (
 	"fmt"
+	"os"
 	"strconv"
 	"strings"
 	"testing"
@@ -42,7 +43,7 @@ func (mw *mockWriter) String() string {
 
 func TestPrinter_Println(t *testing.T) {
 	mockWriter := &mockWriter{}
-	printer := NewPrinter(mockWriter, false)
+	printer := NewPrinter(mockWriter, os.Stderr, false)
 	err := printer.Println("boom")
 	if mockWriter.errorCount != 1 {
 		t.Error("Expected one error actual <" + strconv.Itoa(mockWriter.errorCount) + ">")
@@ -61,7 +62,7 @@ func TestPrinter_Println(t *testing.T) {
 
 func TestPrinter_PrintVerbose_noError(t *testing.T) {
 	builder := &strings.Builder{}
-	printer := NewPrinter(builder, false)
+	printer := NewPrinter(builder, os.Stderr, false)
 
 	expected := "normal\nthing one\n"
 	err := printer.Println("normal")
@@ -84,7 +85,7 @@ func TestPrinter_PrintVerbose_noError(t *testing.T) {
 
 func TestPrinter_PrintVerbose(t *testing.T) {
 	builder := &strings.Builder{}
-	printer := NewPrinter(builder, true)
+	printer := NewPrinter(builder, os.Stderr, true)
 
 	expected := "normal\nverbose\n"
 	err := printer.Println("normal")

diff --git a/cmd/oras/root/blob/push.go b/cmd/oras/root/blob/push.go
@@ -103,7 +103,7 @@ Example - Push blob 'hi.txt' into an OCI image layout folder 'layout-dir':
 func pushBlob(cmd *cobra.Command, opts *pushBlobOptions) (err error) {
 	ctx, logger := command.GetLogger(cmd, &opts.Common)
 	verbose := opts.Verbose && !opts.OutputDescriptor
-	printer := output.NewPrinter(cmd.OutOrStdout(), verbose)
+	printer := output.NewPrinter(cmd.OutOrStdout(), cmd.ErrOrStderr(), verbose)
 
 	target, err := opts.NewTarget(opts.Common, logger)
 	if err != nil {

diff --git a/cmd/oras/root/blob/push_test.go b/cmd/oras/root/blob/push_test.go
@@ -40,7 +40,7 @@ func Test_pushBlobOptions_doPush(t *testing.T) {
 	src := memory.New()
 	content := []byte("test")
 	r := bytes.NewReader(content)
-	printer := output.NewPrinter(os.Stdout, false)
+	printer := output.NewPrinter(os.Stdout, os.Stderr, false)
 	desc := ocispec.Descriptor{
 		MediaType: "application/octet-stream",
 		Digest:    digest.FromBytes(content),

diff --git a/cmd/oras/root/cp_test.go b/cmd/oras/root/cp_test.go
@@ -132,7 +132,7 @@ func Test_doCopy(t *testing.T) {
 	opts.From.Reference = memDesc.Digest.String()
 	dst := memory.New()
 	builder := &strings.Builder{}
-	printer := output.NewPrinter(builder, opts.Verbose)
+	printer := output.NewPrinter(builder, os.Stderr, opts.Verbose)
 	// test
 	_, err = doCopy(context.Background(), printer, memStore, dst, &opts)
 	if err != nil {
@@ -156,7 +156,7 @@ func Test_doCopy_skipped(t *testing.T) {
 	opts.Verbose = true
 	opts.From.Reference = memDesc.Digest.String()
 	builder := &strings.Builder{}
-	printer := output.NewPrinter(builder, opts.Verbose)
+	printer := output.NewPrinter(builder, os.Stderr, opts.Verbose)
 	// test
 	_, err = doCopy(context.Background(), printer, memStore, memStore, &opts)
 	if err != nil {
@@ -191,7 +191,7 @@ func Test_doCopy_mounted(t *testing.T) {
 	}
 	to.PlainHTTP = true
 	builder := &strings.Builder{}
-	printer := output.NewPrinter(builder, opts.Verbose)
+	printer := output.NewPrinter(builder, os.Stderr, opts.Verbose)
 	// test
 	_, err = doCopy(context.Background(), printer, from, to, &opts)
 	if err != nil {

diff --git a/cmd/oras/root/discover.go b/cmd/oras/root/discover.go
@@ -85,7 +85,7 @@ Example - Discover referrers of the manifest tagged 'v1' in an OCI image layout
 			if cmd.Flags().Changed("output") {
 				switch opts.Format.Type {
 				case "tree", "json", "table":
-					fmt.Fprintf(cmd.ErrOrStderr(), "[DEPRECATED] --output is deprecated, try `--format %s` instead\n", opts.Template)
+					_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "[DEPRECATED] --output is deprecated, try `--format %s` instead\n", opts.Template)
 				default:
 					return errors.New("output type can only be tree, table or json")
 				}

diff --git a/cmd/oras/root/manifest/push.go b/cmd/oras/root/manifest/push.go
@@ -113,7 +113,7 @@ Example - Push a manifest to an OCI image layout folder 'layout-dir' and tag wit
 func pushManifest(cmd *cobra.Command, opts pushOptions) error {
 	ctx, logger := command.GetLogger(cmd, &opts.Common)
 	verbose := opts.Verbose && !opts.OutputDescriptor
-	printer := output.NewPrinter(cmd.OutOrStdout(), verbose)
+	printer := output.NewPrinter(cmd.OutOrStdout(), cmd.ErrOrStderr(), verbose)
 	var target oras.Target
 	var err error
 	target, err = opts.NewTarget(opts.Common, logger)

diff --git a/cmd/oras/root/version.go b/cmd/oras/root/version.go
@@ -46,7 +46,7 @@ Example - print version:
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
-			printer := output.NewPrinter(cmd.OutOrStdout(), false)
+			printer := output.NewPrinter(cmd.OutOrStdout(), cmd.ErrOrStderr(), false)
 			return runVersion(printer)
 		},
 	}