fix: discard tagging output when pushing with formatted output (oras-…

…project#1302)

Signed-off-by: Billy Zha <jinzha1@microsoft.com>

cmd/oras/internal/display/metadata/discard.go

@@ -20,7 +20,7 @@ import ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 type discard struct{}
 
 // NewDiscardHandler creates a new handler that discards output for all events.
-func NewDiscardHandler() ManifestFetchHandler {
+func NewDiscardHandler() discard {
 	return discard{}
 }
 

cmd/oras/internal/display/metadata/interface.go

@@ -22,6 +22,8 @@ import (
 
 // PushHandler handles metadata output for push events.
 type PushHandler interface {
+	TagHandler
+
 	OnCopied(opts *option.Target) error
 	OnCompleted(root ocispec.Descriptor) error
 }
@@ -57,3 +59,9 @@ type PullHandler interface {
 	// OnCompleted is called when the pull cmd execution is completed.
 	OnCompleted(opts *option.Target, desc ocispec.Descriptor) error
 }
+
+// TagHandler handles status output for tag command.
+type TagHandler interface {
+	// OnTagged is called when each tagging operation is done.
+	OnTagged(desc ocispec.Descriptor, tag string) error
+}

cmd/oras/internal/display/metadata/json/push.go

@@ -38,6 +38,11 @@ func NewPushHandler(out io.Writer) metadata.PushHandler {
 	}
 }
 
+// OnTagged implements metadata.TagHandler.
+func (ph *PushHandler) OnTagged(desc ocispec.Descriptor, tag string) error {
+	return nil
+}
+
 // OnCopied is called after files are copied.
 func (ph *PushHandler) OnCopied(opts *option.Target) error {
 	ph.path = opts.Path

cmd/oras/internal/display/metadata/template/push.go

@@ -39,6 +39,11 @@ func NewPushHandler(out io.Writer, template string) metadata.PushHandler {
 	}
 }
 
+// OnTagged implements metadata.TagHandler.
+func (ph *PushHandler) OnTagged(desc ocispec.Descriptor, tag string) error {
+	return nil
+}
+
 // OnStarted is called after files are copied.
 func (ph *PushHandler) OnCopied(opts *option.Target) error {
 	ph.path = opts.Path

cmd/oras/internal/display/metadata/text/push.go

@@ -18,6 +18,7 @@ package text
 import (
 	"fmt"
 	"io"
+	"sync"
 
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"oras.land/oras/cmd/oras/internal/display/metadata"
@@ -26,7 +27,8 @@ import (
 
 // PushHandler handles text metadata output for push events.
 type PushHandler struct {
-	out io.Writer
+	out     io.Writer
+	tagLock sync.Mutex
 }
 
 // NewPushHandler returns a new handler for push events.
@@ -36,18 +38,26 @@ func NewPushHandler(out io.Writer) metadata.PushHandler {
 	}
 }
 
+// OnTagged implements metadata.TextTagHandler.
+func (h *PushHandler) OnTagged(_ ocispec.Descriptor, tag string) error {
+	h.tagLock.Lock()
+	defer h.tagLock.Unlock()
+	_, err := fmt.Fprintln(h.out, "Tagged", tag)
+	return err
+}
+
 // OnCopied is called after files are copied.
-func (p *PushHandler) OnCopied(opts *option.Target) error {
-	_, err := fmt.Fprintln(p.out, "Pushed", opts.AnnotatedReference())
+func (h *PushHandler) OnCopied(opts *option.Target) error {
+	_, err := fmt.Fprintln(h.out, "Pushed", opts.AnnotatedReference())
 	return err
 }
 
 // OnCompleted is called after the push is completed.
-func (p *PushHandler) OnCompleted(root ocispec.Descriptor) error {
-	_, err := fmt.Fprintln(p.out, "ArtifactType:", root.ArtifactType)
+func (h *PushHandler) OnCompleted(root ocispec.Descriptor) error {
+	_, err := fmt.Fprintln(h.out, "ArtifactType:", root.ArtifactType)
 	if err != nil {
 		return err
 	}
-	_, err = fmt.Fprintln(p.out, "Digest:", root.Digest)
+	_, err = fmt.Fprintln(h.out, "Digest:", root.Digest)
 	return err
 }

cmd/oras/internal/display/status/deprecated.go

@@ -0,0 +1,72 @@
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package status
+
+import (
+	"os"
+	"sync"
+
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"oras.land/oras-go/v2"
+	"oras.land/oras/internal/listener"
+)
+
+// Types and functions in this file are deprecated and should be removed when
+// no-longer referenced.
+
+// NewTagStatusHintPrinter creates a wrapper type for printing
+// tag status and hint.
+func NewTagStatusHintPrinter(target oras.Target, refPrefix string) oras.Target {
+	var printHint sync.Once
+	var printHintErr error
+	onTagging := func(desc ocispec.Descriptor, tag string) error {
+		printHint.Do(func() {
+			ref := refPrefix + "@" + desc.Digest.String()
+			printHintErr = Print("Tagging", ref)
+		})
+		return printHintErr
+	}
+	onTagged := func(desc ocispec.Descriptor, tag string) error {
+		return Print("Tagged", tag)
+	}
+	return listener.NewTagListener(target, onTagging, onTagged)
+}
+
+// NewTagStatusPrinter creates a wrapper type for printing tag status.
+func NewTagStatusPrinter(target oras.Target) oras.Target {
+	return listener.NewTagListener(target, nil, func(desc ocispec.Descriptor, tag string) error {
+		return Print("Tagged", tag)
+	})
+}
+
+// printer is used by the code being deprecated. Related functions should be
+// removed when no-longer referenced.
+var printer = NewPrinter(os.Stdout)
+
+// Print objects to display concurrent-safely.
+func Print(a ...any) error {
+	return printer.Println(a...)
+}
+
+// StatusPrinter returns a tracking function for transfer status.
+func StatusPrinter(status string, verbose bool) PrintFunc {
+	return printer.StatusPrinter(status, verbose)
+}
+
+// PrintStatus prints transfer status.
+func PrintStatus(desc ocispec.Descriptor, status string, verbose bool) error {
+	return printer.PrintStatus(desc, status, verbose)
+}

cmd/oras/internal/display/status/print.go

@@ -19,13 +19,10 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"os"
 	"sync"
 
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
-	"oras.land/oras-go/v2"
 	"oras.land/oras-go/v2/content"
-	"oras.land/oras-go/v2/registry"
 )
 
 // PrintFunc is the function type returned by StatusPrinter.
@@ -87,93 +84,3 @@ func PrintSuccessorStatus(ctx context.Context, desc ocispec.Descriptor, fetcher
 	}
 	return nil
 }
-
-// NewTagStatusHintPrinter creates a wrapper type for printing
-// tag status and hint.
-func NewTagStatusHintPrinter(target oras.Target, refPrefix string) oras.Target {
-	var printHint sync.Once
-	if repo, ok := target.(registry.Repository); ok {
-		return &tagManifestStatusForRepo{
-			Repository: repo,
-			printHint:  &printHint,
-			refPrefix:  refPrefix,
-		}
-	}
-	return &tagManifestStatusForTarget{
-		Target:    target,
-		printHint: &printHint,
-		refPrefix: refPrefix,
-	}
-}
-
-type tagManifestStatusForRepo struct {
-	registry.Repository
-	printHint *sync.Once
-	refPrefix string
-}
-
-// PushReference overrides Repository.PushReference method to print off which tag(s) were added successfully.
-func (p *tagManifestStatusForRepo) PushReference(ctx context.Context, expected ocispec.Descriptor, content io.Reader, reference string) error {
-	if p.printHint != nil {
-		p.printHint.Do(func() {
-			ref := p.refPrefix + "@" + expected.Digest.String()
-			_ = Print("Tagging", ref)
-		})
-	}
-	if err := p.Repository.PushReference(ctx, expected, content, reference); err != nil {
-		return err
-	}
-	return Print("Tagged", reference)
-}
-
-type tagManifestStatusForTarget struct {
-	oras.Target
-	printHint *sync.Once
-	refPrefix string
-}
-
-// Tag tags a descriptor with a reference string.
-func (p *tagManifestStatusForTarget) Tag(ctx context.Context, desc ocispec.Descriptor, reference string) error {
-	if p.printHint != nil {
-		p.printHint.Do(func() {
-			ref := p.refPrefix + "@" + desc.Digest.String()
-			_ = Print("Tagging", ref)
-		})
-	}
-
-	if err := p.Target.Tag(ctx, desc, reference); err != nil {
-		return err
-	}
-	return Print("Tagged", reference)
-}
-
-// NewTagStatusPrinter creates a wrapper type for printing tag status.
-func NewTagStatusPrinter(target oras.Target) oras.Target {
-	if repo, ok := target.(registry.Repository); ok {
-		return &tagManifestStatusForRepo{
-			Repository: repo,
-		}
-	}
-	return &tagManifestStatusForTarget{
-		Target: target,
-	}
-}
-
-// printer is used by the code being deprecated. Related functions should be
-// removed when no-longer referenced.
-var printer = NewPrinter(os.Stdout)
-
-// Print objects to display concurrent-safely.
-func Print(a ...any) error {
-	return printer.Println(a...)
-}
-
-// StatusPrinter returns a tracking function for transfer status.
-func StatusPrinter(status string, verbose bool) PrintFunc {
-	return printer.StatusPrinter(status, verbose)
-}
-
-// PrintStatus prints transfer status.
-func PrintStatus(desc ocispec.Descriptor, status string, verbose bool) error {
-	return printer.PrintStatus(desc, status, verbose)
-}

cmd/oras/internal/display/status/track/target.go

@@ -31,7 +31,6 @@ type GraphTarget interface {
 	oras.GraphTarget
 	io.Closer
 	Prompt(desc ocispec.Descriptor, prompt string) error
-	Inner() oras.GraphTarget
 }
 
 type graphTarget struct {
@@ -120,8 +119,3 @@ func (t *graphTarget) Prompt(desc ocispec.Descriptor, prompt string) error {
 	status <- progress.EndTiming()
 	return nil
 }
-
-// Inner returns the inner oras.GraphTarget.
-func (t *graphTarget) Inner() oras.GraphTarget {
-	return t.GraphTarget
-}

cmd/oras/internal/display/status/tty_test.go

@@ -93,8 +93,6 @@ func TestTTYPushHandler_TrackTarget(t *testing.T) {
 	}()
 	if ttyPushHandler, ok := ph.(*TTYPushHandler); !ok {
 		t.Errorf("TrackTarget() should return a *TTYPushHandler, got %T", ttyPushHandler)
-	} else if ttyPushHandler.tracked.Inner() != store {
-		t.Errorf("TrackTarget() tracks unexpected tracked target: %T", ttyPushHandler.tracked)
 	}
 }
 

cmd/oras/internal/display/utils/const.go

@@ -0,0 +1,26 @@
+/*
+Copyright The ORAS Authors.
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package utils
+
+// Prompt constants for pull.
+const (
+	PullPromptDownloading = "Downloading"
+	PullPromptPulled      = "Pulled     "
+	PullPromptProcessing  = "Processing "
+	PullPromptSkipped     = "Skipped    "
+	PullPromptRestored    = "Restored   "
+	PullPromptDownloaded  = "Downloaded "
+)

cmd/oras/internal/display/utils/utils.go → cmd/oras/internal/display/utils/json.go

@@ -22,16 +22,6 @@ import (
 	"io"
 )
 
-// Prompt constants for pull.
-const (
-	PullPromptDownloading = "Downloading"
-	PullPromptPulled      = "Pulled     "
-	PullPromptProcessing  = "Processing "
-	PullPromptSkipped     = "Skipped    "
-	PullPromptRestored    = "Restored   "
-	PullPromptDownloaded  = "Downloaded "
-)
-
 // PrintPrettyJSON prints the object to the writer in JSON format.
 func PrintPrettyJSON(out io.Writer, object any) error {
 	encoder := json.NewEncoder(out)

cmd/oras/root/push.go

@@ -29,11 +29,11 @@ import (
 	"oras.land/oras/cmd/oras/internal/argument"
 	"oras.land/oras/cmd/oras/internal/display"
 	"oras.land/oras/cmd/oras/internal/display/status"
-	"oras.land/oras/cmd/oras/internal/display/status/track"
 	oerrors "oras.land/oras/cmd/oras/internal/errors"
 	"oras.land/oras/cmd/oras/internal/fileref"
 	"oras.land/oras/cmd/oras/internal/option"
 	"oras.land/oras/internal/contentutil"
+	"oras.land/oras/internal/listener"
 	"oras.land/oras/internal/registryutil"
 )
 
@@ -196,11 +196,11 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error {
 	}
 
 	// prepare push
-	dst, err := opts.NewTarget(opts.Common, logger)
+	originalDst, err := opts.NewTarget(opts.Common, logger)
 	if err != nil {
 		return err
 	}
-	dst, stopTrack, err := displayStatus.TrackTarget(dst)
+	dst, stopTrack, err := displayStatus.TrackTarget(originalDst)
 	if err != nil {
 		return err
 	}
@@ -232,17 +232,14 @@ func runPush(cmd *cobra.Command, opts *pushOptions) error {
 	}
 
 	if len(opts.extraRefs) != 0 {
-		taggable := dst
-		if tracked, ok := dst.(track.GraphTarget); ok {
-			taggable = tracked.Inner()
-		}
 		contentBytes, err := content.FetchAll(ctx, memoryStore, root)
 		if err != nil {
 			return err
 		}
 		tagBytesNOpts := oras.DefaultTagBytesNOptions
 		tagBytesNOpts.Concurrency = opts.concurrency
-		if _, err = oras.TagBytesN(ctx, status.NewTagStatusPrinter(taggable), root.MediaType, contentBytes, opts.extraRefs, tagBytesNOpts); err != nil {
+		dst := listener.NewTagListener(originalDst, nil, displayMetadata.OnTagged)
+		if _, err = oras.TagBytesN(ctx, dst, root.MediaType, contentBytes, opts.extraRefs, tagBytesNOpts); err != nil {
 			return err
 		}
 	}