From 03ff401b7443b44de30b4b23207ebcd99f61c533 Mon Sep 17 00:00:00 2001 From: Billy Zha Date: Mon, 25 Dec 2023 16:16:03 +0800 Subject: [PATCH] fix: bug in pushing with multiple tags (#1209) Signed-off-by: Billy Zha --- cmd/oras/internal/display/track/target.go | 6 +++ cmd/oras/root/attach.go | 6 +-- cmd/oras/root/pull.go | 22 +++++----- cmd/oras/root/push.go | 52 ++++++++++++----------- 4 files changed, 46 insertions(+), 40 deletions(-) diff --git a/cmd/oras/internal/display/track/target.go b/cmd/oras/internal/display/track/target.go index 28ac8c96f..5ff07efe3 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,8 @@ 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 +} diff --git a/cmd/oras/root/attach.go b/cmd/oras/root/attach.go index a8c055bdd..528a01b47 100644 --- a/cmd/oras/root/attach.go +++ b/cmd/oras/root/attach.go @@ -28,7 +28,6 @@ import ( "oras.land/oras-go/v2/content/file" "oras.land/oras-go/v2/registry/remote/auth" "oras.land/oras/cmd/oras/internal/argument" - "oras.land/oras/cmd/oras/internal/display/track" oerrors "oras.land/oras/cmd/oras/internal/errors" "oras.land/oras/cmd/oras/internal/option" "oras.land/oras/internal/graph" @@ -137,14 +136,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 c96c60ffa..bf39e05e6 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -171,12 +171,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, ok := dst.(track.GraphTarget); ok { defer tracked.Close() } var layerSkipped atomic.Bool @@ -247,7 +246,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 @@ -278,7 +277,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 } } @@ -305,14 +304,15 @@ 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 { - // 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 c9bda91de..0ee581a5f 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -186,15 +186,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 @@ -216,13 +215,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 } } @@ -241,7 +244,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 ( @@ -250,50 +253,49 @@ func updateDisplayOption(opts *oras.CopyGraphOptions, fetcher content.Fetcher, v promptExists = "Exists " promptUploading = "Uploading" ) - - if tracked == nil { - // 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) } } 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) {