diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index 4dd4678..6a7c5e7 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -22,7 +22,8 @@ import ( // // **The mesh's module graph decides, not the repository.** The controller holds the graph — every module, // the repository and directory it is built from — and maps the pull request's changed paths onto it by -// the rule the merge handler and the release planner use (whatTheMergeTouched, issue 280): **a changed file +// the planner's own answer (reachOfMerge, touchedBy — the one place a changed file is mapped onto modules, +// for the merge handler, the release planner, the merge gate and this check; issue 280): **a changed file // touches exactly the modules whose build reads it** — a module's own directory (the whole repository for // one built from its root), or a repository its recipe packages. A file no build reads — a script at the // root, a README — touches no module. A directory the change adds a module.json in, which the graph does @@ -52,14 +53,20 @@ const noMergeCheck = "the repository declares no merge-check.sh: none of its own var coreModules = map[string]string{"mesh-controller": "mesh-controller", "mesh-host": "mesh-host", "node-tools": "mesh-tools", "nats": "mesh-catalog"} -// checkScope is what a pull request touches of the mesh's graph. +// checkScope is what a pull request reaches of the mesh's graph, as the planner reckons it. type checkScope struct { - // Modules are the graph's modules built from the repository into the pull request's base that the - // change touches, sorted; New the directories it adds a module in that the graph does not hold. - Modules []string - New []string - // Manifests are their manifests in the change's tree. + // Modules are the modules a merge of the change would move itself — built from the repository into + // the pull request's base and reading a changed file, or packaging the repository's source — and + // Dependents those the plan would build after them; New the directories it adds a module in. + Modules []string + Dependents []string + New []string + // Manifests are the moved modules' and the new ones' manifests in the change's tree. Manifests []string + // Width is how many modules a merge would build, in how many tiers; Unread the changed files no + // module's build reads. + Width, Tiers int + Unread []string // Mesh says the repository is the mesh's: modules are built from it on some branch, its owner is the // core's, or the change adds a module to it. Mesh bool @@ -71,13 +78,35 @@ type checkScope struct { func (s checkScope) gated() bool { return len(s.Modules)+len(s.New) > 0 } -// pullScope maps a pull request onto the mesh's module graph. -func pullScope(p link.PullUpdated, entries []inventory.Entry, read map[string][]inventory.ReadRepository) checkScope { +// pullScope is what a pull request reaches: **the planner's own answer** (reachOfMerge), asked as if the +// head were merged into the base — never a mapping of its own, so a change to what a merge touches +// changes what is checked with it (novox/hq ADR 0238). +func pullScope(p link.PullUpdated, entries []inventory.Entry, read map[string][]inventory.ReadRepository, + edges []inventory.Edge) checkScope { m := link.SourceMoved{Owner: p.Owner, Repo: p.Repo, Base: p.Base, CloneURL: p.CloneURL, Commit: p.Commit, - Paths: p.Paths, PathsTruncated: p.PathsTruncated, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid} - var s checkScope - var from []inventory.Entry - known := map[string]bool{} + Paths: p.Paths, PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, + ModuleDirsSaid: p.ModuleDirsSaid} + r := reachOfMerge(m, entries, read, edges) + s := checkScope{Modules: r.Moved(), Dependents: r.Dependents(), New: r.Added, Unread: r.Unread, + Width: len(r.Plan.Modules), Tiers: len(r.Plan.Tiers)} + for _, e := range append(append([]inventory.Entry{}, r.Touched...), r.Deleted...) { + s.Manifests = append(s.Manifests, path.Join(strings.Trim(e.Source.Path, "/"), moduleManifestFile)) + switch e.Manifest.Module { + case "mesh-controller": + s.Judge = link.JudgeSelf + case "mesh-host": + if s.Judge == "" { + s.Judge = link.JudgeValidator + } + } + } + for _, d := range r.Added { + s.Manifests = append(s.Manifests, path.Join(d, moduleManifestFile)) + } + sort.Strings(s.Manifests) + s.Manifests = slices.Compact(s.Manifests) + + // Whose repository it is, for how it is cloned and whether its own check is the mesh's to run. owners := map[string]bool{} for i, e := range entries { if e.Provided { @@ -88,77 +117,11 @@ func pullScope(p link.PullUpdated, entries []inventory.Entry, read map[string][] owners[owner] = true } } - if !sameRepository(e.Source.Repository, m) { - continue - } - s.Mesh = true - known[strings.Trim(e.Source.Path, "/")] = true - if s.From == nil { + if sameRepository(e.Source.Repository, m) && s.From == nil { s.From = &entries[i] } - if sourceIs(e.Source, m) { - from = append(from, e) - } - } - touched := map[string]inventory.Entry{} - if len(from) > 0 { - for _, e := range whatTheMergeTouched(from, entries, m) { - touched[e.Manifest.Module] = e - } - } - // A module whose build packages source from this repository's branch is touched by any change to it. - for _, e := range entries { - if !e.Provided && readsFrom(read[e.Manifest.Module], m) { - touched[e.Manifest.Module] = e - } - } - for name, e := range touched { - s.Modules = append(s.Modules, name) - if sameRepository(e.Source.Repository, m) { - s.Manifests = append(s.Manifests, path.Join(strings.Trim(e.Source.Path, "/"), "module.json")) - } - switch name { - case "mesh-controller": - s.Judge = link.JudgeSelf - case "mesh-host": - if s.Judge == "" { - s.Judge = link.JudgeValidator - } - } - } - // A module the graph does not hold yet, in a directory the head says holds one — or at the root. - for _, d := range saidModuleDirs(m) { - if !known[d] { - s.New = append(s.New, d) - s.Manifests = append(s.Manifests, d+"/module.json") - } - } - // And, said or not, a manifest the change adds or moves in a directory the graph does not know. - for _, changed := range p.Paths { - changed = strings.Trim(changed, "/") - if path.Base(changed) != "module.json" { - continue - } - d := path.Dir(changed) - if d == "." { - d = "" - } - if !known[d] { - if d == "" { - d = "." - } - s.New = append(s.New, d) - s.Manifests = append(s.Manifests, changed) - } - } - sort.Strings(s.Modules) - sort.Strings(s.New) - s.New = slices.Compact(s.New) - sort.Strings(s.Manifests) - s.Manifests = slices.Compact(s.Manifests) - if owners[strings.ToLower(p.Owner)] || s.gated() { - s.Mesh = true } + s.Mesh = s.From != nil || owners[strings.ToLower(p.Owner)] || s.gated() return s } @@ -182,7 +145,11 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { if err != nil { return err } - scope := pullScope(p, entries, read) + edges, err := inv.Dependencies(ctx) + if err != nil { + return err + } + scope := pullScope(p, entries, read, edges) direct := link.Checked{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Commit: p.Commit, ID: link.NewBuildID(time.Now()), Verdict: "pass", Summary: noModuleTouched, Gate: &link.CheckLayer{Verdict: "pass", Summary: noModuleTouched}} @@ -213,7 +180,9 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { } what := "its own merge-check.sh alone: " + noModuleTouched if scope.gated() { - what = "the gate over " + strings.Join(append(append([]string{}, scope.Modules...), scope.New...), ", ") + what = fmt.Sprintf("the gate over %s (a merge would build %d module(s) in %d tier(s))", + strings.Join(append(append([]string{}, scope.Modules...), prefixedAll("new:", scope.New)...), ", "), + scope.Width, scope.Tiers) } fmt.Printf("%s/%s#%d (%.8s): asked %s to check it before it merges — %s — as %s\n", p.Owner, p.Repo, p.Number, p.Commit, seat, what, request.ID) @@ -289,8 +258,8 @@ func checkRequestFor(ctx context.Context, open *stores, p link.PullUpdated, scop Seats: seatBases(ctx), Source: sourceOnSeat(source), Check: &link.CheckRequest{Owner: p.Owner, Repo: p.Repo, Number: p.Number, Base: p.Base, - Paths: p.Paths, Beside: beside, Modules: scope.Modules, New: scope.New, Manifests: scope.Manifests, - Judge: scope.Judge}, + Paths: p.Paths, Beside: beside, Modules: scope.Modules, Dependents: scope.Dependents, New: scope.New, + Manifests: scope.Manifests, Judge: scope.Judge}, }, nil } @@ -354,6 +323,9 @@ func checkedOf(result link.BuildResult) link.Checked { if result.Checked != nil && c.Gate != nil && len(c.Gate.Modules) == 0 { c.Gate.Modules = append(append([]string{}, result.Checked.Modules...), prefixedAll("new:", result.Checked.New)...) } + if result.Checked != nil && c.Gate != nil && len(c.Gate.Dependents) == 0 { + c.Gate.Dependents = result.Checked.Dependents + } for _, l := range []*link.CheckLayer{c.Gate, c.RepoCheck} { if l != nil && l.Verdict == "" { l.Verdict = "error" diff --git a/cmd/mesh-controller/checks_test.go b/cmd/mesh-controller/checks_test.go index f60f1b7..2aee808 100644 --- a/cmd/mesh-controller/checks_test.go +++ b/cmd/mesh-controller/checks_test.go @@ -44,7 +44,7 @@ func TestThePullRequestIsMappedOntoTheModuleGraph(t *testing.T) { {"a file no module's build reads touches no module (issue 280)", pull("novox", "mesh-catalog", "main", []string{"merge-check.sh", "README.md"}), "", "", "", true, ""}, {"files the announcer could not list: everything built from it", - link.PullUpdated{Owner: "novox", Repo: "mesh-catalog", Base: "main", PathsTruncated: true, + link.PullUpdated{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "abc", PathsTruncated: true, Paths: []string{"README.md"}}, "gitea,keycloak,nats", "", "modules/gitea/module.json,modules/keycloak/module.json,modules/nats/module.json", true, ""}, {"a new module, said by the head", pull("novox", "mesh-catalog", "main", @@ -67,7 +67,7 @@ func TestThePullRequestIsMappedOntoTheModuleGraph(t *testing.T) { {"a repository adding a module at its root", pull("someone", "newapp", "main", []string{"module.json", "x.js"}), "", ".", "module.json", true, ""}, } { - s := pullScope(c.p, entries, nil) + s := pullScope(c.p, entries, nil, nil) got := []string{strings.Join(s.Modules, ","), strings.Join(s.New, ","), strings.Join(s.Manifests, ",")} want := []string{c.modules, c.new, c.manifest} for i, what := range []string{"modules", "new", "manifests"} { @@ -88,7 +88,7 @@ func TestThePullRequestIsMappedOntoTheModuleGraph(t *testing.T) { func TestAPullRequestTouchesWhatPackagesItsRepository(t *testing.T) { entries := []inventory.Entry{fromRepo("node-tools", "http://forge.internal:20000/novox/mesh-tools.git", "node-tools")} read := map[string][]inventory.ReadRepository{"node-tools": {{Repository: "http://forge.internal:20000/novox/mesh-sdk.git"}}} - s := pullScope(link.PullUpdated{Owner: "novox", Repo: "mesh-sdk", Base: "main", Paths: []string{"go/x.go"}}, entries, read) + s := pullScope(link.PullUpdated{Owner: "novox", Repo: "mesh-sdk", Base: "main", Commit: "abc", Paths: []string{"go/x.go"}}, entries, read, nil) if strings.Join(s.Modules, ",") != "node-tools" || len(s.Manifests) != 0 { t.Fatalf("a change to what node-tools packages touched %v (manifests %v)", s.Modules, s.Manifests) } @@ -115,3 +115,57 @@ func TestAChecksLayersAreEachSaidAndAnErrorIsNeverAPass(t *testing.T) { t.Fatalf("the layers said %+v / %+v", c.Gate, c.RepoCheck) } } + +// **The check asks the planner, never a mapping of its own** (novox/hq ADR 0238): what a pull request +// reaches is reachOfMerge's answer — the same that plans a merge — so a file at the root touches no +// module in both, and what stands on a touched module is named as its dependents in both. +func TestAPullRequestReachesWhatAMergeOfItWouldPlan(t *testing.T) { + const catalogue = "http://forge.internal:20000/novox/mesh-catalog.git" + nats := fromRepo("nats", catalogue, "modules/nats") + nats.Source.BuiltFrom = "old" + gitea := fromRepo("gitea", catalogue, "modules/gitea") + gitea.Source.BuiltFrom = "old" + tools := fromRepo("node-tools", "http://forge.internal:20000/novox/mesh-tools.git", "node-tools") + entries := []inventory.Entry{nats, gitea, tools} + // node-tools stands on the bus's image; a code edge, so a plan takes it along. + edges := []inventory.Edge{{From: "node-tools", To: "nats", Kind: inventory.EdgeStandsOn}} + read := map[string][]inventory.ReadRepository{} + + for _, c := range []struct { + what string + paths []string + moved, dependents string + width int + unread string + }{ + {"a file at the root, read by no build", []string{"merge-check.sh"}, "", "", 0, "merge-check.sh"}, + {"the bus's own directory", []string{"modules/nats/Dockerfile"}, "nats", "node-tools", 2, ""}, + {"both, and a README", []string{"modules/gitea/index.ts", "modules/nats/x", "README.md"}, "gitea,nats", "node-tools", 3, "README.md"}, + } { + p := link.PullUpdated{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "head", Paths: c.paths} + s := pullScope(p, entries, read, edges) + m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "head", Paths: c.paths} + r := reachOfMerge(m, entries, read, edges) + if got := strings.Join(s.Modules, ","); got != c.moved || got != strings.Join(r.Moved(), ",") { + t.Errorf("%s: the check reaches %q, the planner %v, wanted %q", c.what, got, r.Moved(), c.moved) + } + if got := strings.Join(s.Dependents, ","); got != c.dependents { + t.Errorf("%s: dependents %q, wanted %q", c.what, got, c.dependents) + } + if s.Width != c.width || s.Width != len(r.Plan.Modules) { + t.Errorf("%s: a merge would build %d, the planner says %d, wanted %d", c.what, s.Width, len(r.Plan.Modules), c.width) + } + if got := strings.Join(s.Unread, ","); got != c.unread { + t.Errorf("%s: unread %q, wanted %q", c.what, got, c.unread) + } + } + + // A repository whose build another module's recipe packages: that module is reached through the + // planner's `read`, and what stands on it after it. + read["gitea"] = []inventory.ReadRepository{{Repository: "http://forge.internal:20000/novox/mesh-sdk.git"}} + s := pullScope(link.PullUpdated{Owner: "novox", Repo: "mesh-sdk", Base: "main", Commit: "head", + Paths: []string{"src/index.ts"}}, entries, read, edges) + if strings.Join(s.Modules, ",") != "gitea" || len(s.Manifests) != 0 { + t.Fatalf("a change to the source gitea packages reaches %v (manifests %v)", s.Modules, s.Manifests) + } +} diff --git a/cmd/mesh-controller/merge_gate.go b/cmd/mesh-controller/merge_gate.go index 266fb5c..4e6b0fb 100644 --- a/cmd/mesh-controller/merge_gate.go +++ b/cmd/mesh-controller/merge_gate.go @@ -206,9 +206,25 @@ func judgeChange(ctx context.Context, in mergeCheckInput) (mergeVerdict, error) } change := base sources := sourcesOfFacts(in.facts) + // **What the change moves is the planner's answer** (reachOfMerge, novox/hq ADR 0238), asked of the + // snapshot as if the change were merged: the definitions composed with the change are those of the + // modules a merge of it would rebuild, and of the modules it adds — not every definition in the tree, + // which may differ from what the mesh holds for reasons that are not this change's (a module pinned + // at an older commit, a main the mesh has not built yet). + var reach *mergeReach + if in.repository != "" && len(in.changed) > 0 { + r, err := reachOfChange(in.facts, in.repository, in.changed, in.tree) + if err != nil { + v.Notes = append(v.Notes, "what a merge of the change would rebuild could not be worked out, so every "+ + "definition in its tree is judged: "+err.Error()) + } else { + reach = &r + } + } if in.tree != "" { var failures []string - change, failures, err = shelfWithChange(base, sources, in.facts, in.repository, in.tree, v.Modules) + change, failures, err = shelfWithChange(base, sources, in.facts, in.repository, in.tree, v.Modules, + scopeOfReach(reach, in.repository)) if err != nil { return v, err } @@ -306,20 +322,16 @@ func judgeChange(ctx context.Context, in mergeCheckInput) (mergeVerdict, error) } } - if in.repository != "" && len(in.changed) > 0 { - w, err := rebuildWidth(in.facts, in.repository, in.changed, in.tree) - if err != nil { - v.Notes = append(v.Notes, "the width of the rebuild could not be worked out: "+err.Error()) - } else { - v.Width = &w - if len(w.Unread) > 0 { - v.Notes = append(v.Notes, fmt.Sprintf("%s read by no module's build, and rebuild nothing (issue 280)", - readableList(w.Unread))) - } - if len(w.Modules) > wideRebuild { - v.Warnings = append(v.Warnings, fmt.Sprintf("a merge rebuilds %d module(s) in %d tier(s)", - len(w.Modules), w.Tiers)) - } + if reach != nil { + w := widthOf(*reach) + v.Width = &w + if len(w.Unread) > 0 { + v.Notes = append(v.Notes, fmt.Sprintf("%s read by no module's build, and rebuild nothing (issue 280)", + readableList(w.Unread))) + } + if len(w.Modules) > wideRebuild { + v.Warnings = append(v.Warnings, fmt.Sprintf("a merge rebuilds %d module(s) in %d tier(s)", + len(w.Modules), w.Tiers)) } } @@ -443,8 +455,12 @@ var ignoredInTree = map[string]bool{".git": true, "vendor": true, "node_modules" // read by this controller's strict parser — the one the mesh runs, for a change to a catalogue — resolved // with stand-in builds, and read again as registration reads a built one. A module the repository held // and the tree no longer has is removed. Answers the shelf and what the tree itself fails. +// +// `scope` is the directories of the modules a merge of the change would rebuild or add, as the planner says +// (scopeOfReach); a definition elsewhere in the tree is left as the mesh holds it. Nil judges every one. func shelfWithChange(base map[string]catalogue.Manifest, sources map[string]inventory.Source, f snapshot.Facts, - repository, tree string, moved map[string]string) (map[string]catalogue.Manifest, []string, error) { + repository, tree string, moved map[string]string, scope map[string]bool) (map[string]catalogue.Manifest, []string, error) { + inScope := func(dir string) bool { return scope == nil || scope[dir] } out := make(map[string]catalogue.Manifest, len(base)) for k, m := range base { out[k] = m @@ -477,16 +493,23 @@ func shelfWithChange(base map[string]catalogue.Manifest, sources map[string]inve } m, err := catalogue.ParseManifest(raw) if err != nil { - failures = append(failures, fmt.Sprintf("%s: the controller judging this (%s) refuses it — %s", - orRoot(dir), version, oneLine(err.Error()))) + if inScope(dir) { + failures = append(failures, fmt.Sprintf("%s: the controller judging this (%s) refuses it — %s", + orRoot(dir), version, oneLine(err.Error()))) + } return nil } if was, twice := found[m.Module]; twice { - failures = append(failures, fmt.Sprintf("%s and %s both define %s: two definitions of one module", - orRoot(was), orRoot(dir), m.Module)) + if inScope(dir) || inScope(was) { + failures = append(failures, fmt.Sprintf("%s and %s both define %s: two definitions of one module", + orRoot(was), orRoot(dir), m.Module)) + } return nil } found[m.Module] = dir + if !inScope(dir) { + return nil + } if s, held := sources[m.Module]; held && s.Repository != "" && !sameRepository(s.Repository, link.SourceMoved{Owner: ownerOf(repository), Repo: repoOf(repository)}) { failures = append(failures, fmt.Sprintf("%s defines %s, which the mesh builds from %s: two definitions "+ @@ -538,6 +561,9 @@ func shelfWithChange(base map[string]catalogue.Manifest, sources map[string]inve if _, still := found[name]; still { continue } + if !inScope(strings.Trim(s.Path, "/")) { + continue + } delete(out, name) moved[name] = "removed" if on := running[name]; len(on) > 0 { @@ -961,25 +987,32 @@ func standInKey() (string, error) { return base64.StdEncoding.EncodeToString(k.PublicKey().Bytes()), nil } -// mergeWidth is what a merge of the change would rebuild, computed the way the merge handler computes -// it, from the modules, sources and edges the snapshot carries. -func rebuildWidth(f snapshot.Facts, repository string, paths []string, tree string) (mergeWidthOf, error) { +// reachOfChange is what a merge of the change would move and build — **the planner's own answer** +// (reachOfMerge), asked of the modules, sources, reads and edges the snapshot carries, never a mapping of +// the gate's own: what a merge touches and what follows it are decided in one place, so the gate and the +// plan a merge makes cannot disagree (novox/hq ADR 0238). +func reachOfChange(f snapshot.Facts, repository string, paths []string, tree string) (mergeReach, error) { owner, repo, found := strings.Cut(repository, "/") if !found { - return mergeWidthOf{}, fmt.Errorf("%q is not owner/repository", repository) + return mergeReach{}, fmt.Errorf("%q is not owner/repository", repository) } m := link.SourceMoved{Owner: owner, Repo: repo, Base: "main", Commit: "gate", Paths: paths} // Which changed directories hold a module, read from the change's tree as the forge's announcer reads - // them at the merge commit (issue 278): a file inside one is that module's business, held or not. + // them at the head (issue 278): what the planner is told of a module the mesh does not hold yet. if tree != "" { m.ModuleDirs, m.ModuleDirsSaid = moduleDirsIn(tree, paths), true + for _, p := range paths { + if _, err := os.Stat(filepath.Join(tree, p)); os.IsNotExist(err) { + m.Removed = append(m.Removed, p) + } + } } var entries []inventory.Entry read := map[string][]inventory.ReadRepository{} for _, mod := range f.Modules { var manifest catalogue.Manifest if err := json.Unmarshal(mod.Manifest, &manifest); err != nil { - return mergeWidthOf{}, err + return mergeReach{}, err } entries = append(entries, inventory.Entry{Manifest: manifest, Provided: mod.Provided, Source: inventory.Source{Repository: mod.Repository, Path: mod.Path, BuiltFrom: mod.Commit}}) @@ -991,52 +1024,39 @@ func rebuildWidth(f snapshot.Facts, repository string, paths []string, tree stri for _, e := range f.Edges { edges = append(edges, inventory.Edge{From: e.From, To: e.To, Kind: e.Kind}) } - var from []inventory.Entry - for _, e := range entries { - if !e.Provided && sourceIs(e.Source, m) { - from = append(from, e) + return reachOfMerge(m, entries, read, edges), nil +} + +// widthOf is how wide the rebuild of a reach is. +func widthOf(r mergeReach) mergeWidthOf { + w := mergeWidthOf{Tiers: len(r.Plan.Tiers), Unread: r.Unread} + for name := range r.Plan.Modules { + w.Modules = append(w.Modules, name) + } + sort.Strings(w.Modules) + return w +} + +// scopeOfReach is the directories, in the change's repository, of the modules a reach rebuilds or adds; +// nil — every definition judged — when there is no reach. +func scopeOfReach(r *mergeReach, repository string) map[string]bool { + if r == nil { + return nil + } + scope := map[string]bool{} + same := link.SourceMoved{Owner: ownerOf(repository), Repo: repoOf(repository)} + for _, e := range append(append([]inventory.Entry{}, r.Touched...), r.Deleted...) { + if sameRepository(e.Source.Repository, same) { + scope[strings.Trim(e.Source.Path, "/")] = true } } - touched := whatTheMergeTouched(from, entries, m) - var names []string - for _, e := range touched { - names = append(names, e.Manifest.Module) - } - for _, e := range entries { - if readsFrom(read[e.Manifest.Module], m) && !slices.Contains(names, e.Manifest.Module) { - names = append(names, e.Manifest.Module) + for _, d := range r.Added { + if d == "." { + d = "" } + scope[d] = true } - w := mergeWidthOf{} - if len(names) > 0 { - p := planOfMerge(m, names, edges) - w.Tiers = len(p.Tiers) - for name := range p.Modules { - w.Modules = append(w.Modules, name) - } - sort.Strings(w.Modules) - } - // The paths no build reads: in no directory of a module the mesh holds from this repository, or one - // the tree holds. - var dirs []string - for _, e := range from { - if d := strings.Trim(e.Source.Path, "/"); d != "" { - dirs = append(dirs, d+"/") - } - } - for _, d := range m.ModuleDirs { - dirs = append(dirs, strings.Trim(d, "/")+"/") - } - for _, p := range paths { - inModule := false - for _, d := range dirs { - inModule = inModule || strings.HasPrefix(p, d) - } - if !inModule && len(dirs) > 0 { - w.Unread = append(w.Unread, p) - } - } - return w, nil + return scope } // moduleDirsIn are the directories above the changed paths, never the root, that hold a module.json in diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 2e3e611..0ad9255 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -1357,18 +1357,20 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string for _, name := range modules { named[name] = true } - for _, e := range entries { - switch { - case named[e.Manifest.Module]: - from = append(from, e) - case len(named) == 0 && sourceIs(e.Source, m): - from = append(from, e) - case readsFrom(read[e.Manifest.Module], m): - packaging = append(packaging, e) - } - } if len(named) == 0 { - from = whatTheMergeTouched(from, entries, m) + // The changed files: the planner's own answer, as the merge handler, the merge gate and a pull + // request's check ask it (novox/hq ADR 0238). + r := reachOfMerge(m, entries, read, nil) + from, packaging = append(append([]inventory.Entry{}, r.Touched...), r.Deleted...), r.Packaging + } else { + for _, e := range entries { + switch { + case named[e.Manifest.Module]: + from = append(from, e) + case readsFrom(read[e.Manifest.Module], m): + packaging = append(packaging, e) + } + } } moved := append(append([]inventory.Entry{}, from...), packaging...) if len(moved) == 0 { diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index f9094f9..5919a00 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -6,6 +6,7 @@ import ( "flag" "fmt" "regexp" + "slices" "sort" "strings" "sync" @@ -616,14 +617,24 @@ func lastLookAt(entries []inventory.Entry, m link.SourceMoved) time.Time { } // whatTheMergeTouched narrows the modules built from a repository to the ones the merge changed: **a -// changed file touches exactly the modules whose build reads it** (novox/hq issue 280, ADR 0237 as amended). +// changed file touches exactly the modules whose build reads it** (novox/hq issue 280, ADR 0238). It is +// touchedBy's first answer; touchedBy is the one place the mesh maps a changed file onto its modules. +func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved) []inventory.Entry { + touched, _, _ := touchedBy(candidates, known, m) + return touched +} + +// touchedBy maps a merge's changed files onto the mesh's modules — **the one place it is done**, for the +// merge handler, the release planner's what-if, the merge gate and a pull request's check alike (novox/hq +// ADR 0238), so planning and gating cannot disagree about what a change touches. // -// What a build reads is the module's own directory — the builder clones the repository and builds within -// that directory alone: the manifest, the recipes, the bundles' sources, the Docker context — or the whole -// repository for a module built from its root. A second repository a recipe packages (an artifact's -// `context`) is read too, and that is the build record's `read`, answered by readsFrom beside this. So a -// changed file inside a module's directory is that module's; a file in no module's directory — a script -// at the root, a README, CI configuration, a directory of notes — is read by no build and touches nothing. +// **A changed file touches exactly the modules whose build reads it.** What a build reads is the module's +// own directory — the builder clones the repository and builds within that directory alone: the manifest, +// the recipes, the bundles' sources, the Docker context — or the whole repository for a module built from +// its root. A second repository a recipe packages (an artifact's `context`) is read too; that is the build +// record's `read`, answered by readsFrom in mergeCandidates. So a changed file inside a module's directory +// is that module's; a file in no module's directory — a script at the root, a README, CI configuration — +// is read by no build and touches nothing: it is answered as `unread`. // // **It used to be read as shared code**, rebuilding everything built from the repository, on the theory // that a root file might be a build input (04-ISSUES/131). No build reads one: the theory cost a rebuild @@ -631,23 +642,131 @@ func lastLookAt(entries []inventory.Entry, m link.SourceMoved) time.Time { // held by no machine (278) and a new module's directory (252), each patched as an exception to a rule that // was wrong. Rebuilding too much is not the safe direction when every rebuild is a rollout. // -// Nothing said about the files, or not all of them said, is still everything: what is not known cannot be -// narrowed. `known` is kept for the callers; which directories hold a module is no longer needed to read a -// change, because a file is a module's only by being inside it. -func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved) []inventory.Entry { - _ = known - if len(m.Paths) == 0 || m.PathsTruncated { - return candidates - } - var out []inventory.Entry - for _, e := range candidates { - if strings.Trim(e.Source.Path, "/") == "" || anyInside(m.Paths, e.Source.Path) { - out = append(out, e) +// `added` is the directories the change holds a module in that no module of this repository is known +// from — said by the announcer at the head (issue 278), or a module.json among the changed files — `.` +// for the root: a new module, which a check judges before it merges and a merge does not build. +// +// Nothing said about the files, or not all of them said, is still everything: what is not known cannot +// be narrowed. +func touchedBy(candidates, known []inventory.Entry, m link.SourceMoved) (touched []inventory.Entry, added, unread []string) { + knownDirs := map[string]bool{} + for _, e := range known { + if !e.Provided && sameRepository(e.Source.Repository, m) { + knownDirs[strings.Trim(e.Source.Path, "/")] = true } } + newDir := map[string]bool{} + for _, d := range saidModuleDirs(m) { + if !knownDirs[d] { + newDir[d] = true + } + } + for _, p := range m.Paths { + p = strings.Trim(p, "/") + if p != moduleManifestFile && !strings.HasSuffix(p, "/"+moduleManifestFile) { + continue + } + d := strings.TrimSuffix(strings.TrimSuffix(p, moduleManifestFile), "/") + if !knownDirs[d] { + if d == "" { + d = "." + } + newDir[d] = true + } + } + for d := range newDir { + added = append(added, d) + } + sort.Strings(added) + + if len(m.Paths) == 0 || m.PathsTruncated { + return candidates, added, nil + } + for _, e := range candidates { + if strings.Trim(e.Source.Path, "/") == "" || anyInside(m.Paths, e.Source.Path) { + touched = append(touched, e) + } + } + for _, p := range m.Paths { + read := newDir["."] + for _, e := range candidates { + read = read || strings.Trim(e.Source.Path, "/") == "" || inside(p, e.Source.Path) + } + for d := range newDir { + read = read || inside(p, d) + } + if !read { + unread = append(unread, p) + } + } + return touched, added, unread +} + +// mergeReach is what a merge of a repository's branch reaches, as the planner reckons it: the planner's +// one answer, given to the release planner's what-if, the merge gate and a pull request's check (novox/hq +// ADR 0238). The merge handler acts on the same pieces — mergeCandidates, touchedBy, splitDeleted, +// planOfMerge — as it goes. +type mergeReach struct { + // Touched are the modules built from the repository and branch whose build reads a changed file, and + // Deleted those of them whose manifest the merge removes. + Touched, Deleted []inventory.Entry + // Packaging are the modules whose build packages source from the repository. + Packaging []inventory.Entry + // Already counts the modules built from the repository that are built from this very commit. + Already int + // Added are the directories the change holds a new module in; Unread the changed files no build reads. + Added, Unread []string + // Plan is what moves and everything that follows it along the catalogue's dependencies, tiered — + // what a merge would build. Empty when nothing moves. + Plan inventory.Plan +} + +// Moved are the modules the merge moves itself, by name: touched, deleted and packaging. +func (r mergeReach) Moved() []string { + var out []string + for _, group := range [][]inventory.Entry{r.Touched, r.Deleted, r.Packaging} { + for _, e := range group { + out = append(out, e.Manifest.Module) + } + } + sort.Strings(out) + return slices.Compact(out) +} + +// Dependents are the modules the plan builds after the moved ones because they stand on them. +func (r mergeReach) Dependents() []string { + moved := map[string]bool{} + for _, name := range r.Moved() { + moved[name] = true + } + var out []string + for name := range r.Plan.Modules { + if !moved[name] { + out = append(out, name) + } + } + sort.Strings(out) return out } +// reachOfMerge is what a merge of these changed files into this branch would move and build, without +// acting: the merge handler's own judgement and the release plan's dependency walk. +func reachOfMerge(m link.SourceMoved, entries []inventory.Entry, read map[string][]inventory.ReadRepository, + edges []inventory.Edge) mergeReach { + from, packaging, already := mergeCandidates(m, entries, read) + touched, added, unread := touchedBy(from, entries, m) + kept, deleted := splitDeleted(touched, m) + r := mergeReach{Touched: kept, Deleted: deleted, Packaging: packaging, Already: already, Added: added, Unread: unread} + var building []string + for _, e := range append(append([]inventory.Entry{}, kept...), packaging...) { + building = append(building, e.Manifest.Module) + } + if len(building) > 0 { + r.Plan = planOfMerge(m, building, edges) + } + return r +} + // saidModuleDirs is the directories the announcer says hold a module at the merge commit (novox/hq // issue 278): a changed file in one of them is that module's business and nobody else's — the // catalogue's reference module, held by no machine, whose code a merge touched beside its manifest, @@ -674,16 +793,6 @@ func inside(path, dir string) bool { return path == dir || strings.HasPrefix(path, dir+"/") } -// insideAny is whether a changed file is in any of these directories. -func insideAny(path string, dirs []string) bool { - for _, dir := range dirs { - if inside(path, dir) { - return true - } - } - return false -} - // anyInside is whether any of these changed files is in a directory. func anyInside(paths []string, dir string) bool { for _, p := range paths { diff --git a/internal/link/build.go b/internal/link/build.go index 6557d3e..9203b2f 100644 --- a/internal/link/build.go +++ b/internal/link/build.go @@ -111,6 +111,9 @@ type CheckRequest struct { // the gate runs**, not the repository. With neither, only the repository's own merge-check.sh runs. Modules []string `json:"modules,omitempty"` New []string `json:"new,omitempty"` + // Dependents are the modules a merge would build after Modules because they stand on them: the + // planner's dependency walk, said on the pull request. + Dependents []string `json:"dependents,omitempty"` // Manifests are the touched modules' manifests in the change's tree, by path from its root: what the // gate puts through `module check`. Manifests []string `json:"manifests,omitempty"` diff --git a/internal/link/events.go b/internal/link/events.go index e97e50b..a7a6fcb 100644 --- a/internal/link/events.go +++ b/internal/link/events.go @@ -221,6 +221,9 @@ type PullUpdated struct { // Paths are the files the pull request changes; PathsTruncated says there were more. Paths []string `json:"paths,omitempty"` PathsTruncated bool `json:"paths_truncated,omitempty"` + // Removed are the files among Paths the change deletes: a module whose manifest is among them is one + // the merge would remove. + Removed []string `json:"removed,omitempty"` // ModuleDirs are the directories above the changed files that hold a `module.json` at the head, read // as the merge announcer reads them at a merge commit (issue 278); ModuleDirsSaid says it looked. // What the controller finds a module the graph does not hold yet with: a directory the change adds a @@ -263,8 +266,10 @@ type CheckLayer struct { Verdict string `json:"verdict"` Summary string `json:"summary"` // Modules are, for the gate, the modules of the mesh's graph the change touches — `new: