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
2 changes: 1 addition & 1 deletion cmd/oras/internal/command/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (

// GetLogger returns a new FieldLogger and an associated Context derived from command context.
func GetLogger(cmd *cobra.Command, opts *option.Common) (context.Context, logrus.FieldLogger) {
ctx, logger := trace.NewLogger(cmd.Context(), opts.Debug, opts.Verbose)
ctx, logger := trace.NewLogger(cmd.Context(), opts.Debug)
cmd.SetContext(ctx)
return ctx, logger
}
9 changes: 5 additions & 4 deletions cmd/oras/internal/display/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
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
Expand Up @@ -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,
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/internal/display/status/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.PrintVerbose("Preparing", name)
return ph.printer.Println("Preparing", name)
}

// OnEmptyArtifact is called when an empty artifact is being uploaded.
Expand Down
12 changes: 6 additions & 6 deletions cmd/oras/internal/display/status/text_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -173,7 +173,7 @@ func TestTextPushHandler_OnFileLoading(t *testing.T) {
if ph.OnFileLoading("name") != nil {
t.Error("OnFileLoading() should not return an error")
}
validatePrinted(t, "")
validatePrinted(t, "Preparing name")
}

func TestTextPushHandler_PostCopy(t *testing.T) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 5 additions & 6 deletions cmd/oras/internal/option/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,23 +28,22 @@ const NoTTYFlag = "no-tty"

// Common option struct.
type Common struct {
Debug bool
Verbose bool
TTY *os.File
*output.Printer
Debug bool
noTTY bool

TTY *os.File
*output.Printer
}

// ApplyFlags applies flags to a command flag set.
func (opts *Common) ApplyFlags(fs *pflag.FlagSet) {
fs.BoolVarP(&opts.Debug, "debug", "d", false, "output debug logs (implies --no-tty)")
fs.BoolVarP(&opts.Verbose, "verbose", "v", false, "verbose output")
fs.BoolVarP(&opts.noTTY, NoTTYFlag, "", false, "[Preview] do not show progress output")
}

// Parse gets target options from user input.
func (opts *Common) Parse(cmd *cobra.Command) error {
opts.Printer = output.NewPrinter(cmd.OutOrStdout(), cmd.OutOrStderr(), opts.Verbose)
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)
}
Expand Down
31 changes: 13 additions & 18 deletions cmd/oras/internal/output/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ import (

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

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

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

// PrintVerbose prints when verbose is true.
func (p *Printer) PrintVerbose(a ...any) error {
if !p.verbose {
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.PrintVerbose(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)
}
44 changes: 1 addition & 43 deletions cmd/oras/internal/output/print_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) + ">")
Expand All @@ -59,45 +59,3 @@ func TestPrinter_Println(t *testing.T) {
t.Error("Expected error to be ignored")
}
}

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

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.PrintVerbose("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_PrintVerbose(t *testing.T) {
builder := &strings.Builder{}
printer := NewPrinter(builder, os.Stderr, true)

expected := "normal\nverbose\n"
err := printer.Println("normal")
if err != nil {
t.Error("Expected no error got <" + err.Error() + ">")
}
err = printer.PrintVerbose("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 + ">")
}
}
8 changes: 7 additions & 1 deletion cmd/oras/root/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ type attachOptions struct {
option.Format
option.Platform

// Verbose is deprecated. The default behavior is now equivalent to
// Verbose=true, while Verbose=false no longer takes effect.
Verbose bool
artifactType string
concurrency int
}
Expand Down Expand Up @@ -113,9 +116,12 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder
},
}

opts.FlagDescription = "[Preview] attach to an arch-specific subject"
cmd.Flags().StringVarP(&opts.artifactType, "artifact-type", "", "", "artifact type")
cmd.Flags().IntVarP(&opts.concurrency, "concurrency", "", 5, "concurrency level")
opts.FlagDescription = "[Preview] attach to an arch-specific subject"
cmd.Flags().BoolVarP(&opts.Verbose, "verbose", "v", false, "[Deprecated] verbose output")

_ = cmd.Flags().MarkDeprecated("verbose", "and may be removed in a future release.")
_ = cmd.MarkFlagRequired("artifact-type")
opts.EnableDistributionSpecFlag()
opts.SetTypes(option.FormatTypeText, option.FormatTypeJSON, option.FormatTypeGoTemplate)
Expand Down
17 changes: 11 additions & 6 deletions cmd/oras/root/blob/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ type pushBlobOptions struct {
option.Pretty
option.Target

// Verbose is deprecated. The default behavior is now equivalent to
// Verbose=true, while Verbose=false no longer takes effect.
Verbose bool
fileRef string
mediaType string
size int64
Expand Down Expand Up @@ -87,7 +90,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.Verbose = opts.Verbose && !opts.OutputDescriptor
return option.Parse(cmd, &opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
Expand All @@ -97,6 +99,9 @@ Example - Push blob 'hi.txt' into an OCI image layout folder 'layout-dir':

cmd.Flags().Int64VarP(&opts.size, "size", "", -1, "provide the blob size")
cmd.Flags().StringVarP(&opts.mediaType, "media-type", "", ocispec.MediaTypeImageLayer, "specify the returned media type in the descriptor if --descriptor is used")
cmd.Flags().BoolVarP(&opts.Verbose, "verbose", "v", false, "[Deprecated] verbose output")
_ = cmd.Flags().MarkDeprecated("verbose", "and may be removed in a future release.")

option.ApplyFlags(&opts, cmd.Flags())
return oerrors.Command(cmd, &opts.Target)
}
Expand All @@ -121,9 +126,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
Expand All @@ -142,16 +147,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
Expand Down
7 changes: 4 additions & 3 deletions cmd/oras/root/blob/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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),
Expand All @@ -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)
}
Expand Down
6 changes: 6 additions & 0 deletions cmd/oras/root/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ type copyOptions struct {
option.Platform
option.BinaryTarget

// Verbose is deprecated. The default behavior is now equivalent to
// Verbose=true, while Verbose=false no longer takes effect.
Verbose bool
recursive bool
concurrency int
extraRefs []string
Expand Down Expand Up @@ -105,6 +108,9 @@ Example - Copy an artifact with multiple tags with concurrency tuned:
}
cmd.Flags().BoolVarP(&opts.recursive, "recursive", "r", false, "[Preview] recursively copy the artifact and its referrer artifacts")
cmd.Flags().IntVarP(&opts.concurrency, "concurrency", "", 3, "concurrency level")
cmd.Flags().BoolVarP(&opts.Verbose, "verbose", "v", false, "[Deprecated] verbose output")
_ = cmd.Flags().MarkDeprecated("verbose", "and may be removed in a future release.")

opts.EnableDistributionSpecFlag()
option.ApplyFlags(&opts, cmd.Flags())
return oerrors.Command(cmd, &opts.BinaryTarget)
Expand Down
Loading
Loading