diff --git a/x/model/digest_test.go b/x/model/digest_test.go index b6a25946..78f9a837 100644 --- a/x/model/digest_test.go +++ b/x/model/digest_test.go @@ -49,5 +49,10 @@ func TestDigestString(t *testing.T) { if got != want { t.Errorf("ParseDigest(%q).String() = %q; want %q", s, got, want) } + + got = ParseDigest(s).String() + if got != want { + t.Errorf("roundtrip ParseDigest(%q).String() = %q; want %q", s, got, want) + } } } diff --git a/x/model/name.go b/x/model/name.go index 8070f49d..cc833665 100644 --- a/x/model/name.go +++ b/x/model/name.go @@ -44,18 +44,16 @@ const ( // // It should be kept as the last part in the list. PartInvalid - - NumParts = PartInvalid ) var kindNames = map[NamePart]string{ - PartInvalid: "Invalid", PartHost: "Host", PartNamespace: "Namespace", PartModel: "Name", PartTag: "Tag", PartBuild: "Build", PartDigest: "Digest", + PartInvalid: "Invalid", } func (k NamePart) String() string { @@ -93,8 +91,9 @@ func (k NamePart) String() string { // // To make a Name by filling in missing parts from another Name, use [Fill]. type Name struct { - _ structs.Incomparable - parts [NumParts]string + _ structs.Incomparable + parts [5]string // host, namespace, model, tag, build + digest Digest // digest is a special part // TODO(bmizerany): track offsets and hold s (raw string) here? We // could pack the offests all into a single uint64 since the first @@ -141,6 +140,13 @@ func ParseName(s string) Name { if kind == PartInvalid { return Name{} } + if kind == PartDigest { + r.digest = ParseDigest(part) + if !r.digest.Valid() { + return Name{} + } + continue + } r.parts[kind] = part } if r.Valid() || r.Resolved() { @@ -233,8 +239,7 @@ var seps = [...]string{ PartNamespace: "/", PartModel: ":", PartTag: "+", - PartBuild: "@", - PartDigest: "", + PartBuild: "", } // WriteTo implements io.WriterTo. It writes the fullest possible display @@ -247,25 +252,22 @@ var seps = [...]string{ // The full digest is always prefixed with "@". That is if [Name.Valid] // reports false and [Name.Resolved] reports true, then the string is // returned as "@-". -func (r Name) WriteTo(w io.Writer) (n int64, err error) { +func (r Name) writeTo(w io.StringWriter) { + var partsWritten int for i := range r.parts { if r.parts[i] == "" { continue } - if n > 0 || NamePart(i) == PartDigest { - n1, err := io.WriteString(w, seps[i-1]) - n += int64(n1) - if err != nil { - return n, err - } - } - n1, err := io.WriteString(w, r.parts[i]) - n += int64(n1) - if err != nil { - return n, err + if partsWritten > 0 { + w.WriteString(seps[i-1]) } + w.WriteString(r.parts[i]) + partsWritten++ + } + if r.Resolved() { + w.WriteString("@") + w.WriteString(r.digest.String()) } - return n, nil } var builderPool = sync.Pool{ @@ -287,7 +289,7 @@ func (r Name) String() string { defer builderPool.Put(b) b.Reset() b.Grow(50) // arbitrarily long enough for most names - _, _ = r.WriteTo(b) + r.writeTo(b) return b.String() } @@ -296,11 +298,13 @@ func (r Name) String() string { // returns a string that includes all parts of the Name, with missing parts // replaced with a ("?"). func (r Name) GoString() string { - var v Name for i := range r.parts { - v.parts[i] = cmp.Or(r.parts[i], "?") + r.parts[i] = cmp.Or(r.parts[i], "?") } - return v.String() + if !r.Resolved() { + r.digest = Digest{"?", "?"} + } + return r.String() } // LogValue implements slog.Valuer. @@ -320,10 +324,7 @@ func (r Name) MarshalText() ([]byte, error) { b.Reset() b.Grow(50) // arbitrarily long enough for most names defer bufPool.Put(b) - _, err := r.WriteTo(b) - if err != nil { - return nil, err - } + r.writeTo(b) // TODO: We can remove this alloc if/when // https://github.com/golang/go/issues/62384 lands. return b.Bytes(), nil @@ -393,15 +394,15 @@ func (r Name) CompleteNoBuild() bool { // It is possible to have a valid Name, or a complete Name that is not // resolved. func (r Name) Resolved() bool { - return r.parts[PartDigest] != "" + return r.digest.Valid() } // Digest returns the digest part of the Name, if any. // // If Digest returns a non-empty string, then [Name.Resolved] will return // true, and digest is considered valid. -func (r Name) Digest() string { - return r.parts[PartDigest] +func (r Name) Digest() Digest { + return r.digest } // EqualFold reports whether r and o are equivalent model names, ignoring @@ -479,24 +480,14 @@ func Parts(s string) iter.Seq2[NamePart, string] { case '@': switch state { case PartDigest: - part := s[i+1:] - if isValidDigest(part) { - if !yield(PartDigest, part) { - return - } - if i == 0 { - // The name is in - // the form of - // "@digest". This - // is valid ans so - // we want to skip - // the final - // validation for - // any other state. - return - } - } else { - yield(PartInvalid, "") + if !yieldValid(PartDigest, s[i+1:j]) { + return + } + if i == 0 { + // This is the form + // "@" which is valid. + // + // We're done. return } state, j, partLen = PartBuild, i, 0 diff --git a/x/model/name_test.go b/x/model/name_test.go index 54e0dcc5..6b917b04 100644 --- a/x/model/name_test.go +++ b/x/model/name_test.go @@ -23,7 +23,7 @@ func fieldsFromName(p Name) fields { model: p.parts[PartModel], tag: p.parts[PartTag], build: p.parts[PartBuild], - digest: p.parts[PartDigest], + digest: p.digest.String(), } } @@ -101,8 +101,8 @@ var testNames = map[string]fields{ func TestNameParts(t *testing.T) { var p Name - if len(p.Parts()) != int(NumParts) { - t.Errorf("Parts() = %d; want %d", len(p.Parts()), NumParts) + if w, g := int(PartBuild+1), len(p.Parts()); w != g { + t.Errorf("Parts() = %d; want %d", g, w) } } @@ -137,7 +137,7 @@ func TestParseName(t *testing.T) { // test round-trip if !ParseName(name.String()).EqualFold(name) { - t.Errorf("String() = %s; want %s", name.String(), baseName) + t.Errorf("ParseName(%q).String() = %s; want %s", s, name.String(), baseName) } if name.Valid() && name.DisplayModel() == "" { @@ -146,9 +146,9 @@ func TestParseName(t *testing.T) { t.Errorf("Valid() = false; Model() = %q; want empty name", got.model) } - if name.Resolved() && name.Digest() == "" { + if name.Resolved() && !name.Digest().Valid() { t.Errorf("Resolved() = true; Digest() = %q; want non-empty digest", got.digest) - } else if !name.Resolved() && name.Digest() != "" { + } else if !name.Resolved() && name.Digest().Valid() { t.Errorf("Resolved() = false; Digest() = %q; want empty digest", got.digest) } }) @@ -233,7 +233,7 @@ func TestNameDisplay(t *testing.T) { wantLong: "library/mistral:latest", wantComplete: "example.com/library/mistral:latest", wantModel: "mistral", - wantGoString: "example.com/library/mistral:latest+Q4_0@?", + wantGoString: "example.com/library/mistral:latest+Q4_0@?-?", }, { name: "Short Name", @@ -242,7 +242,7 @@ func TestNameDisplay(t *testing.T) { wantLong: "mistral:latest", wantComplete: "mistral:latest", wantModel: "mistral", - wantGoString: "?/?/mistral:latest+?@?", + wantGoString: "?/?/mistral:latest+?@?-?", }, { name: "Long Name", @@ -251,7 +251,7 @@ func TestNameDisplay(t *testing.T) { wantLong: "library/mistral:latest", wantComplete: "library/mistral:latest", wantModel: "mistral", - wantGoString: "?/library/mistral:latest+?@?", + wantGoString: "?/library/mistral:latest+?@?-?", }, { name: "Case Preserved", @@ -260,7 +260,7 @@ func TestNameDisplay(t *testing.T) { wantLong: "Library/Mistral:Latest", wantComplete: "Library/Mistral:Latest", wantModel: "Mistral", - wantGoString: "?/Library/Mistral:Latest+?@?", + wantGoString: "?/Library/Mistral:Latest+?@?-?", }, { name: "With digest",