Artifact c8423602616f9d7e49178222ae305b8f5a1cab45e8676fa95a66f36001099d2e:
- File
internal/deck/marshal.go
— part of check-in
[5036cae392]
at
2026-07-19 22:27:35
on branch trunk
— Enforce card occurrence classes and intra-run ordering (#94)
* Enforce card occurrence classes and intra-run ordering
The deck parser enforced card-type ordering and nothing else. Each of the 14 single-occurrence card types was handled by a case that assigned, so a second occurrence silently overwrote the first, and the repeatable types were appended with no sort check at all. draft-fossil-artifact-format-00 §4.5.1 and §4.5.2 define both rules; implement them.
Duplicate rejection covers A, B, C, D, E, G, H, I, K, L, N, R, U and W. The guard is separate from the card-type check because that check permits equal consecutive letters. F, J and M gain a strictly-ascending check against the preceding entry, T the two-level (name, target hash) rule with the full sign-prefixed name as primary key. P and Q deliberately get neither: P order is semantic — primary parent first — and Q is unordered-repeatable. P also now appends across cards instead of replacing, which is what §4.5.1 means by repeatable. Z belongs to neither class, so it is accepted with no duplicate or last-card rule; the authoritative checksum is the trailing 35 bytes VerifyZ already verified.
All four ordering checks route through one comparator, deck.Compare, which is §4.5.3: unsigned octets, locale-independent, no normalization, no case folding. §4.5.3 exists because the same ordering underlies the delta-manifest merge walk and baseline binary search of §8.2, so a second copy would resolve a delta manifest wrongly rather than merely fail validation. The F check runs on the decoded filename, per §4.5.2.
Malformed input is reported through the error return, never a panic -- these bytes arrive from peers.
Two writer-side defects surfaced, both of which let this package emit artifacts its own parser now rejects. Marshal emitted J cards in caller-supplied order while already sorting F, M and T; it now sorts them. Marshal also sorted T by the concatenation name+target, which lets the target's leading hex digit compete with the tail of a longer name and reorders tags whose names are prefixes of one another; it now uses the two-level key. The F and M sorts and the R-card file enumeration route through deck.Compare as well, the latter per §6.3.
Closes #70
* Order T cards by decoded name and type the Z payload
Review found two ways the first pass diverged from canonical fossil in the accept/reject direction.
The T ordering key used the raw wire name. §4.7.16 says the name is escape-decoded and that the primary ordering key is the full name token including its leading sign character; manifest.c defossilizes the name before the ordering compare. Because "\s" begins 0x5C and a space is 0x20, the two forms order oppositely, so a run like
T *sym-a\sb aaaa... T *sym-a-c bbbb...
is valid, fossil accepts it, and we rejected it. Branch names may contain spaces, so this is an ordinary tag, not a corner case -- the same post-decode trap already handled for F. tagOrderKey now builds the decoded key and both the parser check and the marshaller's sort call it, so the two cannot drift apart.
The Z card accepted any payload. Accepting a Z line inside the body is right -- §4.5.1 puts Z in neither occurrence class and treats an earlier Z as content covered by the final checksum -- but §4.7.19 still fixes the payload at exactly 32 hexadecimal characters. Validating nothing left an arbitrary-byte channel in an otherwise fully typed grammar and would have stored and forwarded artifacts canonical refuses. Upper-case digits are accepted, matching fossil's validate16.
Also: the T sign prefix is now checked against {+,-,*} per §4.7.16, both because canonical checks it and because an unvalidated sign byte >= 0x80 would UTF-8-expand to two bytes when the ordering key is assembled and silently corrupt the comparison. Two stale §4.7.20 citations corrected to §4.7.16, and the two-level T test gained the prefix-name case that actually distinguishes a two-level comparison from a concatenated one.
* Cite the normative grammar for the case-insensitive Z token
The Z payload's case-insensitivity was justified by inferring it from the word "hexadecimal" in §4.7.19. §6.1 states it normatively: md5-token = 32hexdig-ci, a parse-accept grammar admitting both 0-9a-f and A-F. Cite that instead, and record the split it implies -- the structural check is case-insensitive while §6.2 keeps verification byte-exact against the lowercase form.
Also soften deck.Compare's claim that the delta-manifest merge walk relies on F ordering to the future tense parse.go already uses: the merge walk is a map overlay today with no comparator to share. (user: [email protected], size: 4317) [annotate] [blame] [check-ins using]
package deck import ( "fmt" "sort" "strings" ) func (d *Deck) Marshal() ([]byte, error) { if d == nil { panic("deck.Marshal: d must not be nil") } var b strings.Builder marshalCards(&b, d) body := b.String() zHash := computeZ([]byte(body)) return []byte(fmt.Sprintf("%sZ %s\n", body, zHash)), nil } // marshalCards writes all cards in ASCII order to the builder. func marshalCards(b *strings.Builder, d *Deck) { if b == nil { panic("deck.marshalCards: b must not be nil") } if d == nil { panic("deck.marshalCards: d must not be nil") } // Cards in strict ASCII order: A B C D E F G H I J K L M N P Q R T U W Z if d.A != nil { b.WriteString("A ") b.WriteString(FossilEncode(d.A.Filename)) b.WriteString(" ") b.WriteString(d.A.Target) if d.A.Source != "" { b.WriteString(" ") b.WriteString(d.A.Source) } b.WriteString("\n") } if d.B != "" { fmt.Fprintf(b, "B %s\n", d.B) } if d.C != "" { fmt.Fprintf(b, "C %s\n", FossilEncode(d.C)) } if !d.D.IsZero() { fmt.Fprintf(b, "D %s\n", d.D.UTC().Format("2006-01-02T15:04:05.000")) } if d.E != nil { fmt.Fprintf(b, "E %s %s\n", d.E.Date.UTC().Format("2006-01-02T15:04:05"), d.E.UUID) } if len(d.F) > 0 { sorted := make([]FileCard, len(d.F)) copy(sorted, d.F) sort.Slice(sorted, func(i, j int) bool { return Compare(sorted[i].Name, sorted[j].Name) < 0 }) for _, f := range sorted { b.WriteString("F ") b.WriteString(FossilEncode(f.Name)) if f.UUID != "" { b.WriteString(" ") b.WriteString(f.UUID) if f.Perm != "" { b.WriteString(" ") b.WriteString(f.Perm) } } if f.OldName != "" { b.WriteString(" ") b.WriteString(FossilEncode(f.OldName)) } b.WriteString("\n") } } if d.G != "" { fmt.Fprintf(b, "G %s\n", d.G) } if d.H != "" { fmt.Fprintf(b, "H %s\n", FossilEncode(d.H)) } if d.I != "" { fmt.Fprintf(b, "I %s\n", d.I) } if len(d.J) > 0 { // §4.5.2 requires a strictly ascending J run, which the parser // enforces. Callers build TicketField slices in whatever order // suits them, so wire order is the marshaller's to impose — the // same responsibility it already takes for F, M and T. sorted := make([]TicketField, len(d.J)) copy(sorted, d.J) sort.Slice(sorted, func(i, j int) bool { return Compare(sorted[i].Name, sorted[j].Name) < 0 }) for _, j := range sorted { if j.Value != "" { fmt.Fprintf(b, "J %s %s\n", FossilEncode(j.Name), j.Value) } else { fmt.Fprintf(b, "J %s\n", FossilEncode(j.Name)) } } } if d.K != "" { fmt.Fprintf(b, "K %s\n", d.K) } if d.L != "" { fmt.Fprintf(b, "L %s\n", FossilEncode(d.L)) } if len(d.M) > 0 { sorted := make([]string, len(d.M)) copy(sorted, d.M) sort.Slice(sorted, func(i, j int) bool { return Compare(sorted[i], sorted[j]) < 0 }) for _, m := range sorted { fmt.Fprintf(b, "M %s\n", m) } } if d.N != "" { fmt.Fprintf(b, "N %s\n", d.N) } if len(d.P) > 0 { fmt.Fprintf(b, "P %s\n", strings.Join(d.P, " ")) } for _, q := range d.Q { prefix := "+" if q.IsBackout { prefix = "-" } if q.Baseline != "" { fmt.Fprintf(b, "Q %s%s %s\n", prefix, q.Target, q.Baseline) } else { fmt.Fprintf(b, "Q %s%s\n", prefix, q.Target) } } if d.R != "" { fmt.Fprintf(b, "R %s\n", d.R) } if len(d.T) > 0 { sorted := make([]TagCard, len(d.T)) copy(sorted, d.T) // Two-level (decoded name, target hash) per §4.5.2 and §4.7.16, // not a comparison of the concatenated key: concatenating lets the // target's leading characters compete with the tail of a longer // name, which reorders tags whose names are prefixes of one // another into a run the parser rejects. tagOrderKey is the same // key the parser's check uses, so the two cannot drift. sort.Slice(sorted, func(i, j int) bool { if cmp := Compare(tagOrderKey(sorted[i]), tagOrderKey(sorted[j])); cmp != 0 { return cmp < 0 } return Compare(sorted[i].UUID, sorted[j].UUID) < 0 }) for _, tag := range sorted { fmt.Fprintf(b, "T %c%s %s", tag.Type, tag.Name, tag.UUID) if tag.Value != "" { fmt.Fprintf(b, " %s", tag.Value) } b.WriteString("\n") } } if d.U != nil && *d.U != "" { fmt.Fprintf(b, "U %s\n", FossilEncode(*d.U)) } if len(d.W) > 0 { fmt.Fprintf(b, "W %d\n%s\n", len(d.W), d.W) } }