Merge pull request 'A plan says why each module is in it (hq ADR 0267, issue 363)' (#196) from fix/0267-a-plan-says-why-each-module-is-in-it into main
This commit was merged in pull request #196.
This commit is contained in:
@@ -4,6 +4,7 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
|
"fmt"
|
||||||
"hash/fnv"
|
"hash/fnv"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
@@ -11,6 +12,7 @@ import (
|
|||||||
"strings"
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
"github.com/novox/mesh-controller/internal/inventory"
|
"github.com/novox/mesh-controller/internal/inventory"
|
||||||
"github.com/novox/mesh-controller/internal/link"
|
"github.com/novox/mesh-controller/internal/link"
|
||||||
@@ -208,7 +210,9 @@ func TestARollbackNeverPutsBackABuildFromAnotherRepository(t *testing.T) {
|
|||||||
if _, _, err := takeIn(ctx, inv, fork); !errors.Is(err, errNotItsSource) {
|
if _, _, err := takeIn(ctx, inv, fork); !errors.Is(err, errNotItsSource) {
|
||||||
t.Fatalf("the fork's build was taken in: %v", err)
|
t.Fatalf("the fork's build was taken in: %v", err)
|
||||||
}
|
}
|
||||||
failed := onTrunk("build-1791600000000000000", "novox/mesh-catalog", "git", "modules/sudo",
|
// Asked after the registered build, whenever the test runs: an id naming a fixed moment read as older
|
||||||
|
// than the registered build once the clock passed it (2026-10-10 02:40 UTC), and the test failed on main.
|
||||||
|
failed := onTrunk(fmt.Sprintf("build-%d", time.Now().Add(time.Hour).UnixNano()), "novox/mesh-catalog", "git", "modules/sudo",
|
||||||
map[string]any{"module": "sudo", "version": "2"})
|
map[string]any{"module": "sudo", "version": "2"})
|
||||||
failed.Commit = "badbadbad0123456"
|
failed.Commit = "badbadbad0123456"
|
||||||
if _, _, err := takeIn(ctx, inv, failed); err != nil {
|
if _, _, err := takeIn(ctx, inv, failed); err != nil {
|
||||||
|
|||||||
@@ -414,7 +414,9 @@ func gatherFacts(ctx context.Context, open *stores, busVersion string) (snapshot
|
|||||||
// The module's own build source is said apart: a gate that predates it would read an own
|
// The module's own build source is said apart: a gate that predates it would read an own
|
||||||
// entry among Reads as a context of its own repository.
|
// entry among Reads as a context of its own repository.
|
||||||
if r.Own {
|
if r.Own {
|
||||||
mod.Sources = append(mod.Sources, snapshot.BuildSource{Own: true, Paths: r.Paths})
|
if len(r.Paths) > 0 {
|
||||||
|
mod.Sources = append(mod.Sources, snapshot.BuildSource{Own: true, Paths: r.Paths})
|
||||||
|
}
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
mod.Reads = append(mod.Reads, snapshot.RepositoryName(r.Repository))
|
mod.Reads = append(mod.Reads, snapshot.RepositoryName(r.Repository))
|
||||||
|
|||||||
@@ -626,3 +626,55 @@ func TestTheGatePlansFromTheBuildSourcesTheSnapshotCarries(t *testing.T) {
|
|||||||
t.Errorf("a snapshot without build sources: the gate planned %q for a README", got)
|
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"},
|
||||||
|
{"a root manifest is not in a module's directory", gitea, nil, merge("mesh-catalog", "module.json", "modules/gitea/x.ts"),
|
||||||
|
"read whole: no build source recorded, so its directory is its build source; e.g. modules/gitea/x.ts"},
|
||||||
|
{"a context on another branch says nothing of this one", agent, map[string][]inventory.ReadRepository{"build-agent": {
|
||||||
|
{Repository: "novox/mesh-controller", Ref: "release", Paths: []string{"internal/link/"}},
|
||||||
|
{Repository: "novox/mesh-controller", Ref: "main"}}}, merge("mesh-controller", "internal/link/handacts.go"),
|
||||||
|
"read whole: no build source recorded, so every file of novox/mesh-controller, which its build context is, is its build source"},
|
||||||
|
{"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)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -1567,6 +1567,7 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
var from, packaging []inventory.Entry
|
var from, packaging []inventory.Entry
|
||||||
|
deleted := map[string]bool{}
|
||||||
named := map[string]bool{}
|
named := map[string]bool{}
|
||||||
for _, name := range modules {
|
for _, name := range modules {
|
||||||
named[name] = true
|
named[name] = true
|
||||||
@@ -1576,6 +1577,9 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string
|
|||||||
// request's check ask it (novox/hq ADR 0238).
|
// request's check ask it (novox/hq ADR 0238).
|
||||||
r := reachOfMerge(m, entries, read, nil)
|
r := reachOfMerge(m, entries, read, nil)
|
||||||
from, packaging = append(append([]inventory.Entry{}, r.Touched...), r.Deleted...), r.Packaging
|
from, packaging = append(append([]inventory.Entry{}, r.Touched...), r.Deleted...), r.Packaging
|
||||||
|
for _, e := range r.Deleted {
|
||||||
|
deleted[e.Manifest.Module] = true
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
for _, e := range entries {
|
for _, e := range entries {
|
||||||
switch {
|
switch {
|
||||||
@@ -1602,6 +1606,18 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string
|
|||||||
}
|
}
|
||||||
p := planOfMerge(m, names, edges)
|
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))
|
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)
|
||||||
|
if deleted[e.Manifest.Module] {
|
||||||
|
why[e.Manifest.Module] = "its manifest is removed: deleted at its source, forgotten where nothing holds it, not built"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
rolls := map[string]string{}
|
rolls := map[string]string{}
|
||||||
for i, tier := range p.Tiers {
|
for i, tier := range p.Tiers {
|
||||||
fmt.Printf(" tier %d\n", i)
|
fmt.Printf(" tier %d\n", i)
|
||||||
@@ -1619,19 +1635,19 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string
|
|||||||
rolls[name] = how
|
rolls[name] = how
|
||||||
}
|
}
|
||||||
fmt.Printf(" %-22s %s\n", 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) {
|
if hasCycle(p.Tiers, edges) {
|
||||||
fmt.Println(" the last tier depends on itself and would be built together, in no order")
|
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
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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",
|
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 "))
|
m.Owner, m.Repo, m.Base, m.Commit, plan.ID, len(plan.Modules), len(plan.Tiers), strings.Join(tiers, "\n "))
|
||||||
if len(packaging) > 0 {
|
// Why each is in it (issue 363): the files of its build source the merge changed, or why it is read whole.
|
||||||
var also []string
|
for _, e := range moved {
|
||||||
for _, e := range packaging {
|
fmt.Printf(" %s: %s\n", e.Manifest.Module, whyMoved(e, read[e.Manifest.Module], m))
|
||||||
also = append(also, e.Manifest.Module)
|
}
|
||||||
}
|
for _, e := range packaging {
|
||||||
fmt.Printf(" %s package source from it, so they are rebuilt and their own source record "+
|
fmt.Printf(" %s reads %s/%s through its build context: its own source record is left where it is\n",
|
||||||
"is left where it is\n", strings.Join(also, ", "))
|
e.Manifest.Module, m.Owner, m.Repo)
|
||||||
}
|
}
|
||||||
if plan.Waiting() {
|
if plan.Waiting() {
|
||||||
plan.Note = waitingNote(plan)
|
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.
|
// historyMargin is how far a packaging module's last look is taken back before a merge is history for it.
|
||||||
const historyMargin = time.Minute
|
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 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) || (r.Ref != "" && r.Ref != m.Base) {
|
||||||
|
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.
|
// lookedOf is when a module packaging another repository was last looked at, as readForPlanning says.
|
||||||
func lookedOf(read []inventory.ReadRepository) time.Time {
|
func lookedOf(read []inventory.ReadRepository) time.Time {
|
||||||
var at 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
|
// 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
|
// 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.
|
// 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 {
|
for name, rs := range read {
|
||||||
var since time.Time
|
var since time.Time
|
||||||
for _, r := range rs {
|
for _, r := range rs {
|
||||||
@@ -772,7 +831,7 @@ func staleIn(read map[string][]inventory.ReadRepository, plans []inventory.Plan)
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if p.Open() || p.Created.After(since) {
|
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
|
var kept []inventory.ReadRepository
|
||||||
|
overtaken, isStale := stale[name]
|
||||||
for _, r := range rs {
|
for _, r := range rs {
|
||||||
if stale[name] {
|
if isStale {
|
||||||
if r.Own {
|
if r.Own {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
@@ -848,6 +908,10 @@ func planningView(read map[string][]inventory.ReadRepository, plans []inventory.
|
|||||||
r.Looked, r.LookedAt = looked, commits
|
r.Looked, r.LookedAt = looked, commits
|
||||||
kept = append(kept, r)
|
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
|
out[name] = kept
|
||||||
}
|
}
|
||||||
return out
|
return out
|
||||||
|
|||||||
@@ -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
|
// 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.
|
// of one of them is history for the module whatever the clocks say. Never stored.
|
||||||
LookedAt []string `json:"-"`
|
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
|
// 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
|
sources = nil
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if n, known := newest[module]; known && n.failed && n.at.After(built) {
|
whole := ""
|
||||||
sources = nil
|
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 {
|
for k := range of {
|
||||||
of[k].Built, of[k].Looked = built, newest[module].at
|
of[k].Built, of[k].Looked = built, newest[module].at
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -243,7 +243,9 @@ func TestAFailedNewerBuildLeavesTheBuildSourceStale(t *testing.T) {
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
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)
|
t.Fatalf("after a newer failed build the proxy reads as %+v", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user