Skip to content

Commit

Permalink
fix: bug in pushing with multiple tags (oras-project#1209)
Browse files Browse the repository at this point in the history 
Signed-off-by: Billy Zha <[email protected]>
  • Loading branch information
@qweeah
qweeah authored Dec 25, 2023
1 parent 4aca01f commit 03ff401
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 40 deletions.
6 changes: 6 additions & 0 deletions cmd/oras/internal/display/track/target.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type GraphTarget interface {
oras.GraphTarget
io.Closer
Prompt(desc ocispec.Descriptor, prompt string) error
Inner() oras.GraphTarget
}

type graphTarget struct {
Expand Down Expand Up @@ -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
}
6 changes: 2 additions & 4 deletions cmd/oras/root/attach.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
22 changes: 11 additions & 11 deletions cmd/oras/root/pull.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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)
}
52 changes: 27 additions & 25 deletions cmd/oras/root/push.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(&copyOptions.CopyGraphOptions, union, opts.Verbose, tracked)
updateDisplayOption(&copyOptions.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
Expand All @@ -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
}
}
Expand All @@ -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 (
Expand All @@ -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) {
Expand Down

0 comments on commit 03ff401

Please sign in to comment.