diff --git a/cmd/mesh-controller/planner_rules_test.go b/cmd/mesh-controller/planner_rules_test.go index f47cded2..3e07a6d1 100644 --- a/cmd/mesh-controller/planner_rules_test.go +++ b/cmd/mesh-controller/planner_rules_test.go @@ -626,3 +626,49 @@ func TestTheGatePlansFromTheBuildSourcesTheSnapshotCarries(t *testing.T) { t.Errorf("a snapshot without build sources: the gate planned %q for a README", got) } } + +// **A plan says why each module is in it** (novox/hq ADR 0267, issue 363): the files of its build source the +// merge changed, or why it is read whole — never that it packages a repository as though that moved it. +func TestAPlanSaysWhyEachModuleIsInIt(t *testing.T) { + const controllerRepo = "http://forge.internal:20000/novox/mesh-controller.git" + controller := fromRepo("mesh-controller", controllerRepo, "") + agent := fromRepo("build-agent", "http://forge.internal:20000/novox/mesh-catalog.git", "modules/build-agent") + gitea := fromRepo("gitea", "http://forge.internal:20000/novox/mesh-catalog.git", "modules/gitea") + built := time.Date(2026, 10, 10, 12, 26, 0, 0, time.UTC) + read := map[string][]inventory.ReadRepository{ + "mesh-controller": {{Own: true, Paths: []string{"module.json", "cmd/mesh-controller/", "internal/link/"}, Built: built}}, + "build-agent": { + {Repository: "novox/mesh-controller", Ref: "main", Paths: []string{"cmd/mesh-builder/", "internal/link/"}, Built: built}, + {Own: true, Paths: []string{"modules/build-agent/module.json"}, Built: built}, + }, + } + merge := func(repo string, paths ...string) link.SourceMoved { + return link.SourceMoved{Owner: "novox", Repo: repo, Base: "main", Paths: paths} + } + open := []inventory.Plan{{ID: "plan-1", State: inventory.PlanBuilding, Created: built.Add(time.Minute), + Modules: map[string]*inventory.PlanModule{"build-agent": {State: "asked"}}}} + for _, c := range []struct { + what string + e inventory.Entry + read map[string][]inventory.ReadRepository + m link.SourceMoved + says string + }{ + {"the controller's own closure", controller, read, merge("mesh-controller", "README.md", "internal/link/handacts.go"), + "its build source changed: 1 changed file(s) in it, e.g. internal/link/handacts.go"}, + {"the build seat's closure, through its context", agent, read, merge("mesh-controller", "internal/link/handacts.go"), + "its build source in novox/mesh-controller changed: 1 changed file(s) in it, e.g. internal/link/handacts.go"}, + {"an open plan has yet to build it", agent, planningView(read, open), merge("mesh-controller", "cmd/mesh-controller/main.go"), + "read whole: plan plan-1 has not built it yet, so every file of novox/mesh-controller, which its build context is, is its build source"}, + {"nothing recorded, built from its root", controller, nil, merge("mesh-controller", "README.md"), + "read whole: no build source recorded, so every file of its repository is its build source"}, + {"nothing recorded, in its directory", gitea, nil, merge("mesh-catalog", "modules/gitea/index.ts"), + "read whole: no build source recorded, so its directory is its build source; e.g. modules/gitea/index.ts"}, + {"files not all said", controller, read, link.SourceMoved{Owner: "novox", Repo: "mesh-controller", PathsTruncated: true, + Paths: []string{"x"}}, "read whole: the merge's changed files were not all said"}, + } { + if got := whyMoved(c.e, c.read[c.e.Manifest.Module], c.m); got != c.says { + t.Errorf("%s:\n said %q\n wanted %q", c.what, got, c.says) + } + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 05fc4773..e21b361c 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -1602,6 +1602,15 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string } p := planOfMerge(m, names, edges) fmt.Printf("a merge of %s would build %d module(s) in %d tier(s):\n", repository, len(p.Modules), len(p.Tiers)) + why := map[string]string{} + for _, e := range packaging { + why[e.Manifest.Module] = whyMoved(e, read[e.Manifest.Module], m) + } + if len(named) == 0 { + for _, e := range from { + why[e.Manifest.Module] = whyMoved(e, read[e.Manifest.Module], m) + } + } rolls := map[string]string{} for i, tier := range p.Tiers { fmt.Printf(" tier %d\n", i) @@ -1619,19 +1628,19 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string rolls[name] = how } fmt.Printf(" %-22s %s\n", name, how) + reason := why[name] + switch { + case reason == "" && len(named) > 0 && named[name]: + reason = "named" + case reason == "": + reason = "it stands on a module the merge moves" + } + fmt.Printf(" %-22s why: %s\n", "", reason) } } if hasCycle(p.Tiers, edges) { fmt.Println(" the last tier depends on itself and would be built together, in no order") } - if len(packaging) > 0 { - var also []string - for _, e := range packaging { - also = append(also, e.Manifest.Module) - } - fmt.Printf(" %s package source from %s, so they are rebuilt without their own source moving\n", - strings.Join(also, ", "), repository) - } return nil } diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index dc089f96..74261bc8 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -465,13 +465,13 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { } fmt.Printf("%s/%s merged into %s (%.8s); plan %s, %d module(s) in %d tier(s)\n %s\n", m.Owner, m.Repo, m.Base, m.Commit, plan.ID, len(plan.Modules), len(plan.Tiers), strings.Join(tiers, "\n ")) - if len(packaging) > 0 { - var also []string - for _, e := range packaging { - also = append(also, e.Manifest.Module) - } - fmt.Printf(" %s package source from it, so they are rebuilt and their own source record "+ - "is left where it is\n", strings.Join(also, ", ")) + // Why each is in it (issue 363): the files of its build source the merge changed, or why it is read whole. + for _, e := range moved { + fmt.Printf(" %s: %s\n", e.Manifest.Module, whyMoved(e, read[e.Manifest.Module], m)) + } + for _, e := range packaging { + fmt.Printf(" %s reads %s/%s through its build context: its own source record is left where it is\n", + e.Manifest.Module, m.Owner, m.Repo) } if plan.Waiting() { plan.Note = waitingNote(plan) @@ -597,6 +597,63 @@ func lookedAtCommit(read []inventory.ReadRepository, commit string) bool { // historyMargin is how far a packaging module's last look is taken back before a merge is history for it. const historyMargin = time.Minute +// whyMoved is why a merge moves a module, as a plan says it (novox/hq ADR 0267, issue 363): the changed +// files in its build source, or why it is read whole. For a module built from the merged repository or one +// whose build context is that repository; a dependent is in a plan for what it stands on. +func whyMoved(e inventory.Entry, read []inventory.ReadRepository, m link.SourceMoved) string { + if len(m.Paths) == 0 || m.PathsTruncated { + return "read whole: the merge's changed files were not all said" + } + whole := "no build source recorded" + for _, r := range read { + if r.Own && r.Whole != "" { + whole = r.Whole + } + } + held := func(paths []string) []string { + var in []string + for _, p := range m.Paths { + if builder.SourceHolds(paths, p) { + in = append(in, p) + } + } + return in + } + changed := func(where string, in []string) string { + return fmt.Sprintf("its build source%s changed: %d changed file(s) in it, e.g. %s", where, len(in), in[0]) + } + if sameRepository(e.Source.Repository, m) { + if own := ownSource(read); own != nil { + if in := held(own); len(in) > 0 { + return changed("", in) + } + } + dir := strings.Trim(e.Source.Path, "/") + if dir == "" { + return "read whole: " + whole + ", so every file of its repository is its build source" + } + for _, p := range m.Paths { + if p == moduleManifestFile || inside(p, dir) { + return "read whole: " + whole + ", so its directory is its build source; e.g. " + p + } + } + return "read whole: " + whole + } + for _, r := range read { + if r.Own || !sameRepository(r.Repository, m) { + continue + } + if len(r.Paths) > 0 { + if in := held(r.Paths); len(in) > 0 { + return changed(" in "+m.Owner+"/"+m.Repo, in) + } + continue + } + return "read whole: " + whole + ", so every file of " + m.Owner + "/" + m.Repo + ", which its build context is, is its build source" + } + return "read whole: " + whole +} + // lookedOf is when a module packaging another repository was last looked at, as readForPlanning says. func lookedOf(read []inventory.ReadRepository) time.Time { var at time.Time @@ -757,8 +814,10 @@ func readsFile(e inventory.Entry, read []inventory.ReadRepository, p string) boo // or superseded. What such a module is built from is changing, or changed without a build to say so: a merge // that added an import to it, and a later one changing only what that import names, would otherwise move // nothing. Each is read whole, as before, until a build of it works again. -func staleIn(read map[string][]inventory.ReadRepository, plans []inventory.Plan) map[string]bool { - stale := map[string]bool{} +// +// Each is answered with the plan that overtook it, for saying why it is read whole. +func staleIn(read map[string][]inventory.ReadRepository, plans []inventory.Plan) map[string]string { + stale := map[string]string{} for name, rs := range read { var since time.Time for _, r := range rs { @@ -772,7 +831,7 @@ func staleIn(read map[string][]inventory.ReadRepository, plans []inventory.Plan) continue } if p.Open() || p.Created.After(since) { - stale[name] = true + stale[name] = p.ID } } } @@ -838,8 +897,9 @@ func planningView(read map[string][]inventory.ReadRepository, plans []inventory. } } var kept []inventory.ReadRepository + overtaken, isStale := stale[name] for _, r := range rs { - if stale[name] { + if isStale { if r.Own { continue } @@ -848,6 +908,10 @@ func planningView(read map[string][]inventory.ReadRepository, plans []inventory. r.Looked, r.LookedAt = looked, commits kept = append(kept, r) } + if isStale { + kept = append(kept, inventory.ReadRepository{Own: true, Whole: "plan " + overtaken + " has not built it yet", + Looked: looked, LookedAt: commits}) + } out[name] = kept } return out diff --git a/internal/inventory/builds.go b/internal/inventory/builds.go index 73f6020a..c85d9f42 100644 --- a/internal/inventory/builds.go +++ b/internal/inventory/builds.go @@ -98,6 +98,9 @@ type ReadRepository struct { // LookedAt are the merge commits a plan that built the module, or is building it, answered: a merge // of one of them is history for the module whatever the clocks say. Never stored. LookedAt []string `json:"-"` + // Whole is why the module is read whole though a build source may have been recorded — a newer build + // failed, or a plan has not built it yet — said on an Own entry with no Paths. Never stored. + Whole string `json:"-"` } // BuildSource is the build source a build read in one repository (novox/hq ADR 0267): Repository and Ref @@ -408,10 +411,15 @@ func (i *Inventory) ReadRepositories(ctx context.Context) (map[string][]ReadRepo sources = nil } } - if n, known := newest[module]; known && n.failed && n.at.After(built) { - sources = nil + whole := "" + if n, known := newest[module]; known && n.failed && n.at.After(built) && len(sources) > 0 { + sources, whole = nil, "a newer build of it failed" } - if of = WithBuildSources(of, sources); len(of) > 0 { + of = WithBuildSources(of, sources) + if whole != "" { + of = append(of, ReadRepository{Own: true, Whole: whole}) + } + if len(of) > 0 { for k := range of { of[k].Built, of[k].Looked = built, newest[module].at } diff --git a/internal/inventory/builds_test.go b/internal/inventory/builds_test.go index f2e0fe9b..8608ff0b 100644 --- a/internal/inventory/builds_test.go +++ b/internal/inventory/builds_test.go @@ -243,7 +243,9 @@ func TestAFailedNewerBuildLeavesTheBuildSourceStale(t *testing.T) { if err != nil { t.Fatal(err) } - if got := read["route-proxy"]; len(got) != 1 || got[0].Paths != nil || got[0].Own || !got[0].Looked.After(got[0].Built) { + got := read["route-proxy"] + if len(got) != 2 || got[0].Paths != nil || got[0].Own || !got[0].Looked.After(got[0].Built) || + !got[1].Own || got[1].Paths != nil || got[1].Whole != "a newer build of it failed" { t.Fatalf("after a newer failed build the proxy reads as %+v", got) } }