From 46de891e5938820fabb9dfcd4f1217df6b4d3891 Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 25 Jun 2024 19:31:06 -0600 Subject: [PATCH] refactor: Handle stderr similar to stdout (#1427) Signed-off-by: Terry Howe --- cmd/oras/internal/display/handler_test.go | 6 +++--- cmd/oras/internal/display/metadata/text/push_test.go | 3 ++- cmd/oras/internal/option/common.go | 2 +- cmd/oras/internal/option/remote.go | 4 ++-- cmd/oras/internal/output/print.go | 10 +++++----- cmd/oras/internal/output/print_test.go | 7 ++++--- cmd/oras/root/blob/push.go | 2 +- cmd/oras/root/blob/push_test.go | 2 +- cmd/oras/root/cp_test.go | 6 +++--- cmd/oras/root/discover.go | 2 +- cmd/oras/root/manifest/push.go | 2 +- cmd/oras/root/version.go | 2 +- 12 files changed, 25 insertions(+), 23 deletions(-) diff --git a/cmd/oras/internal/display/handler_test.go b/cmd/oras/internal/display/handler_test.go index dc84de56f..9f73e7248 100644 --- a/cmd/oras/internal/display/handler_test.go +++ b/cmd/oras/internal/display/handler_test.go @@ -24,7 +24,7 @@ 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) @@ -32,7 +32,7 @@ func TestNewPushHandler(t *testing.T) { } 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) @@ -40,7 +40,7 @@ func TestNewAttachHandler(t *testing.T) { } 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 index 95372c26f..1ffbb9c37 100644 --- 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 index 73063717f..3fa6cefde 100644 --- 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 index b9f761d8d..0ccd75ef0 100644 --- 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 index 683be830f..f257f3401 100644 --- 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 index 81f8a424b..f63e9607d 100644 --- 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 index 71b3a2864..73cd30ea2 100644 --- 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 index da11a986d..42edccdc5 100644 --- 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 index 099539913..16bb0010d 100644 --- 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 index 381b3db4c..505334bd0 100644 --- 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 index 194d04d6d..13dcf0a6c 100644 --- 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 index ede9abf96..908f46f56 100644 --- 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) }, }