Skip to content

Commit

Permalink
fix: avoid copy suggestion when only config is skipped (#1164)
Browse files Browse the repository at this point in the history 
Signed-off-by: Sajay Antony <[email protected]>
Signed-off-by: Billy Zha <[email protected]>
Co-authored-by: Billy Zha <[email protected]>
  • Loading branch information
@sajayantony @qweeah
sajayantony and qweeah authored Nov 3, 2023
1 parent b911ad6 commit d9b2c5c
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 24 deletions.
21 changes: 14 additions & 7 deletions cmd/oras/root/cp.go
View file
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
View file
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] != "" {
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
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
View file
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
View file
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

0 comments on commit d9b2c5c

Please sign in to comment.