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: avoid copy suggestion when only config is skipped #1164

Merged
merged 8 commits into from
Nov 3, 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
21 changes: 14 additions & 7 deletions cmd/oras/root/cp.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,38 +154,45 @@ func doCopy(ctx context.Context, src oras.ReadOnlyGraphTarget, dst oras.GraphTar
return graph.Referrers(ctx, src, desc, "")
}

const (
promptExists = "Exists "
promptCopying = "Copying"
promptCopied = "Copied "
promptSkipped = "Skipped"
)

if opts.TTY == nil {
// none TTY output
extendedCopyOptions.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error {
committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
return display.PrintStatus(desc, "Exists ", opts.Verbose)
return display.PrintStatus(desc, promptExists, opts.Verbose)
}
extendedCopyOptions.PreCopy = func(ctx context.Context, desc ocispec.Descriptor) error {
return display.PrintStatus(desc, "Copying", opts.Verbose)
return display.PrintStatus(desc, promptCopying, opts.Verbose)
}
extendedCopyOptions.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error {
committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
if err := display.PrintSuccessorStatus(ctx, desc, dst, committed, display.StatusPrinter("Skipped", opts.Verbose)); err != nil {
if err := display.PrintSuccessorStatus(ctx, desc, dst, committed, display.StatusPrinter(promptSkipped, opts.Verbose)); err != nil {
return err
}
return display.PrintStatus(desc, "Copied ", opts.Verbose)
return display.PrintStatus(desc, promptCopied, opts.Verbose)
}
} else {
// TTY output
tracked, err := track.NewTarget(dst, "Copying ", "Copied ", opts.TTY)
tracked, err := track.NewTarget(dst, promptCopying, promptCopied, opts.TTY)
if err != nil {
return ocispec.Descriptor{}, err
}
defer tracked.Close()
dst = tracked
extendedCopyOptions.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error {
committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
return tracked.Prompt(desc, "Exists ")
return tracked.Prompt(desc, promptExists)
}
extendedCopyOptions.PostCopy = func(ctx context.Context, desc ocispec.Descriptor) error {
committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
return display.PrintSuccessorStatus(ctx, desc, tracked, committed, func(desc ocispec.Descriptor) error {
return tracked.Prompt(desc, "Skipped")
return tracked.Prompt(desc, promptSkipped)
})
}
}
Expand Down
29 changes: 21 additions & 8 deletions cmd/oras/root/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,15 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget,
}
}

const (
promptDownloading = "Downloading"
promptPulled = "Pulled "
promptProcessing = "Processing "
promptSkipped = "Skipped "
promptRestored = "Restored "
promptDownloaded = "Downloaded "
)

var tracked track.GraphTarget
dst, tracked, err = getTrackedTarget(dst, po.TTY, "Downloading", "Pulled ")
if err != nil {
Expand All @@ -177,7 +186,7 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget,
}
if po.TTY == nil {
// none TTY, print status log for first-time fetching
if err := display.PrintStatus(target, "Downloading", po.Verbose); err != nil {
if err := display.PrintStatus(target, promptDownloading, po.Verbose); err != nil {
return nil, err
}
}
Expand All @@ -192,7 +201,7 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget,
}()
if po.TTY == nil {
// none TTY, add logs for processing manifest
return rc, display.PrintStatus(target, "Processing ", po.Verbose)
return rc, display.PrintStatus(target, promptProcessing, po.Verbose)
}
return rc, nil
})
Expand All @@ -213,20 +222,24 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget,
config.Annotations[ocispec.AnnotationTitle] = configPath
}
})
nodes = append(nodes, *config)
if config.Size != ocispec.DescriptorEmptyJSON.Size || config.Digest != ocispec.DescriptorEmptyJSON.Digest || config.Annotations[ocispec.AnnotationTitle] != "" {
qweeah marked this conversation as resolved.
Show resolved Hide resolved
nodes = append(nodes, *config)
}
}

var ret []ocispec.Descriptor
qweeah marked this conversation as resolved.
Show resolved Hide resolved
for _, s := range nodes {
if s.Annotations[ocispec.AnnotationTitle] == "" {
skippedLayers++
if content.Equal(s, ocispec.DescriptorEmptyJSON) {
skippedLayers++
qweeah marked this conversation as resolved.
Show resolved Hide resolved
}
ss, err := content.Successors(ctx, fetcher, s)
if err != nil {
return nil, err
}
if len(ss) == 0 {
// skip s if it is unnamed AND has no successors.
if err := printOnce(&printed, s, "Skipped ", po.Verbose, tracked); err != nil {
if err := printOnce(&printed, s, promptSkipped, po.Verbose, tracked); err != nil {
return nil, err
}
continue
Expand All @@ -244,7 +257,7 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget,
}
if po.TTY == nil {
// none TTY, print status log for downloading
return display.PrintStatus(desc, "Downloading", po.Verbose)
return display.PrintStatus(desc, promptDownloading, po.Verbose)
}
// TTY
return nil
Expand All @@ -257,7 +270,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, "Restored ", po.Verbose, tracked); err != nil {
if err := printOnce(&printed, s, promptRestored, po.Verbose, tracked); err != nil {
return err
}
}
Expand All @@ -270,7 +283,7 @@ func doPull(ctx context.Context, src oras.ReadOnlyTarget, dst oras.GraphTarget,
name = desc.MediaType
}
printed.Store(generateContentKey(desc), true)
return display.Print("Downloaded ", display.ShortDigest(desc), name)
return display.Print(promptDownloaded, display.ShortDigest(desc), name)
}

// Copy
Expand Down
19 changes: 13 additions & 6 deletions cmd/oras/root/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,33 +242,40 @@ func doPush(dst oras.Target, pack packFunc, copy copyFunc) (ocispec.Descriptor,
func updateDisplayOption(opts *oras.CopyGraphOptions, fetcher content.Fetcher, verbose bool, tracked track.GraphTarget) {
committed := &sync.Map{}

const (
promptSkipped = "Skipped "
promptUploaded = "Uploaded "
promptExists = "Exists "
promptUploading = "Uploading"
)

if tracked == nil {
// non TTY
opts.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error {
committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
return display.PrintStatus(desc, "Exists ", verbose)
return display.PrintStatus(desc, promptExists, verbose)
}
opts.PreCopy = func(ctx context.Context, desc ocispec.Descriptor) error {
return display.PrintStatus(desc, "Uploading", verbose)
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])
if err := display.PrintSuccessorStatus(ctx, desc, fetcher, committed, display.StatusPrinter("Skipped ", verbose)); err != nil {
if err := display.PrintSuccessorStatus(ctx, desc, fetcher, committed, display.StatusPrinter(promptSkipped, verbose)); err != nil {
return err
}
return display.PrintStatus(desc, "Uploaded ", verbose)
return display.PrintStatus(desc, promptUploaded, verbose)
}
return
}
// TTY
opts.OnCopySkipped = func(ctx context.Context, desc ocispec.Descriptor) error {
committed.Store(desc.Digest.String(), desc.Annotations[ocispec.AnnotationTitle])
return tracked.Prompt(desc, "Exists ")
return tracked.Prompt(desc, promptExists)
}
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, "Skipped ")
return tracked.Prompt(d, promptSkipped)
})
}
}
Expand Down
5 changes: 2 additions & 3 deletions test/e2e/suite/command/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/onsi/gomega"
. "github.com/onsi/gomega"
"github.com/onsi/gomega/gbytes"
"oras.land/oras-go/v2"
"oras.land/oras/test/e2e/internal/testdata/artifact/blob"
"oras.land/oras/test/e2e/internal/testdata/artifact/config"
"oras.land/oras/test/e2e/internal/testdata/feature"
Expand Down Expand Up @@ -77,7 +76,7 @@ var _ = Describe("OCI spec 1.1 registry users:", func() {
It("should skip config if media type not matching", func() {
pullRoot := "pulled"
tempDir := PrepareTempFiles()
stateKeys := append(foobar.ImageLayerStateKeys, foobar.ManifestStateKey, foobar.ImageConfigStateKey(oras.MediaTypeUnknownConfig))
stateKeys := append(foobar.ImageLayerStateKeys, foobar.ManifestStateKey)
ORAS("pull", RegistryRef(ZOTHost, ImageRepo, foobar.Tag), "-v", "--config", fmt.Sprintf("%s:%s", configName, "???"), "-o", pullRoot).
MatchStatus(stateKeys, true, len(stateKeys)).
WithWorkDir(tempDir).Exec()
Expand Down Expand Up @@ -206,7 +205,7 @@ var _ = Describe("OCI image layout users:", func() {
It("should skip config if media type does not match", func() {
pullRoot := "pulled"
root := PrepareTempOCI(ImageRepo)
stateKeys := append(foobar.ImageLayerStateKeys, foobar.ManifestStateKey, foobar.ImageConfigStateKey(oras.MediaTypeUnknownConfig))
stateKeys := append(foobar.ImageLayerStateKeys, foobar.ManifestStateKey)
ORAS("pull", Flags.Layout, LayoutRef(root, foobar.Tag), "-v", "--config", fmt.Sprintf("%s:%s", configName, "???"), "-o", pullRoot).
MatchStatus(stateKeys, true, len(stateKeys)).
WithWorkDir(root).Exec()
Expand Down
Loading