From 390ee5d729add123787ee3dc4617fa6e7f51e29f Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 22 Dec 2023 03:46:54 +0000 Subject: [PATCH 1/6] fix: bug in pushing with multiple tags Signed-off-by: Billy Zha --- cmd/oras/internal/display/track/target.go | 11 ++++++++++ cmd/oras/root/attach.go | 6 ++---- cmd/oras/root/pull.go | 14 ++++++------- cmd/oras/root/push.go | 25 +++++++++++++---------- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/cmd/oras/internal/display/track/target.go b/cmd/oras/internal/display/track/target.go index 28ac8c96f..09496b740 100644 --- a/cmd/oras/internal/display/track/target.go +++ b/cmd/oras/internal/display/track/target.go @@ -31,6 +31,7 @@ type GraphTarget interface { oras.GraphTarget io.Closer Prompt(desc ocispec.Descriptor, prompt string) error + Inner() oras.GraphTarget } type graphTarget struct { @@ -112,3 +113,13 @@ 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 +} + +// Inner returns the inner oras.GraphTarget. +func (t *referenceGraphTarget) Inner() oras.GraphTarget { + return t.GraphTarget +} diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index c485c1927..f6b34dc63 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -27,7 +27,6 @@ import ( "oras.land/oras-go/v2/content" "oras.land/oras-go/v2/content/file" "oras.land/oras-go/v2/registry/remote/auth" - "oras.land/oras/cmd/oras/internal/display/track" "oras.land/oras/cmd/oras/internal/option" "oras.land/oras/internal/graph" "oras.land/oras/internal/registryutil" @@ -135,14 +134,13 @@ func runAttach(ctx context.Context, opts attachOptions) error { } // prepare push - var tracked track.GraphTarget - dst, tracked, err = getTrackedTarget(dst, opts.TTY, "Uploading", "Uploaded ") + dst, err = getTrackedTarget(dst, opts.TTY, "Uploading", "Uploaded ") if err != nil { return err } graphCopyOptions := oras.DefaultCopyGraphOptions graphCopyOptions.Concurrency = opts.concurrency - updateDisplayOption(&graphCopyOptions, store, opts.Verbose, tracked) + updateDisplayOption(&graphCopyOptions, store, opts.Verbose, dst) packOpts := oras.PackManifestOptions{ Subject: &subject, diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index e98b03691..dc842ae5f 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -169,12 +169,11 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget, promptDownloaded = "Downloaded " ) - var tracked track.GraphTarget - dst, tracked, err = getTrackedTarget(dst, po.TTY, "Downloading", "Pulled ") + dst, err = getTrackedTarget(dst, po.TTY, "Downloading", "Pulled ") if err != nil { return ocispec.Descriptor{}, false, err } - if tracked != nil { + if tracked := dst.(track.GraphTarget); tracked != nil { defer tracked.Close() } var layerSkipped atomic.Bool @@ -245,7 +244,7 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget, } if len(ss) == 0 { // skip s if it is unnamed AND has no successors. - if err := printOnce(&printed, s, promptSkipped, po.Verbose, tracked); err != nil { + if err := printOnce(&printed, s, promptSkipped, po.Verbose, dst); err != nil { return nil, err } continue @@ -276,7 +275,7 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget, } for _, s := range successors { if _, ok := s.Annotations[ocispec.AnnotationTitle]; ok { - if err := printOnce(&printed, s, promptRestored, po.Verbose, tracked); err != nil { + if err := printOnce(&printed, s, promptRestored, po.Verbose, dst); err != nil { return err } } @@ -303,11 +302,12 @@ func generateContentKey(desc ocispec.Descriptor) string { return desc.Digest.String() + desc.Annotations[ocispec.AnnotationTitle] } -func printOnce(printed *sync.Map, s ocispec.Descriptor, msg string, verbose bool, tracked track.GraphTarget) error { +func printOnce(printed *sync.Map, s ocispec.Descriptor, msg string, verbose bool, dst any) error { if _, loaded := printed.LoadOrStore(generateContentKey(s), true); loaded { return nil } - if tracked == nil { + tracked, ok := dst.(track.GraphTarget) + if ok { // none TTY return display.PrintStatus(s, msg, verbose) } diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index c9dc0a7f7..df34ea6c1 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -184,15 +184,14 @@ func runPush(ctx context.Context, opts pushOptions) error { if err != nil { return err } - var tracked track.GraphTarget - dst, tracked, err = getTrackedTarget(dst, opts.TTY, "Uploading", "Uploaded ") + dst, err = getTrackedTarget(dst, opts.TTY, "Uploading", "Uploaded ") if err != nil { return err } copyOptions := oras.DefaultCopyOptions copyOptions.Concurrency = opts.concurrency union := contentutil.MultiReadOnlyTarget(memoryStore, store) - updateDisplayOption(©Options.CopyGraphOptions, union, opts.Verbose, tracked) + updateDisplayOption(©Options.CopyGraphOptions, union, opts.Verbose, dst) copy := func(root ocispec.Descriptor) error { // add both pull and push scope hints for dst repository // to save potential push-scope token requests during copy @@ -214,13 +213,17 @@ func runPush(ctx context.Context, opts pushOptions) error { fmt.Println("Pushed", opts.AnnotatedReference()) 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, display.NewTagStatusPrinter(dst), root.MediaType, contentBytes, opts.extraRefs, tagBytesNOpts); err != nil { + if _, err = oras.TagBytesN(ctx, display.NewTagStatusPrinter(taggable), root.MediaType, contentBytes, opts.extraRefs, tagBytesNOpts); err != nil { return err } } @@ -239,7 +242,7 @@ func doPush(dst oras.Target, pack packFunc, copy copyFunc) (ocispec.Descriptor, return pushArtifact(dst, pack, copy) } -func updateDisplayOption(opts *oras.CopyGraphOptions, fetcher content.Fetcher, verbose bool, tracked track.GraphTarget) { +func updateDisplayOption(opts *oras.CopyGraphOptions, fetcher content.Fetcher, verbose bool, dst any) { committed := &sync.Map{} const ( @@ -248,8 +251,8 @@ func updateDisplayOption(opts *oras.CopyGraphOptions, fetcher content.Fetcher, v promptExists = "Exists " promptUploading = "Uploading" ) - - if tracked == nil { + tracked, ok := dst.(track.GraphTarget) + if ok { // non TTY opts.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error { committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) @@ -283,15 +286,15 @@ func updateDisplayOption(opts *oras.CopyGraphOptions, fetcher content.Fetcher, v type packFunc func() (ocispec.Descriptor, error) type copyFunc func(desc ocispec.Descriptor) error -func getTrackedTarget(gt oras.GraphTarget, tty *os.File, actionPrompt, doneprompt string) (oras.GraphTarget, track.GraphTarget, error) { +func getTrackedTarget(gt oras.GraphTarget, tty *os.File, actionPrompt, doneprompt string) (oras.GraphTarget, error) { if tty == nil { - return gt, nil, nil + return gt, nil } tracked, err := track.NewTarget(gt, actionPrompt, doneprompt, tty) if err != nil { - return nil, nil, err + return nil, err } - return tracked, tracked, nil + return tracked, nil } func pushArtifact(dst oras.Target, pack packFunc, copy copyFunc) (ocispec.Descriptor, error) { From a1660cc0ddfd09ea9e99930df08bc56068e2a368 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 22 Dec 2023 03:55:40 +0000 Subject: [PATCH 2/6] fix panic Signed-off-by: Billy Zha --- cmd/oras/root/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index dc842ae5f..38f51570f 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -173,7 +173,7 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget, if err != nil { return ocispec.Descriptor{}, false, err } - if tracked := dst.(track.GraphTarget); tracked != nil { + if tracked, ok := dst.(track.GraphTarget); ok { defer tracked.Close() } var layerSkipped atomic.Bool From 2675540dcf96e4c6589504f91b3fc4bbd51f508c Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 22 Dec 2023 03:59:17 +0000 Subject: [PATCH 3/6] fix non-tty output Signed-off-by: Billy Zha --- cmd/oras/root/push.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index df34ea6c1..78f8190f4 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -252,7 +252,7 @@ func updateDisplayOption(opts *oras.CopyGraphOptions, fetcher content.Fetcher, v promptUploading = "Uploading" ) tracked, ok := dst.(track.GraphTarget) - if ok { + if !ok { // non TTY opts.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error { committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) From 8381f2b33ad7d42efc74d0663451fab5631c423c Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Fri, 22 Dec 2023 04:02:23 +0000 Subject: [PATCH 4/6] fix non-tty Signed-off-by: Billy Zha --- cmd/oras/root/pull.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 38f51570f..954110f88 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -307,7 +307,7 @@ func printOnce(printed *sync.Map, s ocispec.Descriptor, msg string, verbose bool return nil } tracked, ok := dst.(track.GraphTarget) - if ok { + if !ok { // none TTY return display.PrintStatus(s, msg, verbose) } From b9c8e434d11e5332a1101035c7d306668080e6f3 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 25 Dec 2023 02:00:24 +0000 Subject: [PATCH 5/6] code clean Signed-off-by: Billy Zha --- cmd/oras/root/pull.go | 12 ++++++------ cmd/oras/root/push.go | 31 +++++++++++++++---------------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 60b275877..bf39e05e6 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -308,11 +308,11 @@ func printOnce(printed *sync.Map, s ocispec.Descriptor, msg string, verbose bool if _, loaded := printed.LoadOrStore(generateContentKey(s), true); loaded { return nil } - tracked, ok := dst.(track.GraphTarget) - if !ok { - // none TTY - return display.PrintStatus(s, msg, verbose) + if tracked, ok := dst.(track.GraphTarget); ok { + // TTY + return tracked.Prompt(s, msg) + } - // TTY - return tracked.Prompt(s, msg) + // none TTY + return display.PrintStatus(s, msg, verbose) } diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index 12e2fa500..0ee581a5f 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -253,35 +253,34 @@ func updateDisplayOption(opts *oras.CopyGraphOptions, fetcher content.Fetcher, v promptExists = "Exists " promptUploading = "Uploading" ) - tracked, ok := dst.(track.GraphTarget) - if !ok { - // non TTY + if tracked, ok := dst.(track.GraphTarget); ok { + // TTY opts.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error { committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) - return display.PrintStatus(desc, promptExists, verbose) - } - opts.PreCopy = func(ctx context.Context, desc ocispec.Descriptor) error { - return display.PrintStatus(desc, promptUploading, verbose) + return tracked.Prompt(desc, promptExists) } opts.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error { committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) - if err := display.PrintSuccessorStatus(ctx, desc, fetcher, committed, display.StatusPrinter(promptSkipped, verbose)); err != nil { - return err - } - return display.PrintStatus(desc, promptUploaded, verbose) + return display.PrintSuccessorStatus(ctx, desc, fetcher, committed, func(d ocispec.Descriptor) error { + return tracked.Prompt(d, promptSkipped) + }) } return } - // TTY + // non TTY opts.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error { committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) - return tracked.Prompt(desc, promptExists) + return display.PrintStatus(desc, promptExists, verbose) + } + opts.PreCopy = func(ctx context.Context, desc ocispec.Descriptor) error { + return display.PrintStatus(desc, promptUploading, verbose) } opts.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error { committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle]) - return display.PrintSuccessorStatus(ctx, desc, fetcher, committed, func(d ocispec.Descriptor) error { - return tracked.Prompt(d, promptSkipped) - }) + if err := display.PrintSuccessorStatus(ctx, desc, fetcher, committed, display.StatusPrinter(promptSkipped, verbose)); err != nil { + return err + } + return display.PrintStatus(desc, promptUploaded, verbose) } } From b38a5f89cd60967759ce0bc60007950c677a344d Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 25 Dec 2023 02:11:06 +0000 Subject: [PATCH 6/6] code clean Signed-off-by: Billy Zha --- cmd/oras/internal/display/track/target.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cmd/oras/internal/display/track/target.go b/cmd/oras/internal/display/track/target.go index 09496b740..5ff07efe3 100644 --- a/cmd/oras/internal/display/track/target.go +++ b/cmd/oras/internal/display/track/target.go @@ -118,8 +118,3 @@ func (t *graphTarget) Prompt(desc ocispec.Descriptor, prompt string) error { func (t *graphTarget) Inner() oras.GraphTarget { return t.GraphTarget } - -// Inner returns the inner oras.GraphTarget. -func (t *referenceGraphTarget) Inner() oras.GraphTarget { - return t.GraphTarget -}