diff --git a/modules/mesh-delivery-checker/cmd/mesh-checking-program/main_test.go b/modules/mesh-delivery-checker/cmd/mesh-checking-program/main_test.go index 352c822a..9360368c 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-checking-program/main_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-checking-program/main_test.go @@ -112,5 +112,30 @@ func TestTheCheckingProgramAnswersTheVerdictOfItsFacts(t *testing.T) { a.Verdict.Failures[1].Words != "novox/x#3 has files in conflict: go.mod" || a.Verdict.Machines[0] != "laptop" || a.Verdict.Levels[delivery.LevelUnit].Run != 3 { t.Errorf("%s: the verdict %+v, want %+v", name, a.Verdict, want) } + } // Review round 1 of milestone 47/B0: the module graph's cycle rule is the program's (ADR 0603 §2, ADR 0656 §5), over the + // edges the running checker gathers. A cycle the change makes fails; one the trunk has among the same modules is noted. + web, mail := rules.Edge{From: "web", To: "mail", Kind: rules.EdgeAwaits}, rules.Edge{From: "mail", To: "web", Kind: rules.EdgeAwaits} + only := func(e rules.Edge, where string) rules.Edge { e.Only = where; return e } + for _, row := range []struct { + name string + graph rules.Graph + fails string + note string + }{ + {name: "no cycle", graph: rules.Graph{Trunk: true, Edges: []rules.Edge{web}}, note: "has no cycle"}, + {name: "a cycle the change makes", graph: rules.Graph{Trunk: true, Edges: []rules.Edge{mail, only(web, rules.EdgeChange)}}, fails: "has the cycle mail -> web -> mail"}, + {name: "a cycle the trunk has", graph: rules.Graph{Trunk: true, Edges: []rules.Edge{mail, web}}, note: "which the trunk already has"}, + {name: "the trunk unread", graph: rules.Graph{Edges: []rules.Edge{mail, web}}, fails: "has the cycle mail -> web -> mail"}, + {name: "a base on itself", graph: rules.Graph{Trunk: true, Edges: []rules.Edge{only(rules.Edge{From: "web", To: "web", Kind: rules.EdgeBase}, rules.EdgeChange)}}, fails: "has the cycle web -> web"}, + {name: "an edge only the trunk has", graph: rules.Graph{Trunk: true, Edges: []rules.Edge{mail, only(web, rules.EdgeTrunk)}}, note: "has no cycle"}, + } { + g := row.graph + a := rules.Judged(rules.Facts{V: rules.FactsVersion, Steps: []rules.Step{{Graph: &g}}}) + if row.fails != "" && (a.Verdict.State != delivery.VerdictFail || !strings.Contains(a.Verdict.Failed.Words, row.fails)) { + t.Errorf("%s: verdict %+v, want a fail saying %q", row.name, a.Verdict, row.fails) + } + if row.note != "" && (a.Verdict.State != delivery.VerdictPass || !strings.Contains(strings.Join(a.Notes, "\n"), row.note)) { + t.Errorf("%s: verdict %s, notes %q, want a pass noting %q", row.name, a.Verdict.State, a.Notes, row.note) + } } } diff --git a/modules/mesh-delivery-checker/internal/check/graph_cycle.go b/modules/mesh-delivery-checker/internal/check/graph_cycle.go deleted file mode 100644 index 48981459..00000000 --- a/modules/mesh-delivery-checker/internal/check/graph_cycle.go +++ /dev/null @@ -1,76 +0,0 @@ -package check - -import ( - "context" - "fmt" - "maps" - "slices" - "strings" - - "git.novox.be/novox/mesh-sdk/go/catalogue" - "git.novox.be/novox/mesh-sdk/go/composer" - - "git.novox.be/novox/mesh-delivery/sdk/graph" -) - -// The check refuses a change whose module graph has a cycle and names it (novox/hq ADR 0603 §2): modules that would each -// wait for the other for good, through their bases, the provisions and bus state they await, or the verb edges the graph -// keeps (ADR 0614 §6). A verb edge on a cycle is left out of the graph, so it is never refused (§3), and a package or a -// build module's edge is a trigger, never a wait, so a cycle through one is not refused either (§6). The graph is worked out from the -// catalogue as the change leaves it (every trunk manifest, each one a pull request touches replaced by its candidate's), -// with the contract's ManifestGraph, the way the indexer works out its graph's edges. A cycle the trunk already has among -// the same modules is the trunk's: said in the verdict's notes, not the change's failure. - -// ManifestReader reads a kept manifest by its digest (the indexer's manifests state). -type ManifestReader interface { - Manifest(ctx context.Context, digest string) (catalogue.Manifest, error) -} - -// refuseCycle fails the check for a cycle of the change's module graph, naming it; nil when the case touches no manifest, -// when nothing reads manifests, or when there is none. A manifest that cannot be read fails the check, said, never passes it. -func (st *state) refuseCycle(ctx context.Context) { - if st.j.Manifests == nil || st.j.Read == nil || st.narrowTry() { - return - } - trunk := st.j.Manifests.ManifestDigests() - req, touched := judgeRequest(trunk, st.c.Pulls) - if len(touched) == 0 { - return - } - read := func(digests map[string]composer.Manifest) (map[string]catalogue.Manifest, error) { - out := map[string]catalogue.Manifest{} - for _, module := range slices.Sorted(maps.Keys(digests)) { - m, err := st.j.Read.Manifest(ctx, digests[module].Digest) - if err != nil { - return nil, fmt.Errorf("the manifest %s of %s: %w", digests[module].Digest, module, err) - } - out[module] = m - } - return out, nil - } - change, err := read(req.Cases[0].Manifests) - if err != nil { - st.fail(goModule, "the change's module graph cannot be worked out, so no cycle can be ruled out: "+err.Error()) - return - } - cycle := graph.OfManifests(change).Cycle() - if cycle == nil { - st.c.note("the change's module graph has no cycle") - return - } - words := strings.Join(cycle, " -> ") - if before, err := read(req.Cases[1].Manifests); err == nil { - among := graph.Graph{} - for name, n := range graph.OfManifests(before) { - if slices.Contains(cycle, name) { - among[name] = n - } - } - if among.Cycle() != nil { - st.c.note("the module graph has the cycle " + words + ", which the trunk already has among the same modules (ADR 0603 §2)") - return - } - } - st.fail(goModule, fmt.Sprintf("the change's module graph has the cycle %s: these modules would each wait for the other for "+ - "good, through their bases, the provisions and bus state they await, or the verbs they invoke (ADR 0603 §2, ADR 0614 §6)", words)) -} diff --git a/modules/mesh-delivery-checker/internal/check/graph_edges.go b/modules/mesh-delivery-checker/internal/check/graph_edges.go new file mode 100644 index 00000000..60b161ac --- /dev/null +++ b/modules/mesh-delivery-checker/internal/check/graph_edges.go @@ -0,0 +1,115 @@ +package check + +import ( + "cmp" + "context" + "fmt" + "maps" + "slices" + "strings" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/composer" + + "git.novox.be/novox/mesh-delivery/sdk/graph" + "mesh-delivery-checker/rules" +) + +// The module graph of a change (novox/hq ADR 0603 §2, ADR 0614 §6): the running checker gathers its ordering edges, the +// checking program judges whether it has a cycle (rules.JudgeGraph, ADR 0656 §5). The graph is worked out from the +// catalogue as the change leaves it (every trunk manifest, each one a pull request touches replaced by its candidate's) +// and as the trunk has it, with the contract's ManifestGraph, the way the indexer works out its graph's edges; the facts +// carry only each edge's two modules and its kind. + +// ManifestReader reads a kept manifest by its digest (the indexer's manifests state). +type ManifestReader interface { + Manifest(ctx context.Context, digest string) (catalogue.Manifest, error) +} + +// gatherGraph records the change's and the trunk's module graph edges as a fact; none when the case touches no manifest +// or nothing reads manifests. A change's manifest that cannot be read fails the check, said, never passes it; a trunk's +// that cannot be read leaves the trunk's graph unread, so no cycle is the trunk's. +func (st *state) gatherGraph(ctx context.Context) { + if st.j.Manifests == nil || st.j.Read == nil || st.narrowTry() { + return + } + req, touched := judgeRequest(st.j.Manifests.ManifestDigests(), st.c.Pulls) + if len(touched) == 0 { + return + } + read := func(digests map[string]composer.Manifest) (map[string]catalogue.Manifest, error) { + out := map[string]catalogue.Manifest{} + for _, module := range slices.Sorted(maps.Keys(digests)) { + m, err := st.j.Read.Manifest(ctx, digests[module].Digest) + if err != nil { + return nil, fmt.Errorf("the manifest %s of %s: %w", digests[module].Digest, module, err) + } + out[module] = m + } + return out, nil + } + change, err := read(req.Cases[0].Manifests) + if err != nil { + st.fail(goModule, "the change's module graph cannot be worked out, so no cycle can be ruled out: "+err.Error()) + return + } + g := rules.Graph{} + changed := orderingEdges(graph.OfManifests(change)) + trunk := map[[2]string]string{} + if before, err := read(req.Cases[1].Manifests); err == nil { + g.Trunk = true + trunk = orderingEdges(graph.OfManifests(before)) + } + for _, pair := range slices.SortedFunc(maps.Keys(changed), cmpPair) { + e := rules.Edge{From: pair[0], To: pair[1], Kind: changed[pair]} + if _, both := trunk[pair]; !both && g.Trunk { + e.Only = rules.EdgeChange + } + g.Edges = append(g.Edges, e) + } + for _, pair := range slices.SortedFunc(maps.Keys(trunk), cmpPair) { + if _, both := changed[pair]; !both { + g.Edges = append(g.Edges, rules.Edge{From: pair[0], To: pair[1], Kind: trunk[pair], Only: rules.EdgeTrunk}) + } + } + st.facts = append(st.facts, rules.Step{Graph: &g}) +} + +func cmpPair(a, b [2]string) int { + return cmp.Or(strings.Compare(a[0], b[0]), strings.Compare(a[1], b[1])) +} + +// orderingEdges are the graph's ordering edges (graph.Graph.Before) between its modules, each pair once with the kind it +// is first met by: a base, a provision or bus state awaited of another provider, or a verb the graph keeps served by +// another module (a base on itself is a cycle). A verb edge the graph leaves out (ADR 0614 §3), a toolchain and a package edge are none (§6). +func orderingEdges(g graph.Graph) map[[2]string]string { + out := map[[2]string]string{} + add := func(from, to, kind string) { + if _, ours := g[to]; !ours || (from == to && kind != rules.EdgeBase) { + return + } + if _, met := out[[2]string{from, to}]; !met { + out[[2]string{from, to}] = kind + } + } + for _, name := range slices.Sorted(maps.Keys(g)) { + n := g[name] + for _, b := range n.Bases { + add(name, b, rules.EdgeBase) + } + for _, a := range n.Awaits { + for _, p := range g.Providers(a) { + add(name, p, rules.EdgeAwaits) + } + } + for _, v := range n.Invokes { + if slices.Contains(n.LeftOut, v) { + continue + } + for _, s := range g.Servers(v) { + add(name, s, rules.EdgeInvokes) + } + } + } + return out +} diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index 246345c4..d448b765 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -312,7 +312,7 @@ func (j *Examiner) Judge(ctx context.Context, c *Case) (Outcome, error) { var by *delivery.JudgedBy if st.seatList(ctx) { st.judgeManifests(ctx) - st.refuseCycle(ctx) + st.gatherGraph(ctx) st.compose(ctx) // The verdict is the checking program's of the facts (ADR 0644 §1, ADR 0656 §5). answer, by = st.judged(ctx) diff --git a/modules/mesh-delivery-checker/rules/facts.go b/modules/mesh-delivery-checker/rules/facts.go index 573c29de..05b67b2b 100644 --- a/modules/mesh-delivery-checker/rules/facts.go +++ b/modules/mesh-delivery-checker/rules/facts.go @@ -51,7 +51,7 @@ type Member struct { // Step is one fact of the check, in the order met: the node that ran it, and one of a level its build module answered // (Level, as the build module gave it), what its fetch read of compiled programs (Programs), the manifest validator's -// answers (Manifests) or a failure the asking met (Failed: a step not asked, refused or not answered, a conflict, worded +// answers (Manifests), the module graph's ordering edges (Graph) or a failure the asking met (Failed: a step not asked, refused or not answered, a conflict, worded // by the running checker). A step with none only says which node ran it. type Step struct { Repository string `json:"repository"` @@ -62,6 +62,8 @@ type Step struct { Failed *delivery.FailedFirst `json:"failed,omitempty"` Programs *Programs `json:"programs,omitempty"` Manifests *Manifests `json:"manifests,omitempty"` + // Graph is the module graph's ordering edges, as the change leaves them and as the trunk has them (JudgeGraph). + Graph *Graph `json:"graph,omitempty"` } // LevelFact is a level answer as the facts carry it: only what the rules read (Tally.Level), so a group's facts stay @@ -193,6 +195,12 @@ func Judged(f Facts) Answer { if ff := JudgePrograms(s.Node, *s.Programs); ff != nil { failures = append(failures, &delivery.Verdict{State: delivery.VerdictFail, Failed: *ff}) } + case s.Graph != nil: + ff, said := JudgeGraph(*s.Graph) + notes = append(notes, said...) + if ff != nil { + failures = append(failures, &delivery.Verdict{State: delivery.VerdictFail, Failed: *ff}) + } case s.Manifests != nil: ff, said := JudgeManifests(*s.Manifests) notes = append(notes, said...) diff --git a/modules/mesh-delivery-checker/rules/graph.go b/modules/mesh-delivery-checker/rules/graph.go new file mode 100644 index 00000000..ed729098 --- /dev/null +++ b/modules/mesh-delivery-checker/rules/graph.go @@ -0,0 +1,117 @@ +package rules + +import ( + "fmt" + "slices" + "strings" + + "git.novox.be/novox/mesh-delivery/modules/mesh-delivery-builder/buildrun" + "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// The module graph's cycle rule (novox/hq ADR 0603 §2, ADR 0614 §3, §6), judged by the checking program (ADR 0656 §5): +// the check refuses a change whose module graph has a cycle and names it; a cycle the trunk already has among the same +// modules is the trunk's, said in the notes. The running checker gathers the graph's ordering edges of the catalogue as +// the change leaves it and as the trunk has it, worked out with the contract's graph (sdk/graph: bases, the provisions +// and bus state a module awaits, the verb edges the graph keeps), as module names and the edge's kind, never the +// manifests themselves, so the facts stay within the run input's bound. + +// The kinds of an ordering edge. +const ( + EdgeBase = "base" + EdgeAwaits = "awaits" + EdgeInvokes = "invokes" +) + +// Edge is one ordering edge: From waits for To. Only is EdgeChange when only the change's graph has it, EdgeTrunk when +// only the trunk's does, empty when both have it. +type Edge struct { + From string `json:"from"` + To string `json:"to"` + Kind string `json:"kind"` + Only string `json:"only,omitempty"` +} + +// Where only one graph has an edge. +const ( + EdgeChange = "change" + EdgeTrunk = "trunk" +) + +// Graph is the module graph's facts: every ordering edge of either graph, between modules of the catalogue. Trunk says +// whether the trunk's graph was read; when it was not, every edge is the change's and no cycle is the trunk's. +type Graph struct { + Edges []Edge `json:"edges"` + Trunk bool `json:"trunk"` +} + +// JudgeGraph is the cycle rule over the graph's facts: the failure of a cycle the change makes, or the note of no cycle +// or of a cycle the trunk already has. +func JudgeGraph(g Graph) (*delivery.FailedFirst, []string) { + cycle := firstCycle(g.Edges, func(e Edge) bool { return e.Only != EdgeTrunk || !g.Trunk }) + if cycle == nil { + return nil, []string{"the change's module graph has no cycle"} + } + words := strings.Join(cycle, " -> ") + if g.Trunk { + among := firstCycle(g.Edges, func(e Edge) bool { + return e.Only != EdgeChange && slices.Contains(cycle, e.From) && slices.Contains(cycle, e.To) + }) + if among != nil { + return nil, []string{"the module graph has the cycle " + words + ", which the trunk already has among the same modules (ADR 0603 §2)"} + } + } + return &delivery.FailedFirst{Machine: buildrun.GoBuildModule, Words: fmt.Sprintf("the change's module graph has the cycle %s: these modules would "+ + "each wait for the other for good, through their bases, the provisions and bus state they await, or the verbs they invoke (ADR 0603 §2, ADR 0614 §6)", words)}, nil +} + +// firstCycle is the first cycle of the edges kept, as the modules it passes through with the first named again at its +// end, visiting the modules and each one's edges in sorted order (as sdk/graph's Cycle does), or nil for none. +func firstCycle(edges []Edge, keep func(Edge) bool) []string { + next := map[string][]string{} + for _, e := range edges { + if keep(e) && !slices.Contains(next[e.From], e.To) { + next[e.From] = append(next[e.From], e.To) + } + } + var names []string + for name, to := range next { + slices.Sort(to) + names = append(names, name) + } + slices.Sort(names) + const ( + unseen = iota + onPath + done + ) + state := map[string]int{} + var path []string + var visit func(string) []string + visit = func(name string) []string { + state[name] = onPath + path = append(path, name) + for _, to := range next[name] { + switch state[to] { + case onPath: + i := slices.Index(path, to) + return append(slices.Clone(path[i:]), to) + case unseen: + if c := visit(to); c != nil { + return c + } + } + } + path = path[:len(path)-1] + state[name] = done + return nil + } + for _, name := range names { + if state[name] == unseen { + if c := visit(name); c != nil { + return c + } + } + } + return nil +}