Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: bug in pushing with multiple tags #1209

Merged
merged 7 commits into from
Dec 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cmd/oras/internal/display/track/target.go
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
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
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
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
Loading