From d9b2c5c0f0d69ab9fccb41e70240fb935bf8ab9c Mon Sep 17 00:00:00 2001 From: Sajay Antony Date: Fri, 3 Nov 2023 03:19:38 -0700 Subject: [PATCH] fix: avoid copy suggestion when only config is skipped (#1164) Signed-off-by: Sajay Antony Signed-off-by: Billy Zha Co-authored-by: Billy Zha --- cmd/oras/root/cp.go | 21 ++++++++++++++------- cmd/oras/root/pull.go | 29 +++++++++++++++++++++-------- cmd/oras/root/push.go | 19 +++++++++++++------ test/e2e/suite/command/pull.go | 5 ++--- 4 files changed, 50 insertions(+), 24 deletions(-) diff --git a/cmd/oras/root/cp.go b/cmd/oras/root/cp.go index 8df14084f..2bd463b8c 100644 --- a/cmd/oras/root/cp.go +++ b/cmd/oras/root/cp.go @@ -154,25 +154,32 @@ 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 } @@ -180,12 +187,12 @@ func doCopy(ctx context.Context, src oras.ReadOnlyGraphTarget, dst oras.GraphTar 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) }) } } diff --git a/cmd/oras/root/pull.go b/cmd/oras/root/pull.go index 27afa98b9..fb5d50e4f 100644 --- a/cmd/oras/root/pull.go +++ b/cmd/oras/root/pull.go @@ -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 { @@ -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 } } @@ -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 }) @@ -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] != "" { + nodes = append(nodes, *config) + } } var ret []ocispec.Descriptor for _, s := range nodes { if s.Annotations[ocispec.AnnotationTitle] == "" { - skippedLayers++ + if content.Equal(s, ocispec.DescriptorEmptyJSON) { + skippedLayers++ + } 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 @@ -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 @@ -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 } } @@ -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 diff --git a/cmd/oras/root/push.go b/cmd/oras/root/push.go index 740ad29b2..c9dc0a7f7 100644 --- a/cmd/oras/root/push.go +++ b/cmd/oras/root/push.go @@ -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) }) } } diff --git a/test/e2e/suite/command/pull.go b/test/e2e/suite/command/pull.go index 65d5e6eea..e12ee232c 100644 --- a/test/e2e/suite/command/pull.go +++ b/test/e2e/suite/command/pull.go @@ -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" @@ -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() @@ -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()