Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: deprecate --verbose and print all status output by default #1545

Closed
wants to merge 19 commits into from
Closed
Prev Previous commit
Next Next commit
refactor print
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
  • Loading branch information
Wwwsylvia committed Nov 27, 2024
commit 3904150b4368fe344cff66671a4997a3fa100558
9 changes: 5 additions & 4 deletions cmd/oras/internal/display/handler_test.go
Original file line number Diff line number Diff line change
@@ -16,17 +16,18 @@ limitations under the License.
package display

import (
"oras.land/oras/internal/testutils"
"os"
"testing"

"oras.land/oras/internal/testutils"

"oras.land/oras/cmd/oras/internal/option"
"oras.land/oras/cmd/oras/internal/output"
)

func TestNewPushHandler(t *testing.T) {
mockFetcher := testutils.NewMockFetcher()
printer := output.NewPrinter(os.Stdout, os.Stderr, false)
printer := output.NewPrinter(os.Stdout, os.Stderr)
_, _, err := NewPushHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout, mockFetcher.Fetcher)
if err != nil {
t.Errorf("NewPushHandler() error = %v, want nil", err)
@@ -35,15 +36,15 @@ func TestNewPushHandler(t *testing.T) {

func TestNewAttachHandler(t *testing.T) {
mockFetcher := testutils.NewMockFetcher()
printer := output.NewPrinter(os.Stdout, os.Stderr, false)
printer := output.NewPrinter(os.Stdout, os.Stderr)
_, _, err := NewAttachHandler(printer, option.Format{Type: option.FormatTypeText.Name}, os.Stdout, mockFetcher.Fetcher)
if err != nil {
t.Errorf("NewAttachHandler() error = %v, want nil", err)
}
}

func TestNewPullHandler(t *testing.T) {
printer := output.NewPrinter(os.Stdout, os.Stderr, false)
printer := output.NewPrinter(os.Stdout, os.Stderr)
_, _, err := NewPullHandler(printer, option.Format{Type: option.FormatTypeText.Name}, "", os.Stdout)
if err != nil {
t.Errorf("NewPullHandler() error = %v, want nil", err)
2 changes: 1 addition & 1 deletion cmd/oras/internal/display/metadata/text/push_test.go
Original file line number Diff line number Diff line change
@@ -65,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, os.Stderr, false)
printer := output.NewPrinter(tt.out, os.Stderr)
p := &PushHandler{
printer: printer,
}
2 changes: 1 addition & 1 deletion cmd/oras/internal/display/status/text.go
Original file line number Diff line number Diff line change
@@ -49,7 +49,7 @@ func NewTextPushHandler(printer *output.Printer, fetcher content.Fetcher) PushHa

// OnFileLoading is called when a file is being prepared for upload.
func (ph *TextPushHandler) OnFileLoading(name string) error {
return ph.printer.PrintUntitled("Preparing", name)
return ph.printer.Println("Preparing", name)
}

// OnEmptyArtifact is called when an empty artifact is being uploaded.
10 changes: 5 additions & 5 deletions cmd/oras/internal/display/status/text_test.go
Original file line number Diff line number Diff line change
@@ -39,7 +39,7 @@ func TestMain(m *testing.M) {
mockFetcher = testutils.NewMockFetcher()
ctx = context.Background()
builder = &strings.Builder{}
printer = output.NewPrinter(builder, os.Stderr, false)
printer = output.NewPrinter(builder, os.Stderr)
bogus = ocispec.Descriptor{MediaType: ocispec.MediaTypeImageManifest}
os.Exit(m.Run())
}
@@ -204,14 +204,14 @@ func TestTextManifestIndexUpdateHandler_OnManifestAdded(t *testing.T) {
}{
{
name: "ref is a digest",
printer: output.NewPrinter(os.Stdout, os.Stderr, false),
printer: output.NewPrinter(os.Stdout, os.Stderr),
ref: "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb",
desc: ocispec.Descriptor{MediaType: "test", Digest: "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb", Size: 25},
wantErr: false,
},
{
name: "ref is not a digest",
printer: output.NewPrinter(os.Stdout, os.Stderr, false),
printer: output.NewPrinter(os.Stdout, os.Stderr),
ref: "v1",
desc: ocispec.Descriptor{MediaType: "test", Digest: "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb", Size: 25},
wantErr: false,
@@ -239,14 +239,14 @@ func TestTextManifestIndexUpdateHandler_OnIndexMerged(t *testing.T) {
}{
{
name: "ref is a digest",
printer: output.NewPrinter(os.Stdout, os.Stderr, false),
printer: output.NewPrinter(os.Stdout, os.Stderr),
ref: "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb",
desc: ocispec.Descriptor{MediaType: "test", Digest: "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb", Size: 25},
wantErr: false,
},
{
name: "ref is not a digest",
printer: output.NewPrinter(os.Stdout, os.Stderr, false),
printer: output.NewPrinter(os.Stdout, os.Stderr),
ref: "v1",
desc: ocispec.Descriptor{MediaType: "test", Digest: "sha256:fd6ed2f36b5465244d5dc86cb4e7df0ab8a9d24adc57825099f522fe009a22bb", Size: 25},
wantErr: false,
7 changes: 2 additions & 5 deletions cmd/oras/internal/option/common.go
Original file line number Diff line number Diff line change
@@ -35,10 +35,7 @@ type Common struct {
// no longer takes effect.
Verbose bool

// SuppressUntitled suppresses the status output for untitled blobs.
// Default value: false.
SuppressUntitled bool
TTY *os.File
TTY *os.File
*output.Printer
}

@@ -53,7 +50,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(), cmd.OutOrStderr(), opts.SuppressUntitled)
opts.Printer = output.NewPrinter(cmd.OutOrStdout(), cmd.OutOrStderr())
// use STDERR as TTY output since STDOUT is reserved for pipeable output
return opts.parseTTY(os.Stderr)
}
31 changes: 13 additions & 18 deletions cmd/oras/internal/output/print.go
Original file line number Diff line number Diff line change
@@ -27,15 +27,14 @@ import (

// Printer prints for status handlers.
type Printer struct {
out io.Writer
err io.Writer
lock sync.Mutex
suppressUntitled bool
out io.Writer
err io.Writer
lock sync.Mutex
}

// NewPrinter creates a new Printer.
func NewPrinter(out io.Writer, err io.Writer, suppressUntitled bool) *Printer {
return &Printer{out: out, err: err, suppressUntitled: suppressUntitled}
func NewPrinter(out io.Writer, err io.Writer) *Printer {
return &Printer{out: out, err: err}
}

// Write implements the io.Writer interface.
@@ -71,19 +70,15 @@ func (p *Printer) Printf(format string, a ...any) error {
return nil
}

// PrintUntitled prints unless suppressed.
func (p *Printer) PrintUntitled(a ...any) error {
if p.suppressUntitled {
return nil
}
return p.Println(a...)
}

// PrintStatus prints transfer status.
func (p *Printer) PrintStatus(desc ocispec.Descriptor, status string) error {
name, isTitle := descriptor.GetTitleOrMediaType(desc)
if !isTitle {
return p.PrintUntitled(status, descriptor.ShortDigest(desc), name)
return p.Println(status, descriptor.ShortDigest(desc), descriptor.GetName(desc))
}

// PrintStatusUnlessSuppressed prints transfer status unless suppressed.
func (p *Printer) PrintStatusUnlessSuppressed(desc ocispec.Descriptor, status string, suppressed bool) error {
if suppressed {
return nil
}
return p.Println(status, descriptor.ShortDigest(desc), name)
return p.PrintStatus(desc, status)
}
46 changes: 1 addition & 45 deletions cmd/oras/internal/output/print_test.go
Original file line number Diff line number Diff line change
@@ -43,7 +43,7 @@ func (mw *mockWriter) String() string {

func TestPrinter_Println(t *testing.T) {
mockWriter := &mockWriter{}
printer := NewPrinter(mockWriter, os.Stderr, false)
printer := NewPrinter(mockWriter, os.Stderr)
err := printer.Println("boom")
if mockWriter.errorCount != 1 {
t.Error("Expected one error actual <" + strconv.Itoa(mockWriter.errorCount) + ">")
@@ -59,47 +59,3 @@ func TestPrinter_Println(t *testing.T) {
t.Error("Expected error to be ignored")
}
}

func TestPrinter_SuppressUntitled(t *testing.T) {
builder := &strings.Builder{}
suppressUntitled := true
printer := NewPrinter(builder, os.Stderr, suppressUntitled)

expected := "normal\nthing one\n"
err := printer.Println("normal")
if err != nil {
t.Error("Expected no error got <" + err.Error() + ">")
}
err = printer.Printf("thing %s\n", "one")
if err != nil {
t.Error("Expected no error got <" + err.Error() + ">")
}
err = printer.PrintUntitled("verbose")
if err != nil {
t.Error("Expected no error got <" + err.Error() + ">")
}
actual := builder.String()
if expected != actual {
t.Error("Expected <" + expected + "> not equal to actual <" + actual + ">")
}
}

func TestPrinter_PrintUntitled(t *testing.T) {
builder := &strings.Builder{}
suppressUntitled := false
printer := NewPrinter(builder, os.Stderr, suppressUntitled)

expected := "normal\nverbose\n"
err := printer.Println("normal")
if err != nil {
t.Error("Expected no error got <" + err.Error() + ">")
}
err = printer.PrintUntitled("verbose")
if err != nil {
t.Error("Expected no error got <" + err.Error() + ">")
}
actual := builder.String()
if expected != actual {
t.Error("Expected <" + expected + "> not equal to actual <" + actual + ">")
}
}
11 changes: 5 additions & 6 deletions cmd/oras/root/blob/push.go
Original file line number Diff line number Diff line change
@@ -87,7 +87,6 @@ Example - Push blob 'hi.txt' into an OCI image layout folder 'layout-dir':
return errors.New("`--size` must be provided if the blob is read from stdin")
}
}
opts.SuppressUntitled = opts.OutputDescriptor
return option.Parse(cmd, &opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
@@ -121,9 +120,9 @@ func pushBlob(cmd *cobra.Command, opts *pushBlobOptions) (err error) {
return err
}
if exists {
err = opts.PrintStatus(desc, "Exists")
err = opts.PrintStatusUnlessSuppressed(desc, "Exists", opts.OutputDescriptor)
} else {
err = opts.doPush(ctx, opts.Printer, target, desc, rc)
err = opts.doPush(ctx, opts.Printer, target, desc, rc, opts.OutputDescriptor)
}
if err != nil {
return err
@@ -142,16 +141,16 @@ func pushBlob(cmd *cobra.Command, opts *pushBlobOptions) (err error) {

return nil
}
func (opts *pushBlobOptions) doPush(ctx context.Context, printer *output.Printer, t oras.Target, desc ocispec.Descriptor, r io.Reader) error {
func (opts *pushBlobOptions) doPush(ctx context.Context, printer *output.Printer, t oras.Target, desc ocispec.Descriptor, r io.Reader, statusSuppressed bool) error {
if opts.TTY == nil {
// none TTY output
if err := printer.PrintStatus(desc, "Uploading"); err != nil {
if err := printer.PrintStatusUnlessSuppressed(desc, "Uploading", statusSuppressed); err != nil {
return err
}
if err := t.Push(ctx, desc, r); err != nil {
return err
}
return printer.PrintStatus(desc, "Uploaded ")
return printer.PrintStatusUnlessSuppressed(desc, "Uploaded ", statusSuppressed)
}

// TTY output
7 changes: 4 additions & 3 deletions cmd/oras/root/blob/push_test.go
Original file line number Diff line number Diff line change
@@ -20,10 +20,11 @@ package blob
import (
"bytes"
"context"
"oras.land/oras/cmd/oras/internal/output"
"os"
"testing"

"oras.land/oras/cmd/oras/internal/output"

"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras-go/v2/content/memory"
@@ -40,7 +41,7 @@ func Test_pushBlobOptions_doPush(t *testing.T) {
src := memory.New()
content := []byte("test")
r := bytes.NewReader(content)
printer := output.NewPrinter(os.Stdout, os.Stderr, false)
printer := output.NewPrinter(os.Stdout, os.Stderr)
desc := ocispec.Descriptor{
MediaType: "application/octet-stream",
Digest: digest.FromBytes(content),
@@ -49,7 +50,7 @@ func Test_pushBlobOptions_doPush(t *testing.T) {
var opts pushBlobOptions
opts.Common.TTY = device
// test
err = opts.doPush(context.Background(), printer, src, desc, r)
err = opts.doPush(context.Background(), printer, src, desc, r, false)
if err != nil {
t.Fatal(err)
}
6 changes: 3 additions & 3 deletions cmd/oras/root/cp_test.go
Original file line number Diff line number Diff line change
@@ -133,7 +133,7 @@ func Test_doCopy(t *testing.T) {
opts.From.Reference = memDesc.Digest.String()
dst := memory.New()
builder := &strings.Builder{}
printer := output.NewPrinter(builder, os.Stderr, opts.SuppressUntitled)
printer := output.NewPrinter(builder, os.Stderr)
handler := status.NewTextCopyHandler(printer, dst)
// test
_, err = doCopy(context.Background(), handler, memStore, dst, &opts)
@@ -157,7 +157,7 @@ func Test_doCopy_skipped(t *testing.T) {
opts.TTY = slave
opts.From.Reference = memDesc.Digest.String()
builder := &strings.Builder{}
printer := output.NewPrinter(builder, os.Stderr, opts.SuppressUntitled)
printer := output.NewPrinter(builder, os.Stderr)
handler := status.NewTextCopyHandler(printer, memStore)

// test
@@ -193,7 +193,7 @@ func Test_doCopy_mounted(t *testing.T) {
}
to.PlainHTTP = true
builder := &strings.Builder{}
printer := output.NewPrinter(builder, os.Stderr, opts.SuppressUntitled)
printer := output.NewPrinter(builder, os.Stderr)
handler := status.NewTextCopyHandler(printer, to)

// test
7 changes: 3 additions & 4 deletions cmd/oras/root/manifest/push.go
Original file line number Diff line number Diff line change
@@ -96,7 +96,6 @@ Example - Push a manifest to an OCI image layout folder 'layout-dir' and tag wit
refs := strings.Split(args[0], ",")
opts.RawReference = refs[0]
opts.extraRefs = refs[1:]
opts.SuppressUntitled = opts.OutputDescriptor
return option.Parse(cmd, &opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
@@ -157,17 +156,17 @@ func pushManifest(cmd *cobra.Command, opts pushOptions) error {
return err
}
if match {
if err := opts.PrintStatus(desc, "Exists"); err != nil {
if err := opts.PrintStatusUnlessSuppressed(desc, "Exists", opts.OutputDescriptor); err != nil {
return err
}
} else {
if err = opts.PrintStatus(desc, "Uploading"); err != nil {
if err = opts.PrintStatusUnlessSuppressed(desc, "Uploading", opts.OutputDescriptor); err != nil {
return err
}
if _, err := oras.TagBytes(ctx, target, mediaType, contentBytes, ref); err != nil {
return err
}
if err = opts.PrintStatus(desc, "Uploaded "); err != nil {
if err = opts.PrintStatusUnlessSuppressed(desc, "Uploaded ", opts.OutputDescriptor); err != nil {
return err
}
}
2 changes: 1 addition & 1 deletion cmd/oras/root/version.go
Original file line number Diff line number Diff line change
@@ -46,7 +46,7 @@ Example - print version:
return nil
},
RunE: func(cmd *cobra.Command, args []string) error {
printer := output.NewPrinter(cmd.OutOrStdout(), cmd.ErrOrStderr(), false)
printer := output.NewPrinter(cmd.OutOrStdout(), cmd.ErrOrStderr())
return runVersion(printer)
},
}
10 changes: 5 additions & 5 deletions internal/descriptor/descriptor.go
Original file line number Diff line number Diff line change
@@ -66,13 +66,13 @@ func Plain(desc ocispec.Descriptor) ocispec.Descriptor {
}
}

// GetTitleOrMediaType gets a descriptor name using either title or media type.
func GetTitleOrMediaType(desc ocispec.Descriptor) (name string, isTitle bool) {
name, ok := desc.Annotations[ocispec.AnnotationTitle]
// GetName gets a descriptor name using either title or media type.
func GetName(desc ocispec.Descriptor) string {
title, ok := desc.Annotations[ocispec.AnnotationTitle]
if !ok {
return desc.MediaType, false
return desc.MediaType
}
return name, true
return title
}

// GenerateContentKey generates a unique key for each content descriptor using
Loading