diff --git a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer-tools/main.go b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer-tools/main.go index 464a1921..2fc39c4d 100644 --- a/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer-tools/main.go +++ b/modules/mesh-delivery-indexer/cmd/mesh-delivery-indexer-tools/main.go @@ -129,9 +129,10 @@ func Tools(ask asker, now func() time.Time, read stdio.KeptState, counts func() }}, {Name: index.SeatDefinitionsAgainTool, Description: "Read a contracts module's seat definitions again now (design 26): its trunk's, at the commit the indexer " + - "holds the module at, or with pull and head a pull request's head, and keep them in the seat-definitions state as " + - "a merge or a pull request's push would (seat-definition-changed said where a digest moved; the trunk's " + - "seat-definition-unparsed finding written or cleared). For a read the forge did not answer. A hand act, " + + "holds the module at, or with pull and head a pull request's head (read with the head's own manifest, so its " + + "generated-from as the head has it), and keep them in the seat-definitions state as a merge or a pull request's " + + "push would (seat-definition-changed said where a digest moved; the trunk's seat-definition-unparsed and " + + "seat-defined-twice findings written or cleared). For a read the forge did not answer. A hand act, " + "recorded through mesh-hand-acts before it is asked. indexer_seat_definition reads the result.", Input: map[string]any{"type": "object", "required": []string{"module", "why"}, "properties": map[string]any{ "module": map[string]any{"type": "string", "description": "the contracts module, such as mesh-contracts"}, diff --git a/modules/mesh-delivery-indexer/internal/index/control_records_test.go b/modules/mesh-delivery-indexer/internal/index/control_records_test.go index 06357b29..d0f73bf7 100644 --- a/modules/mesh-delivery-indexer/internal/index/control_records_test.go +++ b/modules/mesh-delivery-indexer/internal/index/control_records_test.go @@ -182,6 +182,42 @@ func TestTheControlRecords(t *testing.T) { if n := len(said); n != 4 || said[2] != (SeatDefinitionChanged{Seat: "gone", Source: SourceTrunk, Repository: repo}) || said[3].Digest == "" { t.Errorf("said %+v, want gone's removal and m3's git", said) } + // Ruling (lead, on A2i's finding 4): another repository's trunk defining git is refused, the trunk's entry stands, + // nothing is said, and its finding names both repositories; its next merge without git clears the finding. + const other = "novox/other-contracts" + files["o1"] = map[string]string{"seats/index.json": `{"format":1,"seats":["git"]}`, "seats/git/seat.json": strings.Replace(git, "ADR 0111", "ADR 0300", 1)} + files["o2"] = map[string]string{"seats/index.json": `{"format":1,"seats":[]}`} + read := ix.ReadFile + ix.ReadFile = func(ctx context.Context, r, sha, p string) ([]byte, error) { + if r == other { + r = repo + } + return read(ctx, r, sha, p) + } + otherManifest := strings.Replace(manifest, `"mesh-contracts"`, `"other-contracts"`, 1) + mergedOther := func(merge string) { + t.Helper() + if err := ix.ApplySeatDefinitions(ctx, delivery.Forge{Kind: delivery.ForgeMerged, Repository: other, Number: 2, Merge: merge, + Manifests: map[string]string{delivery.RootPath: otherManifest}}); err != nil { + t.Fatal(err) + } + } + twice := health.FindingKey(health.ScopeModule, findingID("seat-defined-twice", "other-contracts"), SeatDefinedTwiceKind) + before := len(said) + mergedOther("o1") + if g, _ := entry("git.trunk"); g.Repository != repo || g.Commit != m3 || len(said) != before { + t.Errorf("a second repository's git replaced the trunk's entry or was said: %+v, said %+v", g, said[before:]) + } + if v, ok := findings.vals[twice]; !ok || !strings.Contains(string(v), other) || !strings.Contains(string(v), repo) { + t.Errorf("finding %s = %s, want it naming both repositories", twice, v) + } + mergedOther("o2") + if _, ok := findings.vals[twice]; ok { + t.Error("the seat-defined-twice finding stands once the second repository defines git no more") + } + if g, ok := entry("git.trunk"); !ok || g.Repository != repo { + t.Errorf("the second repository's merge without git removed the first's entry: %+v", g) + } // A pull request's head: its own entry, the trunk's answered unless it is named. if err := ix.ApplySeatDefinitions(ctx, delivery.Forge{Kind: delivery.ForgePull, Action: delivery.PullUpdated, Repository: repo, Number: 9, Head: head, Dirs: []string{delivery.RootPath}, Manifests: map[string]string{delivery.RootPath: manifest}}); err != nil { @@ -225,6 +261,25 @@ func TestTheControlRecords(t *testing.T) { if len(findings.vals) != 0 { t.Errorf("findings %v stand after a clean build end", findings.vals) } + // Ruling (lead, on A2i's finding 3): indexer_seat_definitions_again for a pull request reads the head's own manifest's + // generated-from: a trunk that names none, a head that names it, is read as the contracts module the head is. + ix.Modules, ix.Manifests = newMem(), newMem() + plain := `{"module":"mesh-contracts","version":"1"}` + kept, _ := delivery.EncodeManifest(delivery.Manifest{Module: "mesh-contracts", JSON: plain}) + mkey, _ := delivery.ManifestsKey(delivery.ManifestDigest([]byte(plain))) + ix.Manifests.(*memStore).vals[mkey] = kept + mod, _ := delivery.EncodeModule(delivery.Module{FP2: "src2:" + strings.Repeat("a", 64), Manifest: delivery.ManifestDigest([]byte(plain)), Repository: repo, Commit: m3, Path: delivery.RootPath}) + ix.Modules.(*memStore).vals["mesh-contracts"] = mod + files["h2"] = map[string]string{"module.json": manifest, "seats/index.json": `{"format":1,"seats":["git"]}`, "seats/git/seat.json": strings.Replace(git, "ADR 0111", "ADR 0400", 1)} + if r := ix.SeatDefinitionsAgain(ctx, "mesh-contracts", 9, "h2"); !r.Stored { + t.Errorf("again for the pull request's head = %+v, want it read with the head's generated-from", r) + } + if e, ok := entry(pk); !ok || e.Commit != "h2" { + t.Errorf("the pull request's entry after again = %+v (%v)", e, ok) + } + if r := ix.SeatDefinitionsAgain(ctx, "mesh-contracts", 0, ""); r.Stored || !strings.Contains(r.Answer, "trunk's manifest") { + t.Errorf("again for the trunk, whose manifest names no generated-from = %+v", r) + } // The pull request closes: its entries and its out-of-step files go, said. if err := ix.ApplySeatDefinitions(ctx, delivery.Forge{Kind: delivery.ForgePull, Action: delivery.PullClosed, Repository: repo, Number: 9, Head: head}); err != nil { t.Fatal(err) diff --git a/modules/mesh-delivery-indexer/internal/index/seatdefs.go b/modules/mesh-delivery-indexer/internal/index/seatdefs.go index 3227fcd7..05946ef6 100644 --- a/modules/mesh-delivery-indexer/internal/index/seatdefs.go +++ b/modules/mesh-delivery-indexer/internal/index/seatdefs.go @@ -6,8 +6,10 @@ package index // it with the format's one reader (mesh-sdk seatdef), and keeps it in its own state `seat-definitions`, keyed by seat // and source, as its only writer. Only the trunk's entry is the seat's contract; a pull request's entries go when it // closes or merges. A trunk definition that does not parse keeps the trunk's entry and writes the finding -// seat-definition-unparsed; generated types out of step on a trunk, said by the builder's build end, write the finding -// generated-types-out-of-step, cleared only by a clean build end of the same contracts module. +// seat-definition-unparsed; a trunk defining a seat another repository's trunk already defines is refused with the +// finding seat-defined-twice naming both (a seat has one definition); generated types out of step on a trunk, said by +// the builder's build end, write the finding generated-types-out-of-step, cleared only by a clean build end of the same +// contracts module. import ( "context" @@ -37,9 +39,11 @@ const ( // definition's digest changed (ADR 0608 §3). SeatDefinitionChangedEvent = "seat-definition-changed" SeatDefinitionsStream = "indexer-seat-definitions" - // The two findings (ADR 0608 §4, ADR 0564: the indexer writes findings, the condition keeper raises them). + // The findings (ADR 0608 §4, ADR 0564: the indexer writes findings, the condition keeper raises them). SeatDefinitionUnparsedKind = "seat-definition-unparsed" OutOfStepKind = "generated-types-out-of-step" + // SeatDefinedTwiceKind: a contracts module's trunk defines a seat another repository's trunk already defines. + SeatDefinedTwiceKind = "seat-defined-twice" // The three tools (design 26). SeatDefinitionTool = "indexer_seat_definition" SeatDefinitionsAgainTool = "indexer_seat_definitions_again" @@ -284,7 +288,8 @@ func NewSeatDefinitionsAgain(module string, pull int, head, why, by string, at t const seatDefinitionsPrefix = "seat-definitions." // SeatDefinitionsAgain reads a contracts module's seat definitions again now (indexer_seat_definitions_again): its -// trunk's at the commit the modules state holds it at, or a pull request's at the head given, with the trunk's manifest. +// trunk's at the commit the modules state holds it at, or a pull request's at the head given, with the head's own manifest +// (its generated-from as the head has it). func (ix *Indexer) SeatDefinitionsAgain(ctx context.Context, module string, pull int, head string) TakenIn { out := TakenIn{Module: module} if ix.SeatDefinitions == nil || ix.ReadFile == nil || ix.Modules == nil { @@ -315,9 +320,27 @@ func (ix *Indexer) SeatDefinitionsAgain(ctx context.Context, module string, pull if dir == "" { dir = delivery.RootPath } + whose := "its trunk's" + if pull > 0 { + // A pull request is read with its head's own manifest, so a change of generated-from is read as the head has it. + file := "module.json" + if dir != delivery.RootPath { + file = path.Join(dir, file) + } + raw, err := ix.ReadFile(ctx, m.Repository, head, file) + switch { + case err != nil: + out.Answer = fmt.Sprintf("%s at %.12s cannot be read: %v", file, head, err) + return out + case raw == nil: + out.Answer = fmt.Sprintf("%s#%d's head %.12s holds no %s", m.Repository, pull, head, file) + return out + } + manifest, whose = string(raw), fmt.Sprintf("%s#%d's head's", m.Repository, pull) + } cs := contractsModules(map[string]string{dir: manifest}) if len(cs) == 0 { - out.Answer = module + " is no contracts module: no package artifact of its trunk's manifest names generated-from" + out.Answer = module + " is no contracts module: no package artifact of " + whose + " manifest names generated-from" return out } out.Repository, out.Commit, out.Path = m.Repository, m.Commit, m.Path @@ -348,10 +371,16 @@ func (ix *Indexer) keepTrunk(ctx context.Context, repository, commit string, c c } ix.unparsedFinding(ctx, c.module, unparsed) var errs []error + twice := map[string]string{} // seat -> the repository and module whose trunk holds it for seat, d := range defs { - errs = append(errs, ix.putDefinition(ctx, SeatDefinitionEntry{Seat: seat, Source: SourceTrunk, Repository: repository, - Module: c.module, Directory: c.dir, Commit: commit}, d)) + holder, err := ix.putDefinition(ctx, SeatDefinitionEntry{Seat: seat, Source: SourceTrunk, Repository: repository, + Module: c.module, Directory: c.dir, Commit: commit}, d) + if holder != "" { + twice[seat] = holder + } + errs = append(errs, err) } + ix.twiceFinding(ctx, repository, c.module, twice) // A seat the module's trunk no longer defines leaves the state. keys, err := ix.SeatDefinitions.Keys(ctx) if err != nil { @@ -391,7 +420,8 @@ func (ix *Indexer) keepPull(ctx context.Context, repository string, pull int, he for seat, d := range defs { e := base e.Seat = seat - errs = append(errs, ix.putDefinition(ctx, e, d)) + _, err := ix.putDefinition(ctx, e, d) + errs = append(errs, err) } for seat, why := range unparsed { e := base @@ -410,18 +440,25 @@ func (ix *Indexer) keepPull(ctx context.Context, repository string, pull int, he return errors.Join(errs...) } -// putDefinition writes one parsed definition at its key, said when its digest moved. -func (ix *Indexer) putDefinition(ctx context.Context, e SeatDefinitionEntry, d seatdef.Definition) error { +// putDefinition writes one parsed definition at its key, said when its digest moved. A trunk's definition of a seat +// another repository's (or module's) trunk already defines is refused, written nowhere: a seat has one definition. The +// refused one answers who holds it (" ()"). +func (ix *Indexer) putDefinition(ctx context.Context, e SeatDefinitionEntry, d seatdef.Definition) (string, error) { raw, digest, err := digestOf(d) if err != nil { - return err + return "", err } e.Definition, e.Digest, e.At = raw, digest, ix.now().UTC() - moved := false + moved, holder := false, "" err = write(ctx, ix.SeatDefinitions, SeatDefinitionKey(e.Seat, e.Source, e.Repository, e.Pull), func(value []byte, found bool) ([]byte, bool, error) { var cur SeatDefinitionEntry - if found && json.Unmarshal(value, &cur) == nil && cur.Digest == digest && cur.Unparsed == "" { - moved = false + known := found && json.Unmarshal(value, &cur) == nil + moved, holder = false, "" + if known && e.Source == SourceTrunk && (cur.Repository != e.Repository || cur.Module != e.Module) { + holder = cur.Repository + " (" + cur.Module + ")" + return nil, false, nil + } + if known && cur.Digest == digest && cur.Unparsed == "" { return nil, false, nil } moved = true @@ -431,7 +468,23 @@ func (ix *Indexer) putDefinition(ctx context.Context, e SeatDefinitionEntry, d s if err == nil && moved { ix.sayChanged(ctx, SeatDefinitionChanged{Seat: e.Seat, Source: e.Source, Repository: e.Repository, Pull: e.Pull, Digest: digest}) } - return err + return holder, err +} + +// twiceFinding writes the contracts module's seat-defined-twice finding for the seats its trunk defines that another +// repository's trunk already holds, naming both, or clears it when there are none. +func (ix *Indexer) twiceFinding(ctx context.Context, repository, module string, twice map[string]string) { + var lines []string + for seat, holder := range twice { + lines = append(lines, fmt.Sprintf("%s: defined by %s (%s) and already by %s", seat, repository, module, holder)) + } + slices.Sort(lines) + ix.finding(ctx, findingID("seat-defined-twice", module), SeatDefinedTwiceKind, len(lines) > 0, + fmt.Sprintf("%s's trunk defines seats another trunk already defines, refused: %s", module, strings.Join(lines, "; ")), + "A seat on "+module+"'s trunk is already defined by another repository", + "A seat has one definition: the one that stood first stays the seat's contract and "+module+"'s is kept nowhere. "+ + "Remove the seat from one of the two repositories; the next merge of "+module+" clears this.", + "No seat on "+module+"'s trunk is defined by another repository.") } // dropPull removes a pull request's entries and its out-of-step files.