Skip to content

Commit

Permalink
fix(cp): avoid showing empty descriptor in progress bar (#1528)
Browse files Browse the repository at this point in the history
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Co-authored-by: Terry Howe <terrylhowe@gmail.com>
  • Loading branch information
qweeah and TerryHowe authored Nov 11, 2024
1 parent b8db531 commit b4be9a6
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 6 deletions.
15 changes: 10 additions & 5 deletions cmd/oras/internal/display/status/progress/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ func (s *status) String(width int) (string, string) {
}
// todo: doesn't support multiline prompt
total := uint64(s.descriptor.Size)
var percent float64

name := s.descriptor.Annotations["org.opencontainers.image.title"]
if name == "" {
Expand All @@ -81,12 +80,18 @@ func (s *status) String(width int) (string, string) {
// mark(1) bar(22) speed(8) action(<=11) name(<=126) size_per_size(<=13) percent(8) time(>=6)
// └─ digest(72)
var offset string
switch s.done {
case true: // 100%, show exact size
var percent float64
if s.done {
// 100%, show exact size
offset = fmt.Sprint(s.total.Size)
percent = 1
default: // 0% ~ 99%, show 2-digit precision
if total != 0 && s.offset >= 0 {
} else if total == 0 {
// 0 byte, show 100%
offset = "0"
percent = 1
} else {
// 0% ~ 99%, show 2-digit precision
if s.offset >= 0 {
// calculate percentage
percent = float64(s.offset) / float64(total)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/oras/internal/display/status/progress/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func Test_status_String_zeroWidth(t *testing.T) {
})
// not done
statusStr, digestStr := s.String(120)
if err := testutils.OrderedMatch(statusStr+digestStr, "\x1b[0m....................]", s.prompt, s.descriptor.MediaType, "0.00/0 B", "0.00%", s.descriptor.Digest.String()); err != nil {
if err := testutils.OrderedMatch(statusStr+digestStr, "\x1b[104m \x1b[0m", s.prompt, s.descriptor.MediaType, "0/0 B", "100.00%", s.descriptor.Digest.String()); err != nil {
t.Error(err)
}
// done
Expand Down
6 changes: 6 additions & 0 deletions cmd/oras/internal/display/status/track/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ package track

import (
"context"
"errors"
"io"
"os"

ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras-go/v2"
"oras.land/oras-go/v2/errdef"
"oras.land/oras-go/v2/registry"
"oras.land/oras/cmd/oras/internal/display/status/progress"
)
Expand Down Expand Up @@ -81,6 +83,10 @@ func (t *graphTarget) Push(ctx context.Context, expected ocispec.Descriptor, con
defer r.Close()
r.Start()
if err := t.GraphTarget.Push(ctx, expected, r); err != nil {
if errors.Is(err, errdef.ErrAlreadyExists) {
// allowed error types in oras-go oci and memory store
r.Done()
}
return err
}
r.Done()
Expand Down
43 changes: 43 additions & 0 deletions cmd/oras/internal/display/status/track/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@ package track
import (
"bytes"
"context"
"errors"
"io"
"testing"

"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
"oras.land/oras-go/v2"
"oras.land/oras-go/v2/content/memory"
"oras.land/oras-go/v2/errdef"
"oras.land/oras-go/v2/registry/remote"
"oras.land/oras/internal/testutils"
)
Expand Down Expand Up @@ -86,3 +88,44 @@ func Test_referenceGraphTarget_Mount(t *testing.T) {
target := graphTarget{GraphTarget: &remote.Repository{}}
_ = target.Mount(context.Background(), ocispec.Descriptor{}, "", nil)
}

func Test_graphTarget_Push_alreadyExists(t *testing.T) {
// prepare
pty, device, err := testutils.NewPty()
if err != nil {
t.Fatal(err)
}
defer device.Close()
src := memory.New()
content := []byte("test")
r := bytes.NewReader(content)
desc := ocispec.Descriptor{
MediaType: "application/octet-stream",
Digest: digest.FromBytes(content),
Size: int64(len(content)),
}
if err := src.Push(context.Background(), desc, r); err != nil {
t.Fatal("Failed to prepare test environment:", err)
}
// test
actionPrompt := "action"
donePrompt := "done"
target, err := NewTarget(src, actionPrompt, donePrompt, device)
if err != nil {
t.Fatal(err)
}
if gt, ok := target.(*graphTarget); ok {
if err := gt.Push(context.Background(), desc, r); !errors.Is(err, errdef.ErrAlreadyExists) {
t.Fatal("Expected ErrAlreadyExists, got:", err)
}
if err := gt.manager.Close(); err != nil {
t.Fatal(err)
}
} else {
t.Fatal("not testing based on a referenceGraphTarget")
}
// validate
if err = testutils.MatchPty(pty, device, donePrompt, desc.MediaType, "100.00%", desc.Digest.String()); err != nil {
t.Fatal(err)
}
}

0 comments on commit b4be9a6

Please sign in to comment.