From c4ec80ff0c65609c769df26d4aed2858a987f9a2 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 1 Sep 2026 09:44:50 +0200 Subject: [PATCH] format: two archives, one vocabulary, and a guard that keeps it that way ZIP and TAR.GZ each carried their own copy of what an archive holds and how to read it. Measured across the two packages before the move: six constants with identical values, two identical property readers, an identical mustSize, the entry ceiling refusal written out twice down to its wording, and a groupsFor whose code was identical to the byte. Every difference between the two copies was a comment, and one of those comments pointed at the other copy. That comment was the whole mechanism holding them together. 7Z is already named and measured in docs/MVP-FORMATS.md section 2.5 and M1 lists RAR and ISO behind it, so the copy was going to be taken a third time and a fourth. It now lives in internal/format/archive, the third family package after imagelabel for the pictures and opc for the Office formats. A container names the axes it takes - archive.Axes(archive.Entries, ...) - rather than receiving all of them. That is not tidiness: a window sends every setting it draws, so a format must only ever declare a setting it can actually carry. Nothing a user sees moves. Same keys, same defaults, same refusals, and the pinned generator hashes are unchanged - TestOurOwnGeneratorsHaveNotDrifted is the proof rather than the claim, since D11 only allows this refactor to be invisible. What was deliberately NOT merged is the padding ceiling. Both formats cap it at 65 535 B and the agreement is a coincidence: in ZIP it is the width of the two byte length field, in TAR.GZ it is the measured point where 7-Zip stops reading a gzip comment. One number, two independent facts, and merging them would have left the next container inheriting a limit with no reason attached. Sharing is for what is the same fact, not for what is the same number. Three things came out of the work that were not planned: A guard was written and deleted the same day, and the deletion is worth more than the guard. It said a format which is not a container may not use these names at all. Run against the registry it went red at once: log declares its own entry_format and means the shape of a line by it. That is not a collision - each field is drawn from its own format's Detail and SettingLabel only spaces and capitalises the key - it is proof the premise was false. entries and entry_size are just as reasonable for a log, and "how many entries" is already a named deferred log setting, so the guard would have gone red on a format doing nothing wrong. A defence that reddens on the legitimate case is worse than none. The naming hazard went to the glossary instead. TestTheArchiveEntryFormatSortsBeforeTheArchive was scraping defaultEntryFmt out of zip.go as text, because the constant was unexported and the guard lives outside that package. It went red on the move, which is the good failure. The constant is exported from archive now, so it asks the constant instead and there is nothing left to go stale. TestEveryLogShapeWritesWholeLinesAtTheRightSize shipped unproven with the log work and now has a mutation: the default line terminator becomes a space, one byte either way, so every size and every exit code stays right and only this guard can see it. CI could not have caught that gap, because mutate.py lives in tools/ which is outside the repository, so the mutation coverage guard skips on a fresh clone. The depth crowding ceiling comes down from 54 to 52, which is the ratchet reporting that two duplicated functions stopped existing. Co-Authored-By: Claude Opus 5 --- internal/format/archive/archive.go | 283 +++++++++++++++++++++++++++ internal/format/targz/targz.go | 153 +-------------- internal/format/zip/properties.go | 44 ----- internal/format/zip/zip.go | 146 +------------- internal/guard/archiveaxes_test.go | 94 +++++++++ internal/guard/branching_test.go | 2 +- internal/guard/layers_test.go | 6 +- internal/guard/registrations_test.go | 39 ++-- 8 files changed, 416 insertions(+), 351 deletions(-) create mode 100644 internal/format/archive/archive.go delete mode 100644 internal/format/zip/properties.go create mode 100644 internal/guard/archiveaxes_test.go diff --git a/internal/format/archive/archive.go b/internal/format/archive/archive.go new file mode 100644 index 0000000..aebdb19 --- /dev/null +++ b/internal/format/archive/archive.go @@ -0,0 +1,283 @@ +// Package archive is what a container format takes and how it reads it. +// +// ZIP and TAR.GZ are different files with one vocabulary. Both hold real +// generated files of other formats, both take how many either as a contains +// list or as the entries, entry_format and entry_size properties, and both +// refuse the same things for the same reasons. +// +// Until 2026-09-01 they said all of that twice. Measured that day across the +// two packages: six constants with identical values, two identical reader +// helpers, an identical mustSize, the entry ceiling refusal written out twice +// down to its wording, and a groupsFor whose code was identical to the byte - +// every difference between the two copies was a comment, and one of those +// comments pointed at the other copy. Nothing but that comment held them +// together. +// +// A third container is already named and measured (7Z, docs/MVP-FORMATS.md +// section 2.5) and M1 lists more behind it, so the copy was going to be taken a +// third time and a fourth. +// +// This is the third family package, after imagelabel for the pictures and opc +// for the Office formats, and it draws its line where they draw theirs: what +// every container shares lives here, what one of them measured for itself stays +// with it. The padding channel is the example worth naming. Both formats cap it +// at 65 535 bytes and that agreement is a coincidence - in ZIP it is the width +// of the field carrying the length, in TAR.GZ it is where 7-Zip stops reading a +// gzip comment. Merging them would put one number where there are two facts. +package archive + +import ( + "fmt" + "sort" + "strconv" + + "github.com/donislawdev/TestingFilesGenerator/internal/core" + "github.com/donislawdev/TestingFilesGenerator/internal/format" +) + +// The keys a recipe writes. Public names under rule 10, spelled once here +// rather than quoted at each place that reads one. +const ( + Entries = "entries" + EntryFormat = "entry_format" + EntrySize = "entry_size" +) + +const ( + // DefaultFormat is what an archive holds when nothing says otherwise. It + // is exported because a container works its own minimum size out by asking + // this format for an empty file. + DefaultFormat = "txt" + + defaultEntries = 1 + + // maxEntries is the ceiling on how many files an archive holds. It bounds + // the declaration below and the reading of both doors into it, so the two + // cannot drift - which they did, in two packages, until 2026-08-26. + maxEntries = 10000 + + // defaultSizeText is the default size of a file inside, written the way + // somebody writes it. The number is derived from it rather than spelled a + // second time. + // + // They used to be two constants and they had drifted: the declaration said + // 8kb and the generator used 4096, so tfg formats printed one answer and + // generating without the setting gave the other. Nothing could see it, + // because the declaration is only read for printing. The declaration is + // the half consumers believe - AR9 makes the registry the place a consumer + // asks - so the generator was moved to it rather than the other way round. + defaultSizeText = "8kb" +) + +// defaultSize is defaultSizeText in bytes. Package variables are initialised +// before any init runs, so a registration can rely on it. +var defaultSize = mustSize(defaultSizeText) + +func mustSize(s string) int64 { + n, err := core.ParseSize(s) + if err != nil { + panic(fmt.Sprintf("archive: the default entry size %q is not a size this build can parse: %v", s, err)) + } + return n +} + +// axes is the declaration of every setting a container may take, by key. +// +// One copy, so two containers cannot offer the same setting with different +// bounds, a different default or a different sentence beside it. Which is not +// hypothetical tidiness: the two that exist today were identical only because +// somebody kept them so by hand, and the comment saying so was the whole +// mechanism. +var axes = map[string]format.Property{ + Entries: { + Name: Entries, Kind: format.PropertyInt, + Min: 0, Max: maxEntries, + Default: strconv.Itoa(defaultEntries), + Detail: "How many files the archive holds. Use contains instead when the files are not all alike.", + }, + EntryFormat: { + Name: EntryFormat, Kind: format.PropertyText, + Shape: "the id of a format, as tfg formats lists them", + // Not a choice, because the allowed values are whatever this build + // registered, and a list frozen here would drift away from the + // registry the moment a format is added. + Default: DefaultFormat, + Detail: "The format of the files inside. Run tfg formats to see what this build supports.", + }, + EntrySize: { + Name: EntrySize, Kind: format.PropertySize, + Default: defaultSizeText, + Detail: "How big each file inside is.", + }, +} + +// Names is every container setting this build declares, in a stable order. +// +// It exists for the guard that compares what a container offers against what +// this package says the setting is. Given the list, that guard covers an axis +// added tomorrow without a line being changed in it - which is the same reason +// the entries ceiling is read out of the registry rather than repeated in a +// test. +func Names() []string { + out := make([]string, 0, len(axes)) + for n := range axes { + out = append(out, n) + } + sort.Strings(out) + return out +} + +// Axes is the declarations for the settings named, in the order given. +// +// A container lists what it takes rather than receiving all of it, because the +// settings a container understands are not the same set for every container - +// tar carries an owner and a mode where zip carries neither. What a format does +// not name here it does not declare, so a window never draws a field for it and +// a recipe naming it is a typo the registry refuses. +// +// An unknown name panics rather than being skipped. This is called from init, +// the caller is a programmer and not a user, and a silently dropped axis is a +// setting that vanishes from both surfaces with nothing said. +func Axes(names ...string) []format.Property { + out := make([]format.Property, 0, len(names)) + for _, n := range names { + p, ok := axes[n] + if !ok { + panic(fmt.Sprintf("archive: %q is not a container setting this build declares", n)) + } + out = append(out, p) + } + return out +} + +// Groups works out what the archive holds. +// +// There are two ways to say it. "contains" in a recipe is the general one and +// takes a list of groups of different formats. The entries, entry_format and +// entry_size properties are the flag sized one, reachable through --set, and +// they say the same thing for a single format. +// +// Both at once is refused rather than one of them being picked. Picking would +// produce an archive holding something other than what the recipe says, and the +// recipe is the thing somebody reads in a pull request. Same rule as a boundary +// declared beside a size. +// +// id is the container asking, and it is the whole of what used to differ +// between the two copies of this: it names the format in every refusal and it +// is what an archive may not hold, since an archive inside an archive needs a +// depth limit that does not exist yet. +func Groups(id string, r format.Request) ([]format.Content, error) { + var stated []string + for _, key := range []string{Entries, EntryFormat, EntrySize} { + if _, ok := r.Properties[key]; ok { + stated = append(stated, key) + } + } + + // Not len() > 0. An empty contains says "an archive holding nothing", + // which is a legitimate thing to ask for, and it is a different statement + // from saying nothing at all. + if r.Contains != nil { + return fromContains(id, r, stated) + } + + if r.SizeFromContents { + // Only reachable if a caller sets the flag without contents. Saying so + // beats producing an empty archive and calling it the answer. + return nil, fmt.Errorf("%s: the size was left to the contents and there are none", id) + } + + entries, err := intProperty(id, r.Properties, Entries, defaultEntries, 0, maxEntries) + if err != nil { + return nil, err + } + // Sizes in properties use the same syntax as --size. Anything else would + // mean entry_size=200kb failing while size=200kb works, which nobody would + // predict. + entrySize, err := sizeProperty(id, r.Properties, EntrySize, defaultSize) + if err != nil { + return nil, err + } + entryFmt := DefaultFormat + if v, ok := r.Properties[EntryFormat]; ok && v != "" { + entryFmt = v + } + if entryFmt == id { + return nil, &format.NestingUnsupportedError{Format: id} + } + return []format.Content{{Format: entryFmt, Count: entries, Bytes: entrySize}}, nil +} + +// fromContains reads the general door, the one a recipe writes. +// +// Split out of Groups so the branching stays under the ceiling the shape gates +// hold, and because it is one subject: everything here is about a contains list +// and nothing about it reads a property. +func fromContains(id string, r format.Request, stated []string) ([]format.Content, error) { + if len(stated) > 0 { + return nil, &format.ContentsConflictError{Format: id, Keys: stated} + } + asked := 0 + for _, g := range r.Contains { + if g.Format == id { + return nil, &format.NestingUnsupportedError{Format: id} + } + asked += g.Count + } + // The same ceiling the entries property has, on the other way of saying the + // same thing. Until 2026-08-26 this path had none: a recipe asking for + // fifty thousand entries through contains validated clean and dry ran + // clean, while entries=50000 was refused with "must be between 0 and + // 10000". One quantity, two doors, two answers - and the door with no + // ceiling planned a format.Plan for every child. + if asked > maxEntries { + // Spelled as the refusal the entries property already produces, down + // to the exit code: a plain error here would have landed on 1 while + // the same request through entries lands on 4, which is the same + // disagreement one level further down. + return nil, tooMany(id, "contains", asked) + } + return r.Contains, nil +} + +// tooMany is the refusal both doors give, written once. +// +// TestBothWaysOfAskingForEntriesShareOneCeiling compares the reason the two +// produce, and before this it compared two sentences somebody had typed twice. +func tooMany(id, key string, asked int) *format.PropertyValueError { + return &format.PropertyValueError{ + Format: id, + Key: key, + Value: strconv.Itoa(asked), + Reason: fmt.Sprintf("it takes a whole number from 0 to %d", maxEntries), + Remedy: fmt.Sprintf("Ask for %d entries or fewer.", maxEntries), + } +} + +// sizeProperty reads a byte count written the way --size accepts it. +func sizeProperty(id string, props map[string]string, key string, fallback int64) (int64, error) { + raw, ok := props[key] + if !ok || raw == "" { + return fallback, nil + } + n, err := core.ParseSize(raw) + if err != nil { + return 0, fmt.Errorf("%s: %s: %w", id, key, err) + } + return n, nil +} + +func intProperty(id string, props map[string]string, key string, fallback, min, max int) (int, error) { + raw, ok := props[key] + if !ok || raw == "" { + return fallback, nil + } + n, err := strconv.Atoi(raw) + if err != nil { + return 0, fmt.Errorf("%s: %s must be a whole number, got %q", id, key, raw) + } + if n < min || n > max { + return 0, fmt.Errorf("%s: %s must be between %d and %d, got %d", id, key, min, max, n) + } + return n, nil +} diff --git a/internal/format/targz/targz.go b/internal/format/targz/targz.go index d9682c7..63e5227 100644 --- a/internal/format/targz/targz.go +++ b/internal/format/targz/targz.go @@ -10,12 +10,12 @@ import ( "context" "fmt" "io" - "strconv" "strings" "time" "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/format" + "github.com/donislawdev/TestingFilesGenerator/internal/format/archive" ) const ( @@ -74,36 +74,9 @@ const ( // storeBlockCost is what each stored block costs on top of its content. storeBlockCost = 5 - defaultEntries = 1 - defaultEntryFmt = "txt" - maxEntries = 10000 - - // defaultEntrySizeText is the default size of a file inside, written the - // way somebody would write it. The number below is derived from it rather - // than written a second time, because the declaration is what tfg formats - // prints and there is no other mechanism making a printed default agree - // with the one the code uses. - // - // The same value as ZIP on purpose. Two containers taking a setting of the - // same name and defaulting it differently is a difference nobody would - // predict and nothing would explain. - defaultEntrySizeText = "8kb" - writeChunk = 32 * 1024 ) -// defaultEntrySize is defaultEntrySizeText in bytes. Package variables are -// initialised before init runs, so the registration below can rely on it. -var defaultEntrySize = mustSize(defaultEntrySizeText) - -func mustSize(s string) int64 { - n, err := core.ParseSize(s) - if err != nil { - panic(fmt.Sprintf("targz: the default entry size %q is not a size this build can parse: %v", s, err)) - } - return n -} - // A fixed timestamp on every entry. Taking one from the clock would make two // runs of the same recipe differ, and relying on the zero value would be an // unstated dependency on what the library does with it. @@ -128,28 +101,10 @@ func init() { }, Label: format.LabelInternal, Oracle: "7z", - Properties: []format.Property{ - { - Name: "entries", Kind: format.PropertyInt, - Min: 0, Max: maxEntries, - Default: strconv.Itoa(defaultEntries), - Detail: "How many files the archive holds. Use contains instead when the files are not all alike.", - }, - { - Name: "entry_format", Kind: format.PropertyText, - Shape: "the id of a format, as tfg formats lists them", - // Not a choice, for the same reason as in ZIP: the allowed - // values are whatever this build registered, and a list frozen - // here would drift the moment a format is added. - Default: defaultEntryFmt, - Detail: "The format of the files inside. Run tfg formats to see what this build supports.", - }, - { - Name: "entry_size", Kind: format.PropertySize, - Default: defaultEntrySizeText, - Detail: "How big each file inside is.", - }, - }, + // The settings every container shares, declared once in the archive + // package. Listed rather than received whole, so a format takes only + // the axes it can actually carry. + Properties: archive.Axes(archive.Entries, archive.EntryFormat, archive.EntrySize), Container: true, GeneratorVersion: generatorVersion, Generator: generator{}, @@ -172,76 +127,8 @@ type memo struct { seed uint64 } -// groupsFor works out what the archive holds. -// -// Two ways to say it, exactly as in ZIP. "contains" in a recipe is the general -// one and takes groups of different formats. The entries, entry_format and -// entry_size properties are the flag sized one, reachable through --set. -// -// Both at once is refused rather than one of them being picked, because -// picking would build an archive holding something other than what the recipe -// says, and the recipe is what somebody reads in a pull request. -func groupsFor(r format.Request) ([]format.Content, error) { - var stated []string - for _, key := range []string{"entries", "entry_format", "entry_size"} { - if _, ok := r.Properties[key]; ok { - stated = append(stated, key) - } - } - - // Not len() > 0. An empty contains says "an archive holding nothing", which - // is a legitimate request and a different statement from saying nothing. - if r.Contains != nil { - if len(stated) > 0 { - return nil, &format.ContentsConflictError{Format: "targz", Keys: stated} - } - asked := 0 - for _, g := range r.Contains { - if g.Format == "targz" { - return nil, &format.NestingUnsupportedError{Format: "targz"} - } - asked += g.Count - } - // The ceiling the entries property has, applied to the other way of - // asking for the same thing. See the note beside the same check in - // the zip package - both doors were measured saying different things - // about fifty thousand entries on 2026-08-26. - if asked > maxEntries { - return nil, &format.PropertyValueError{ - Format: "targz", - Key: "contains", - Value: strconv.Itoa(asked), - Reason: fmt.Sprintf("it takes a whole number from 0 to %d", maxEntries), - Remedy: fmt.Sprintf("Ask for %d entries or fewer.", maxEntries), - } - } - return r.Contains, nil - } - - if r.SizeFromContents { - return nil, fmt.Errorf("targz: the size was left to the contents and there are none") - } - - entries, err := intProperty(r.Properties, "entries", defaultEntries, 0, maxEntries) - if err != nil { - return nil, err - } - entrySize, err := sizeProperty(r.Properties, "entry_size", defaultEntrySize) - if err != nil { - return nil, err - } - entryFmt := defaultEntryFmt - if v, ok := r.Properties["entry_format"]; ok && v != "" { - entryFmt = v - } - if entryFmt == "targz" { - return nil, &format.NestingUnsupportedError{Format: "targz"} - } - return []format.Content{{Format: entryFmt, Count: entries, Bytes: entrySize}}, nil -} - func (generator) Plan(r format.Request) (format.Plan, error) { - groups, err := groupsFor(r) + groups, err := archive.Groups("targz", r) if err != nil { return format.Plan{}, err } @@ -396,34 +283,6 @@ func (generator) Write(ctx context.Context, w io.Writer, p format.Plan) error { return build(ctx, w, m) } -// sizeProperty reads a byte count written the way --size accepts it. -func sizeProperty(props map[string]string, key string, fallback int64) (int64, error) { - raw, ok := props[key] - if !ok || raw == "" { - return fallback, nil - } - n, err := core.ParseSize(raw) - if err != nil { - return 0, fmt.Errorf("targz: %s: %w", key, err) - } - return n, nil -} - -func intProperty(props map[string]string, key string, fallback, min, max int) (int, error) { - raw, ok := props[key] - if !ok || raw == "" { - return fallback, nil - } - n, err := strconv.Atoi(raw) - if err != nil { - return 0, fmt.Errorf("targz: %s must be a whole number, got %q", key, raw) - } - if n < min || n > max { - return 0, fmt.Errorf("targz: %s must be between %d and %d, got %d", key, min, max, n) - } - return n, nil -} - // minimumBytes is the structural floor of the format: an archive holding // nothing, with no label and no padding. The end of archive marker is two // empty blocks, and gzip frames that into 1052 bytes. diff --git a/internal/format/zip/properties.go b/internal/format/zip/properties.go deleted file mode 100644 index b829f84..0000000 --- a/internal/format/zip/properties.go +++ /dev/null @@ -1,44 +0,0 @@ -package zip - -import ( - "fmt" - "strconv" - - "github.com/donislawdev/TestingFilesGenerator/internal/core" -) - -// Reading the settings a request carries. -// -// Split out of zip.go on 2026-08-26, when the guard against files creeping -// towards the size ceiling went red after the contains ceiling was added. The -// split is by subject rather than by line count - these two read a declared -// property out of the request and say so in the format's own voice, and -// nothing else in the package does that. - -// sizeProperty reads a byte count written the way --size accepts it. -func sizeProperty(props map[string]string, key string, fallback int64) (int64, error) { - raw, ok := props[key] - if !ok || raw == "" { - return fallback, nil - } - n, err := core.ParseSize(raw) - if err != nil { - return 0, fmt.Errorf("zip: %s: %w", key, err) - } - return n, nil -} - -func intProperty(props map[string]string, key string, fallback, min, max int) (int, error) { - raw, ok := props[key] - if !ok || raw == "" { - return fallback, nil - } - n, err := strconv.Atoi(raw) - if err != nil { - return 0, fmt.Errorf("zip: %s must be a whole number, got %q", key, raw) - } - if n < min || n > max { - return 0, fmt.Errorf("zip: %s must be between %d and %d, got %d", key, min, max, n) - } - return n, nil -} diff --git a/internal/format/zip/zip.go b/internal/format/zip/zip.go index 463194d..307a181 100644 --- a/internal/format/zip/zip.go +++ b/internal/format/zip/zip.go @@ -10,12 +10,12 @@ import ( "context" "fmt" "io" - "strconv" "strings" "time" "github.com/donislawdev/TestingFilesGenerator/internal/core" "github.com/donislawdev/TestingFilesGenerator/internal/format" + "github.com/donislawdev/TestingFilesGenerator/internal/format/archive" ) const ( @@ -47,37 +47,9 @@ const ( // is rather than wonder. fillerName = "tfg-padding.bin" - defaultEntries = 1 - defaultEntryFmt = "txt" - maxEntries = 10000 - - // defaultEntrySizeText is the default size of a file inside, written the - // way somebody writes it. The number below is derived from it rather than - // spelled a second time. - // - // They used to be two constants and they had drifted: the declaration said - // 8kb and the generator used 4096, so tfg formats printed one answer and - // generating without the setting gave the other. Nothing could see it, - // because the declaration is only read for printing. The declaration is - // the half consumers believe - AR9 makes the registry the place a consumer - // asks - so the generator was moved to it rather than the other way round. - defaultEntrySizeText = "8kb" - writeChunk = 32 * 1024 ) -// defaultEntrySize is defaultEntrySizeText in bytes. Package variables are -// initialised before init runs, so the registration below can rely on it. -var defaultEntrySize = mustSize(defaultEntrySizeText) - -func mustSize(s string) int64 { - n, err := core.ParseSize(s) - if err != nil { - panic(fmt.Sprintf("zip: the default entry size %q is not a size this build can parse: %v", s, err)) - } - return n -} - // A fixed timestamp on every entry. Taking one from the clock would make two // runs of the same recipe differ, and relying on the zero value would be an // unstated dependency on what the library does with it. @@ -98,28 +70,10 @@ func init() { }, Label: format.LabelInternal, Oracle: "7z", - Properties: []format.Property{ - { - Name: "entries", Kind: format.PropertyInt, - Min: 0, Max: maxEntries, - Default: strconv.Itoa(defaultEntries), - Detail: "How many files the archive holds. Use contains instead when the files are not all alike.", - }, - { - Name: "entry_format", Kind: format.PropertyText, - Shape: "the id of a format, as tfg formats lists them", - // Not a choice, because the allowed values are whatever this - // build registered, and a list frozen here would drift away - // from the registry the moment a format is added. - Default: defaultEntryFmt, - Detail: "The format of the files inside. Run tfg formats to see what this build supports.", - }, - { - Name: "entry_size", Kind: format.PropertySize, - Default: defaultEntrySizeText, - Detail: "How big each file inside is.", - }, - }, + // The settings every container shares, declared once in the archive + // package. Listed rather than received whole, so a format takes only + // the axes it can actually carry. + Properties: archive.Axes(archive.Entries, archive.EntryFormat, archive.EntrySize), Container: true, GeneratorVersion: generatorVersion, Generator: generator{}, @@ -142,90 +96,8 @@ type memo struct { seed uint64 } -// groupsFor works out what the archive holds. -// -// There are two ways to say it. "contains" in a recipe is the general one and -// takes a list of groups of different formats. The entries, entry_format and -// entry_size properties are the flag sized one, reachable through --set, and -// they say the same thing for a single format. -// -// Both at once is refused rather than one of them being picked. Picking would -// produce an archive holding something other than what the recipe says, and -// the recipe is the thing somebody reads in a pull request. Same rule as a -// boundary declared beside a size. -func groupsFor(r format.Request) ([]format.Content, error) { - var stated []string - for _, key := range []string{"entries", "entry_format", "entry_size"} { - if _, ok := r.Properties[key]; ok { - stated = append(stated, key) - } - } - - // Not len() > 0. An empty contains says "an archive holding nothing", - // which is a legitimate thing to ask for, and it is a different statement - // from saying nothing at all. - if r.Contains != nil { - if len(stated) > 0 { - return nil, &format.ContentsConflictError{Format: "zip", Keys: stated} - } - asked := 0 - for _, g := range r.Contains { - if g.Format == "zip" { - return nil, &format.NestingUnsupportedError{Format: "zip"} - } - asked += g.Count - } - // The same ceiling the entries property has, on the other way of - // saying the same thing. Until 2026-08-26 this path had none: a - // recipe asking for fifty thousand entries through contains validated - // clean and dry ran clean, while entries=50000 was refused with "must - // be between 0 and 10000". One quantity, two doors, two answers - and - // the door with no ceiling planned a format.Plan for every child. - if asked > maxEntries { - // Spelled as the refusal the entries property already produces, - // down to the exit code: a plain error here would have landed on - // 1 while the same request through entries lands on 4, which is - // the same disagreement one level further down. - return nil, &format.PropertyValueError{ - Format: "zip", - Key: "contains", - Value: strconv.Itoa(asked), - Reason: fmt.Sprintf("it takes a whole number from 0 to %d", maxEntries), - Remedy: fmt.Sprintf("Ask for %d entries or fewer.", maxEntries), - } - } - return r.Contains, nil - } - - if r.SizeFromContents { - // Only reachable if a caller sets the flag without contents. Saying so - // beats producing an empty archive and calling it the answer. - return nil, fmt.Errorf("zip: the size was left to the contents and there are none") - } - - entries, err := intProperty(r.Properties, "entries", defaultEntries, 0, maxEntries) - if err != nil { - return nil, err - } - // Sizes in properties use the same syntax as --size. Anything else would - // mean entry_size=200kb failing while size=200kb works, which nobody - // would predict. - entrySize, err := sizeProperty(r.Properties, "entry_size", defaultEntrySize) - if err != nil { - return nil, err - } - entryFmt := defaultEntryFmt - if v, ok := r.Properties["entry_format"]; ok && v != "" { - entryFmt = v - } - if entryFmt == "zip" { - return nil, &format.NestingUnsupportedError{Format: "zip"} - } - return []format.Content{{Format: entryFmt, Count: entries, Bytes: entrySize}}, nil -} - func (generator) Plan(r format.Request) (format.Plan, error) { - groups, err := groupsFor(r) + groups, err := archive.Groups("zip", r) if err != nil { return format.Plan{}, err } @@ -661,13 +533,13 @@ func (c *counter) Write(p []byte) (int, error) { c.n += int64(len(p)); return le // of mistake, so this matches it: a build that cannot state its own minimum // fails at start rather than at every use. func minimumBytes() int64 { - desc, err := format.Get(defaultEntryFmt) + desc, err := format.Get(archive.DefaultFormat) if err != nil { - panic(fmt.Sprintf("zip: the default entry format %q is not registered yet, so the minimum size of an archive cannot be worked out. Check the import order in internal/format/all", defaultEntryFmt)) + panic(fmt.Sprintf("zip: the default entry format %q is not registered yet, so the minimum size of an archive cannot be worked out. Check the import order in internal/format/all", archive.DefaultFormat)) } cp, err := desc.Generator.Plan(format.Request{Bytes: 0, Seed: 0, Label: false}) if err != nil { - panic(fmt.Sprintf("zip: the default entry format %q cannot produce an empty file, so the minimum size of an archive cannot be worked out: %v", defaultEntryFmt, err)) + panic(fmt.Sprintf("zip: the default entry format %q cannot produce an empty file, so the minimum size of an archive cannot be worked out: %v", archive.DefaultFormat, err)) } n, err := archiveSize(memo{children: []child{{ name: "txt_0001.txt", desc: desc, plan: cp, diff --git a/internal/guard/archiveaxes_test.go b/internal/guard/archiveaxes_test.go new file mode 100644 index 0000000..74bba64 --- /dev/null +++ b/internal/guard/archiveaxes_test.go @@ -0,0 +1,94 @@ +package guard + +import ( + "reflect" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" + _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" + "github.com/donislawdev/TestingFilesGenerator/internal/format/archive" +) + +// Two containers offering one setting have to be offering the same setting. +// +// Until 2026-09-01 nothing said so. ZIP and TAR.GZ each wrote out the entries, +// entry_format and entry_size declarations in full, and the two copies were +// identical because somebody had kept them that way - the whole mechanism was a +// comment in the second one saying it matched the first on purpose. Measured +// that day: the two blocks were the same to the byte apart from the wording of +// one comment, and six constants behind them agreed the same way. +// +// A setting that drifts here does not fail loudly. tfg formats zip and tfg +// formats targz would simply print different defaults for a key with one name, +// a window would draw two different fields for it, and a recipe moved from one +// container to the other would behave differently with nothing said. That is +// the quiet kind, which is the kind this project writes guards for. +// +// Asked of every registered container and of every axis the archive package +// declares, so a third container and a fourth setting are both covered on the +// day they arrive rather than the day somebody remembers this file. +func TestEveryContainerDeclaresTheSharedAxesAsTheyAreDeclaredOnce(t *testing.T) { + containers := 0 + for _, d := range format.All() { + if !d.Container { + continue + } + containers++ + declared := byName(d.Properties) + + for _, axis := range archive.Names() { + p, offered := declared[axis] + if !offered { + // Not every container has to carry every axis - tar takes an + // owner and a mode that zip has nowhere to put. What is + // forbidden is carrying one and meaning something else by it. + continue + } + want := archive.Axes(axis)[0] + if !reflect.DeepEqual(p, want) { + t.Errorf("%s declares %q its own way rather than the way the archive package declares it\n"+ + " container: %+v\n"+ + " shared: %+v", + d.ID, axis, p, want) + } + } + } + + if containers == 0 { + t.Fatal("no container is registered, so this proved nothing") + } +} + +// There was a second guard here on 2026-09-01 and it is worth saying why it is +// not, because the reason is a measurement rather than a change of mind. +// +// It said that a format which is not a container may not declare any of these +// names at all, on the argument that a PNG offering entries would grow a field +// asking how many files a picture holds. Run against the registry it went red +// at once: log declares entry_format, and means the shape of a log line by it - +// apache-combined, nginx, syslog - where an archive means the format of the +// files it holds. +// +// That is not the collision it looked like. The window draws each field from +// the declaring format's own Detail, and SettingLabel only spaces and +// capitalises the key, so neither surface shows one sentence for two meanings. +// What it did show is that the guard's premise was false: "entry" is ordinary +// English and the archive package has no claim on it. entries and entry_size +// are just as reasonable for a log - "how many entries" is a log setting this +// project has already named as deferred, in docs/MVP-FORMATS.md section 5.1 - +// so the guard would have gone red the day that work started, on a format doing +// nothing wrong. +// +// A defence that reddens on the legitimate case is worse than none, so it went. +// The naming hazard it was reaching for is real and belongs in GLOSSARY.md, +// where distinctions that have cost something already live, rather than in a +// test asserting a rule the tree disproves. + +// byName is a format's declarations keyed by the name a recipe writes. +func byName(props []format.Property) map[string]format.Property { + out := make(map[string]format.Property, len(props)) + for _, p := range props { + out[p.Name] = p + } + return out +} diff --git a/internal/guard/branching_test.go b/internal/guard/branching_test.go index bd95f6f..745870c 100644 --- a/internal/guard/branching_test.go +++ b/internal/guard/branching_test.go @@ -43,7 +43,7 @@ const ( // either, and one handles neither. Flattening it in TIFF alone would make // two functions that answer the same question look different, which costs // more than the depth does. - crowdedDepthFunctions = 54 + crowdedDepthFunctions = 52 // An axis this set does not watch. crowding() asks n >= band, so nothing // reaches it. diff --git a/internal/guard/layers_test.go b/internal/guard/layers_test.go index e886358..4a28b06 100644 --- a/internal/guard/layers_test.go +++ b/internal/guard/layers_test.go @@ -36,6 +36,7 @@ var layer = map[string]int{ "internal/format": 1, "internal/format/all": 1, "internal/format/imagelabel": 1, + "internal/format/archive": 1, "internal/format/txt": 1, "internal/format/md": 1, "internal/format/logfile": 1, @@ -113,6 +114,7 @@ var sameLayerAllowed = map[string][]string{ "internal/format/wav", }, "internal/format/imagelabel": {"internal/format"}, + "internal/format/archive": {"internal/format"}, "internal/format/txt": {"internal/format", "internal/format/imagelabel"}, "internal/format/md": {"internal/format"}, "internal/format/logfile": {"internal/format"}, @@ -131,8 +133,8 @@ var sameLayerAllowed = map[string][]string{ "internal/format/jpg": {"internal/format", "internal/format/imagelabel"}, "internal/format/png": {"internal/format", "internal/format/imagelabel"}, "internal/format/pdf": {"internal/format", "internal/format/imagelabel"}, - "internal/format/zip": {"internal/format", "internal/format/imagelabel"}, - "internal/format/targz": {"internal/format"}, + "internal/format/zip": {"internal/format", "internal/format/imagelabel", "internal/format/archive"}, + "internal/format/targz": {"internal/format", "internal/format/archive"}, "internal/format/tiff": {"internal/format", "internal/format/imagelabel"}, "internal/format/webp": {"internal/format", "internal/format/imagelabel"}, "internal/format/avif": {"internal/format", "internal/format/imagelabel"}, diff --git a/internal/guard/registrations_test.go b/internal/guard/registrations_test.go index a9a5ebc..f7b8bb9 100644 --- a/internal/guard/registrations_test.go +++ b/internal/guard/registrations_test.go @@ -3,12 +3,12 @@ package guard import ( "os" "os/exec" - "path/filepath" "strings" "testing" "github.com/donislawdev/TestingFilesGenerator/internal/format" _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" + "github.com/donislawdev/TestingFilesGenerator/internal/format/archive" ) // Both binaries carry the format registrations, asked of the compiler. @@ -120,12 +120,12 @@ func TestTheArchiveEntryFormatSortsBeforeTheArchive(t *testing.T) { const module = "github.com/donislawdev/TestingFilesGenerator/internal/format/" entry := entryFormatOfZip(t) - archive := module + "zip" - if entryPath := module + entry; entryPath >= archive { + archivePath := module + "zip" + if entryPath := module + entry; entryPath >= archivePath { t.Errorf("%q does not sort before %q, so the archive can be initialised first and "+ "panic while working out its own minimum. Go initialises packages in the order of "+ "their import paths - see the specification, Program initialization", - entryPath, archive) + entryPath, archivePath) } // The entry format is really registered under that id, or the constant @@ -136,24 +136,23 @@ func TestTheArchiveEntryFormatSortsBeforeTheArchive(t *testing.T) { } } -// entryFormatOfZip reads the id the archive builds its minimum from. +// entryFormatOfZip is the id the archive builds its minimum from. // -// From the source because the constant is unexported and this guard lives -// outside its package. Naming it here as well would be the second copy of a -// value, which is the shape this project spends its time removing. +// It used to read the constant out of internal/format/zip/zip.go as text, +// because the constant was unexported and this guard lives outside that +// package. On 2026-09-01 the containers' shared vocabulary moved to +// internal/format/archive and the constant went with it, exported, and this +// went red saying it could no longer find what it was reading - which is the +// good failure, but it was reading source in the first place only because there +// was nothing to import. +// +// Now there is, so it asks the constant itself. Still one copy of the value, +// which was the point of scraping, and one that a rename moves rather than +// breaks. func entryFormatOfZip(t *testing.T) string { t.Helper() - source := readFile(t, filepath.Join(repoRoot(t), "internal", "format", "zip", "zip.go")) - const marker = "defaultEntryFmt = " - at := strings.Index(source, marker) - if at < 0 { - t.Fatalf("internal/format/zip/zip.go no longer declares %s, so this guard reads nothing", marker) - } - rest := source[at+len(marker):] - open := strings.Index(rest, `"`) - shut := strings.Index(rest[open+1:], `"`) - if open < 0 || shut < 0 { - t.Fatal("the default entry format is not a plain string constant any more") + if archive.DefaultFormat == "" { + t.Fatal("the archive package declares no default entry format, so this guard reads nothing") } - return rest[open+1 : open+1+shut] + return archive.DefaultFormat }