Skip to content

Commit

Permalink
fix!: downgrade the global verbose flag (#1553)
Browse files Browse the repository at this point in the history
Signed-off-by: Lixia (Sylvia) Lei <[email protected]>
  • Loading branch information
Wwwsylvia authored Dec 2, 2024
1 parent 0483d7b commit 325572c
Show file tree
Hide file tree
Showing 26 changed files with 88 additions and 68 deletions.
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
10 changes: 5 additions & 5 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 @@ -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
9 changes: 4 additions & 5 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
Printer *output.Printer
TTY *os.File
*output.Printer
Debug bool

noTTY bool
}

// 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
15 changes: 8 additions & 7 deletions cmd/oras/internal/output/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,16 @@ import (

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

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 @@ -73,7 +74,7 @@ func (p *Printer) Printf(format string, a ...any) error {

// PrintVerbose prints when verbose is true.
func (p *Printer) PrintVerbose(a ...any) error {
if !p.verbose {
if !p.Verbose {
return nil
}
return p.Println(a...)
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 @@ -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 @@ -62,7 +62,7 @@ func TestPrinter_Println(t *testing.T) {

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

expected := "normal\nthing one\n"
err := printer.Println("normal")
Expand All @@ -85,7 +85,8 @@ func TestPrinter_PrintVerbose_noError(t *testing.T) {

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

expected := "normal\nverbose\n"
err := printer.Println("normal")
Expand Down
3 changes: 3 additions & 0 deletions cmd/oras/root/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type attachOptions struct {

artifactType string
concurrency int
verbose bool
}

func attachCmd() *cobra.Command {
Expand Down Expand Up @@ -109,12 +110,14 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder
return err
},
RunE: func(cmd *cobra.Command, args []string) error {
opts.Printer.Verbose = opts.verbose
return runAttach(cmd, &opts)
},
}

cmd.Flags().StringVarP(&opts.artifactType, "artifact-type", "", "", "artifact type")
cmd.Flags().IntVarP(&opts.concurrency, "concurrency", "", 5, "concurrency level")
cmd.Flags().BoolVarP(&opts.verbose, "verbose", "v", false, "print status output for unnamed blobs")
opts.FlagDescription = "[Preview] attach to an arch-specific subject"
_ = cmd.MarkFlagRequired("artifact-type")
opts.EnableDistributionSpecFlag()
Expand Down
4 changes: 2 additions & 2 deletions cmd/oras/root/blob/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func deleteBlob(cmd *cobra.Command, opts *deleteBlobOptions) (err error) {
if errors.Is(err, errdef.ErrNotFound) {
if opts.Force && !opts.OutputDescriptor {
// ignore nonexistent
_ = opts.Println("Missing", opts.RawReference)
_ = opts.Printer.Println("Missing", opts.RawReference)
return nil
}
return fmt.Errorf("%s: the specified blob does not exist", opts.RawReference)
Expand Down Expand Up @@ -118,7 +118,7 @@ func deleteBlob(cmd *cobra.Command, opts *deleteBlobOptions) (err error) {
return opts.Output(os.Stdout, descJSON)
}

_ = opts.Println("Deleted", opts.AnnotatedReference())
_ = opts.Printer.Println("Deleted", opts.AnnotatedReference())

return nil
}
10 changes: 6 additions & 4 deletions cmd/oras/root/blob/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type pushBlobOptions struct {
fileRef string
mediaType string
size int64
verbose bool
}

func pushCmd() *cobra.Command {
Expand Down Expand Up @@ -87,16 +88,17 @@ 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 {
opts.Printer.Verbose = opts.verbose && !opts.OutputDescriptor
return pushBlob(cmd, &opts)
},
}

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, "print status output for unnamed blobs")
option.ApplyFlags(&opts, cmd.Flags())
return oerrors.Command(cmd, &opts.Target)
}
Expand All @@ -121,7 +123,7 @@ func pushBlob(cmd *cobra.Command, opts *pushBlobOptions) (err error) {
return err
}
if exists {
err = opts.PrintStatus(desc, "Exists")
err = opts.Printer.PrintStatus(desc, "Exists")
} else {
err = opts.doPush(ctx, opts.Printer, target, desc, rc)
}
Expand All @@ -137,8 +139,8 @@ func pushBlob(cmd *cobra.Command, opts *pushBlobOptions) (err error) {
return opts.Output(os.Stdout, descJSON)
}

_ = opts.Println("Pushed", opts.AnnotatedReference())
_ = opts.Println("Digest:", desc.Digest)
_ = opts.Printer.Println("Pushed", opts.AnnotatedReference())
_ = opts.Printer.Println("Digest:", desc.Digest)

return nil
}
Expand Down
5 changes: 3 additions & 2 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 Down
7 changes: 5 additions & 2 deletions cmd/oras/root/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ type copyOptions struct {
recursive bool
concurrency int
extraRefs []string
verbose bool
}

func copyCmd() *cobra.Command {
Expand Down Expand Up @@ -100,11 +101,13 @@ Example - Copy an artifact with multiple tags with concurrency tuned:
return option.Parse(cmd, &opts)
},
RunE: func(cmd *cobra.Command, args []string) error {
opts.Printer.Verbose = opts.verbose
return runCopy(cmd, &opts)
},
}
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, "print status output for unnamed blobs")
opts.EnableDistributionSpecFlag()
option.ApplyFlags(&opts, cmd.Flags())
return oerrors.Command(cmd, &opts.BinaryTarget)
Expand Down Expand Up @@ -139,7 +142,7 @@ func runCopy(cmd *cobra.Command, opts *copyOptions) error {
// correct source digest
opts.From.RawReference = fmt.Sprintf("%s@%s", opts.From.Path, desc.Digest.String())
}
_ = opts.Println("Copied", opts.From.AnnotatedReference(), "=>", opts.To.AnnotatedReference())
_ = opts.Printer.Println("Copied", opts.From.AnnotatedReference(), "=>", opts.To.AnnotatedReference())

if len(opts.extraRefs) != 0 {
tagNOpts := oras.DefaultTagNOptions
Expand All @@ -150,7 +153,7 @@ func runCopy(cmd *cobra.Command, opts *copyOptions) error {
}
}

_ = opts.Println("Digest:", desc.Digest)
_ = opts.Printer.Println("Digest:", desc.Digest)

return nil
}
Expand Down
17 changes: 9 additions & 8 deletions cmd/oras/root/cp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"oras.land/oras/cmd/oras/internal/display/status"
"oras.land/oras/cmd/oras/internal/output"
"os"
"strings"
"testing"

"oras.land/oras/cmd/oras/internal/display/status"
"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 Down Expand Up @@ -129,11 +130,11 @@ func Test_doCopy(t *testing.T) {
defer slave.Close()
var opts copyOptions
opts.TTY = slave
opts.Verbose = true
opts.From.Reference = memDesc.Digest.String()
dst := memory.New()
builder := &strings.Builder{}
printer := output.NewPrinter(builder, os.Stderr, opts.Verbose)
printer := output.NewPrinter(builder, os.Stderr)
printer.Verbose = true
handler := status.NewTextCopyHandler(printer, dst)
// test
_, err = doCopy(context.Background(), handler, memStore, dst, &opts)
Expand All @@ -155,10 +156,10 @@ func Test_doCopy_skipped(t *testing.T) {
defer slave.Close()
var opts copyOptions
opts.TTY = slave
opts.Verbose = true
opts.From.Reference = memDesc.Digest.String()
builder := &strings.Builder{}
printer := output.NewPrinter(builder, os.Stderr, opts.Verbose)
printer := output.NewPrinter(builder, os.Stderr)
printer.Verbose = true
handler := status.NewTextCopyHandler(printer, memStore)

// test
Expand All @@ -181,7 +182,6 @@ func Test_doCopy_mounted(t *testing.T) {
defer slave.Close()
var opts copyOptions
opts.TTY = slave
opts.Verbose = true
opts.From.Reference = manifestDigest
// mocked repositories
from, err := remote.NewRepository(fmt.Sprintf("%s/%s", host, repoFrom))
Expand All @@ -195,7 +195,8 @@ func Test_doCopy_mounted(t *testing.T) {
}
to.PlainHTTP = true
builder := &strings.Builder{}
printer := output.NewPrinter(builder, os.Stderr, opts.Verbose)
printer := output.NewPrinter(builder, os.Stderr)
printer.Verbose = true
handler := status.NewTextCopyHandler(printer, to)

// test
Expand Down
Loading

0 comments on commit 325572c

Please sign in to comment.