Skip to content

Commit

Permalink
refactor: Handle stderr similar to stdout (#1427)
Browse files Browse the repository at this point in the history
Signed-off-by: Terry Howe <[email protected]>
  • Loading branch information
Terry Howe authored Jun 26, 2024
1 parent f8519a9 commit 46de891
Show file tree
Hide file tree
Showing 12 changed files with 25 additions and 23 deletions.
6 changes: 3 additions & 3 deletions cmd/oras/internal/display/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion cmd/oras/internal/display/metadata/text/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"bytes"
"fmt"
"io"
"os"
"testing"

"github.com/opencontainers/go-digest"
Expand Down Expand Up @@ -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,
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/internal/option/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/internal/option/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 5 additions & 5 deletions cmd/oras/internal/output/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"context"
"fmt"
"io"
"os"
"sync"

"oras.land/oras/internal/descriptor"
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down
7 changes: 4 additions & 3 deletions cmd/oras/internal/output/print_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package output

import (
"fmt"
"os"
"strconv"
"strings"
"testing"
Expand All @@ -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) + ">")
Expand All @@ -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")
Expand All @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/root/blob/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/root/blob/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
6 changes: 3 additions & 3 deletions cmd/oras/root/cp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/root/discover.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/root/manifest/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/root/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
},
}
Expand Down

0 comments on commit 46de891

Please sign in to comment.