From a52e9742ce35d5abe544c1aabd6f183048f64eac Mon Sep 17 00:00:00 2001
From: Billy Zha <jinzha1@microsoft.com>
Date: Thu, 5 Dec 2024 08:10:52 +0000
Subject: [PATCH] fix attach

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
---
 cmd/oras/root/attach.go | 49 +++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go
index 596e98ec1..90508a2f4 100644
--- a/cmd/oras/root/attach.go
+++ b/cmd/oras/root/attach.go
@@ -44,12 +44,13 @@ type attachOptions struct {
 
 	artifactType string
 	concurrency  int
+	verbose      bool
 }
 
 func attachCmd() *cobra.Command {
 	var opts attachOptions
 	cmd := &cobra.Command{
-		Use:   "attach [flags] --artifact-type=<type> <name>{:<tag>|@<digest>} <file>[:<layer_media_type>] [...]",
+		Use:   "attach [flags] --artifact-type=<type> <name>{:<tag>|@<digest>} {<file>[:<layer_media_type>]|--annotation <key>=<value>} [...]",
 		Short: "[Preview] Attach files to an existing artifact",
 		Long: `[Preview] Attach files to an existing artifact
 
@@ -93,38 +94,41 @@ Example - Attach file to the manifest tagged 'v1' in an OCI image layout folder
 		PreRunE: func(cmd *cobra.Command, args []string) error {
 			opts.RawReference = args[0]
 			opts.FileRefs = args[1:]
-			if err := option.Parse(cmd, &opts); err != nil {
-				return err
+			err := option.Parse(cmd, &opts)
+			if err == nil {
+				if err = opts.EnsureReferenceNotEmpty(cmd, true); err == nil {
+					return nil
+				}
+			}
+			if len(opts.FileRefs) == 0 {
+				// no file argument provided
+				if err, ok := err.(*oerrors.Error); ok && err.OperationType == oerrors.OperationTypeParseArtifactReference {
+					// invalid reference
+					err.Recommendation = fmt.Sprintf("Are you missing an artifact reference to attach to? %s", err.Recommendation)
+				}
 			}
-			return nil
+			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()
-	opts.AllowedTypes = []*option.FormatType{option.FormatTypeJSON, option.FormatTypeGoTemplate}
+	opts.SetTypes(option.FormatTypeText, option.FormatTypeJSON, option.FormatTypeGoTemplate)
 	option.ApplyFlags(&opts, cmd.Flags())
 	return oerrors.Command(cmd, &opts.Target)
 }
 
 func runAttach(cmd *cobra.Command, opts *attachOptions) error {
 	ctx, logger := command.GetLogger(cmd, &opts.Common)
-	displayStatus, displayMetadata, err := display.NewAttachHandler(cmd.OutOrStdout(), opts.Format, opts.TTY, opts.Verbose)
-	if err != nil {
-		return err
-	}
-
-	annotations, err := opts.LoadManifestAnnotations()
-	if err != nil {
-		return err
-	}
-	if len(opts.FileRefs) == 0 && len(annotations[option.AnnotationManifest]) == 0 {
+	if len(opts.FileRefs) == 0 && len(opts.Annotations[option.AnnotationManifest]) == 0 {
 		return &oerrors.Error{
 			Err:            errors.New(`neither file nor annotation provided in the command`),
 			Usage:          fmt.Sprintf("%s %s", cmd.Parent().CommandPath(), cmd.Use),
@@ -143,9 +147,6 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error {
 	if err != nil {
 		return err
 	}
-	if err := opts.EnsureReferenceNotEmpty(cmd, true); err != nil {
-		return err
-	}
 	// add both pull and push scope hints for dst repository
 	// to save potential push-scope token requests during copy
 	ctx = registryutil.WithScopeHint(ctx, dst, auth.ActionPull, auth.ActionPush)
@@ -155,7 +156,11 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error {
 	if err != nil {
 		return fmt.Errorf("failed to resolve %s: %w", opts.Reference, err)
 	}
-	descs, err := loadFiles(ctx, store, annotations, opts.FileRefs, displayStatus)
+	displayStatus, displayMetadata, err := display.NewAttachHandler(opts.Printer, opts.Format, opts.TTY, store)
+	if err != nil {
+		return err
+	}
+	descs, err := loadFiles(ctx, store, opts.Annotations, opts.FileRefs, displayStatus)
 	if err != nil {
 		return err
 	}
@@ -167,11 +172,13 @@ func runAttach(cmd *cobra.Command, opts *attachOptions) error {
 	}
 	graphCopyOptions := oras.DefaultCopyGraphOptions
 	graphCopyOptions.Concurrency = opts.concurrency
-	displayStatus.UpdateCopyOptions(&graphCopyOptions, store)
+	graphCopyOptions.OnCopySkipped = displayStatus.OnCopySkipped
+	graphCopyOptions.PreCopy = displayStatus.PreCopy
+	graphCopyOptions.PostCopy = displayStatus.PostCopy
 
 	packOpts := oras.PackManifestOptions{
 		Subject:             &subject,
-		ManifestAnnotations: annotations[option.AnnotationManifest],
+		ManifestAnnotations: opts.Annotations[option.AnnotationManifest],
 		Layers:              descs,
 	}
 	pack := func() (ocispec.Descriptor, error) {