diff --git a/cmd/mesh-controller/build.go b/cmd/mesh-controller/build.go index 6f1c0cd5..5b772e91 100644 --- a/cmd/mesh-controller/build.go +++ b/cmd/mesh-controller/build.go @@ -166,6 +166,10 @@ func buildFrom(result link.BuildResult) inventory.Build { for _, r := range result.Read { kept.Read = append(kept.Read, inventory.ReadRepository{Repository: r.Repository, Ref: r.Ref}) } + // What it was made from, as files (novox/hq ADR 0267): the planner maps the next merge onto it. + for _, s := range result.Sources { + kept.Sources = append(kept.Sources, inventory.BuildSource{Repository: s.Repository, Ref: s.Ref, Paths: s.Paths}) + } var announced []inventory.Artifact for _, made := range result.Made { announced = append(announced, inventory.Artifact{ diff --git a/cmd/mesh-controller/check_here.go b/cmd/mesh-controller/check_here.go index 58645c71..201e1dec 100644 --- a/cmd/mesh-controller/check_here.go +++ b/cmd/mesh-controller/check_here.go @@ -82,7 +82,9 @@ func checkHereCommand(ctx context.Context, args []string) error { if _, err := git("fetch", "--quiet", "origin", *base); err != nil { return fmt.Errorf("cannot fetch %s to say what the change touches: %w", *base, err) } - changedText, err := git("diff", "--name-only", "origin/"+*base+"...HEAD") + // Without rename detection, so a file moved out of a build source is said under its old name as well: + // its going is a change to the build that held it (novox/hq ADR 0267). + changedText, err := git("diff", "--name-only", "--no-renames", "origin/"+*base+"...HEAD") if err != nil { return err } diff --git a/cmd/mesh-controller/facts.go b/cmd/mesh-controller/facts.go index 01ffd437..2d0cda96 100644 --- a/cmd/mesh-controller/facts.go +++ b/cmd/mesh-controller/facts.go @@ -411,7 +411,16 @@ func gatherFacts(ctx context.Context, open *stores, busVersion string) (snapshot Commit: e.Source.BuiltFrom, Provided: e.Provided, RollOut: current[e.Manifest.Module].RollOut, Manifest: raw} for _, r := range read[e.Manifest.Module] { + // 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. + if r.Own { + mod.Sources = append(mod.Sources, snapshot.BuildSource{Own: true, Paths: r.Paths}) + continue + } mod.Reads = append(mod.Reads, snapshot.RepositoryName(r.Repository)) + if len(r.Paths) > 0 { + mod.Sources = append(mod.Sources, snapshot.BuildSource{Repository: snapshot.RepositoryName(r.Repository), Paths: r.Paths}) + } } f.Modules = append(f.Modules, mod) if e.Provided || e.Source.Repository == "" { diff --git a/cmd/mesh-controller/merge_gate.go b/cmd/mesh-controller/merge_gate.go index dd925642..ddcfd609 100644 --- a/cmd/mesh-controller/merge_gate.go +++ b/cmd/mesh-controller/merge_gate.go @@ -1204,7 +1204,18 @@ func graphOfFacts(f snapshot.Facts) ([]inventory.Entry, map[string][]inventory.R entries = append(entries, inventory.Entry{Manifest: manifest, Provided: mod.Provided, Source: inventory.Source{Repository: mod.Repository, Path: mod.Path, BuiltFrom: mod.Commit}}) for _, r := range mod.Reads { - read[mod.Name] = append(read[mod.Name], inventory.ReadRepository{Repository: r}) + entry := inventory.ReadRepository{Repository: r} + for _, s := range mod.Sources { + if !s.Own && s.Repository == r && len(s.Paths) > 0 { + entry.Paths = s.Paths + } + } + read[mod.Name] = append(read[mod.Name], entry) + } + for _, s := range mod.Sources { + if s.Own && len(s.Paths) > 0 { + read[mod.Name] = append(read[mod.Name], inventory.ReadRepository{Own: true, Paths: s.Paths}) + } } } var edges []inventory.Edge diff --git a/cmd/mesh-controller/missed_merges.go b/cmd/mesh-controller/missed_merges.go index d68bfebf..fb43871f 100644 --- a/cmd/mesh-controller/missed_merges.go +++ b/cmd/mesh-controller/missed_merges.go @@ -48,7 +48,7 @@ func catchingUpOnMerges(ctx context.Context, open *stores, announced merges) { if err != nil { return nil, nil, err } - read, err := open.inventory.ReadRepositories(ctx) + read, err := readForAMerge(ctx, open.inventory) return entries, read, err } failing := "" diff --git a/cmd/mesh-controller/order_test.go b/cmd/mesh-controller/order_test.go index 7c6a901c..f362c8df 100644 --- a/cmd/mesh-controller/order_test.go +++ b/cmd/mesh-controller/order_test.go @@ -157,7 +157,7 @@ func TestAMergeRebuildsTheModulesItChanged(t *testing.T) { {"a file directly among the modules", merge([]string{"modules/README.md"}, false), ""}, {"a root file beside a module's", merge([]string{"merge-check.sh", "modules/keycloak/x.ts"}, false), "keycloak"}, } { - if got := named(whatTheMergeTouched(candidates, known, c.m)); got != c.want { + if got := named(whatTheMergeTouched(candidates, known, c.m, nil)); got != c.want { t.Errorf("%s: rebuilt %q, wanted %q", c.what, got, c.want) } } @@ -285,7 +285,7 @@ func TestAChangeInsideAModuleIsThatModulesHeldOrNot(t *testing.T) { {"an old announcer saying nothing", merge(showcase, nil, false), ""}, {"a manifest the merge removed, said or not", merge([]string{"modules/gone/module.json", "modules/gone/x.ts"}, nil, true), ""}, } { - if got := named(whatTheMergeTouched(candidates, known, c.m)); got != c.want { + if got := named(whatTheMergeTouched(candidates, known, c.m, nil)); got != c.want { t.Errorf("%s: rebuilt %q, wanted %q", c.what, got, c.want) } } diff --git a/cmd/mesh-controller/planner_records_test.go b/cmd/mesh-controller/planner_records_test.go index bb636f13..6d3e261c 100644 --- a/cmd/mesh-controller/planner_records_test.go +++ b/cmd/mesh-controller/planner_records_test.go @@ -15,16 +15,16 @@ import ( // inventory.Dependencies (dependenciesOf over the records), and the merge planned by reachOfMerge — the // path a real merge takes, short of the bus. // -// **The repository rows are CURRENT BEHAVIOUR, documented — not the rule the operator states** -// (novox/hq issue 338, and the decision pending on it): a build that read a repository gives its module a -// packages edge to every module built from that repository, and mergeCandidates moves it on any merge to -// that repository, whatever the files. So a change to C alone, or to a README, moves the module that -// packages C's repository. ADR 0238 §3 records exactly that today ("a repository a recipe names"); the -// expectations marked 338 change with that decision. -func TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday(t *testing.T) { +// **A shared repository moves only what a change's files are in the build source of** (novox/hq ADR 0267, +// issues 338 and 363): each build records the build source it said, and a merge is mapped onto those of the +// newest builds. A build that said none (P below, as every build before ADR 0267) is read as before: P moves +// on any merge to the repository it packages, though through no edge. The rows tagged 338 held the opposite +// until ADR 0267 was built. +func TestASharedRepositoryIsPlannedFromTheRecordedBuildSources(t *testing.T) { inv := inventory.ForTest(t) ctx := t.Context() asked := time.Now().Add(-time.Hour) + sourcesOf := map[string][]inventory.BuildSource{} register := func(m catalogue.Manifest, repository, path string, against []string, read []inventory.ReadRepository) { t.Helper() if err := inv.RegisterModule(ctx, m, inventory.Source{Repository: repository, Seat: "git", Path: path, @@ -32,7 +32,8 @@ func TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday(t *testing.T) { t.Fatal(err) } if err := inv.RecordBuild(ctx, inventory.Build{ID: "build-" + m.Module, Repository: repository, Ref: "main", - Module: m.Module, Commit: "old", On: "builder", Path: path, Against: against, Read: read, Asked: asked}); err != nil { + Module: m.Module, Commit: "old", On: "builder", Path: path, Against: against, Read: read, Asked: asked, + Sources: sourcesOf[m.Module]}); err != nil { t.Fatal(err) } } @@ -40,7 +41,16 @@ func TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday(t *testing.T) { agent := catalogue.Manifest{Module: "build-agent", Version: "1", Claims: []catalogue.Claim{{Name: "node-build-agent", Scope: catalogue.ScopeNode}}} - // The shape of issue 338. + // The shape of issue 338, each build saying its build source (ADR 0267). + gomod := []string{"go.mod", "go.sum"} + sourcesOf["mesh-controller"] = []inventory.BuildSource{{Paths: append([]string{"module.json", "cmd/mesh-controller/", + "internal/conditions/", "internal/broker/"}, gomod...)}} + sourcesOf["build-agent"] = []inventory.BuildSource{ + {Paths: []string{"modules/build-agent/Dockerfile", "modules/build-agent/module.json"}}, + {Repository: "novox/mesh-controller", Ref: "main", Paths: append([]string{"cmd/mesh-builder/", "internal/broker/"}, gomod...)}} + sourcesOf["route-proxy"] = []inventory.BuildSource{ + {Paths: []string{"modules/route-proxy/Dockerfile", "modules/route-proxy/module.json"}}, + {Repository: "novox/mesh-controller", Ref: "main", Paths: append([]string{"examples/route-proxy/", "internal/broker/"}, gomod...)}} register(catalogue.Manifest{Module: "mesh-controller", Version: "1"}, "novox/mesh-controller", "", nil, nil) register(agent, "novox/mesh-catalog", "modules/build-agent", nil, controllerRead) register(catalogue.Manifest{Module: "route-proxy", Version: "1"}, "novox/mesh-catalog", "modules/route-proxy", nil, controllerRead) @@ -81,13 +91,15 @@ func TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday(t *testing.T) { if !reflect.DeepEqual(shared, sharedRepositoryEdges) { t.Errorf("derived %v\nthe hand-written rows use %v", shared, sharedRepositoryEdges) } - // Each kind derived from its record: built against (stands-on), build.on (declared), read (packages). + // Each kind derived from its record: built against (stands-on), build.on (declared); a read draws none. + for _, e := range edges { + if e.Kind == inventory.EdgePackages { + t.Errorf("a packages edge was drawn (ADR 0267 rule 4): %v", e) + } + } for _, want := range []inventory.Edge{ dep("d", inventory.EdgeStandsOn, "a"), dep("e", inventory.EdgeDeclared, "b"), - dep("p", inventory.EdgePackages, "a"), - dep("p", inventory.EdgePackages, "b"), - dep("p", inventory.EdgePackages, "c"), dep("d", inventory.EdgeBuiltBy, "build-agent"), } { found := false @@ -105,15 +117,23 @@ func TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday(t *testing.T) { want string issue338 bool }{ - {"A and B changed, C untouched: D after A, E after B; P packages their repository", "one", + {"A and B changed, C untouched: D after A, E after B; P, which said no build source, reads all", "one", []string{"modules/a/x.go", "modules/b/x.go"}, "a,b,p | d,e", false}, - {"C alone: C, and P, which packages C's repository", "one", - []string{"modules/c/x.go"}, "c,p", true}, - {"a README of the repository P packages: P moves, nothing built from it does", "one", - []string{"README.md"}, "p", true}, + {"C alone: C, and P, read whole as before; P after nothing", "one", + []string{"modules/c/x.go"}, "c,p", false}, + {"a README of the repository P packages: P, read whole as before", "one", + []string{"README.md"}, "p", false}, {"the dependent's repository: D alone", "two", []string{"d/main.go"}, "d", false}, - {"a README of the controller's repository: all three, three tiers", "mesh-controller", - []string{"README.md"}, "mesh-controller | build-agent | route-proxy", true}, + {"a README of the controller's repository: no module", "mesh-controller", + []string{"README.md"}, "", true}, + {"the controller's own command: the controller alone", "mesh-controller", + []string{"cmd/mesh-controller/main.go"}, "mesh-controller", true}, + {"a package only the controller builds from: the controller alone", "mesh-controller", + []string{"internal/conditions/condition.go"}, "mesh-controller", true}, + {"the route proxy's program: the route proxy alone", "mesh-controller", + []string{"examples/route-proxy/main.go"}, "route-proxy", true}, + {"a package all three build from: all three", "mesh-controller", + []string{"internal/broker/broker.go"}, "mesh-controller | build-agent | route-proxy", false}, {"the route proxy's directory in the catalogue: it alone", "mesh-catalog", []string{"modules/route-proxy/module.json"}, "route-proxy", false}, {"the build agent's directory: it alone, nothing it builds", "mesh-catalog", @@ -123,7 +143,7 @@ func TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday(t *testing.T) { if got != c.want { tag := "" if c.issue338 { - tag = " (current behaviour, issue 338)" + tag = " (issue 338, flipped by ADR 0267)" } t.Errorf("%s: planned %q, wanted %q%s", c.what, got, c.want, tag) } diff --git a/cmd/mesh-controller/planner_rules_test.go b/cmd/mesh-controller/planner_rules_test.go index 6b8dd23d..1aab2260 100644 --- a/cmd/mesh-controller/planner_rules_test.go +++ b/cmd/mesh-controller/planner_rules_test.go @@ -1,12 +1,14 @@ package main import ( + "encoding/json" "fmt" "math/rand/v2" "sort" "strings" "testing" + snapshot "github.com/novox/mesh-controller/internal/facts" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" ) @@ -21,7 +23,7 @@ import ( // kind widens the plan orders the tiers // stands-on yes yes, after its base is built // declared yes yes, after its base is built -// packages yes no, the same tier (a code dependency) +// packages no no — retired by novox/hq ADR 0267; one recorded before is read and ignored // built-by no yes, after the build machine — except for what the build machine stands // on, and for the controller whose worker it binds // worker-of no yes, the build seat's holder after the controller (hq issue 206) @@ -123,9 +125,9 @@ func TestAPlanIsWhatTheChangeTouchedAndWhatIsBuiltOnIt(t *testing.T) { {what: "transitive: F on D on A, A changed", edges: []inventory.Edge{dep("f", standsOn, "d"), dep("d", standsOn, "a")}, repo: "one", paths: []string{"modules/a/x"}, want: "a | d | f"}, - {what: "transitive across kinds: F declared on D, D packages A", + {what: "transitive across kinds stops at a packages edge: F declared on D, D packages A (ADR 0267)", edges: []inventory.Edge{dep("f", declared, "d"), dep("d", packages, "a")}, - repo: "one", paths: []string{"modules/a/x"}, want: "a,d | f"}, + repo: "one", paths: []string{"modules/a/x"}, want: "a"}, // Each kind alone: X depends on A, A changed (widening), then both changed (ordering). {what: "stands-on (built against A's artifact) widens", edges: []inventory.Edge{dep("x", standsOn, "a")}, @@ -136,8 +138,8 @@ func TestAPlanIsWhatTheChangeTouchedAndWhatIsBuiltOnIt(t *testing.T) { repo: "one", paths: []string{"modules/a/x"}, want: "a | x"}, {what: "declared orders", edges: []inventory.Edge{dep("x", declared, "a")}, repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a | x"}, - {what: "packages widens, into the same tier", edges: []inventory.Edge{dep("x", packages, "a")}, - repo: "one", paths: []string{"modules/a/x"}, want: "a,x"}, + {what: "packages, recorded before ADR 0267, widens nothing", edges: []inventory.Edge{dep("x", packages, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a"}, {what: "packages does not order", edges: []inventory.Edge{dep("x", packages, "a")}, repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a,x"}, {what: "built-by never widens", edges: []inventory.Edge{dep("x", builtBy, "a")}, @@ -188,7 +190,7 @@ func TestAPlanIsWhatTheChangeTouchedAndWhatIsBuiltOnIt(t *testing.T) { repo: "one", paths: []string{"modules/z/x"}, want: "z | a,b | d"}, {what: "a diamond of mixed kinds orders on the ordering side only", edges: []inventory.Edge{dep("d", standsOn, "a"), dep("d", packages, "b")}, - repo: "one", paths: []string{"modules/b/x"}, want: "b,d"}, + repo: "one", paths: []string{"modules/b/x"}, want: "b"}, // A cycle the catalogue should never produce: what remains is one last tier, and said. {what: "a cycle is one last tier, not lost", edges: []inventory.Edge{dep("a", standsOn, "b"), dep("b", standsOn, "a"), @@ -225,22 +227,16 @@ func TestAPlanIsWhatTheChangeTouchedAndWhatIsBuiltOnIt(t *testing.T) { } } -// **CURRENT BEHAVIOUR, documented — not the rule the operator states.** novox/hq issue 338 (a module -// built from a shared repository moves on every merge to it) and the decision pending on it would change -// every row here. Today: +// **A shared repository moves only what a change's files are in the build source of** (novox/hq ADR 0267, +// issue 338, issue 363). The controller is built from its repository's root as a Go bundle; the route proxy +// and the build seat's holder build images whose context is that repository and which name the package they +// compile. Each newest trunk build said its build source — the import closure of its program — and a merge +// is mapped onto those. The rows tagged 338 held the opposite until ADR 0267 was built: every merge to the +// controller's repository planned all three, in three tiers. // -// - mesh-controller is built from its repository's root, so every file of that repository touches it; -// - route-proxy and build-agent package the whole of that repository (a build context), so the build -// record's `read` makes them move on any merge to it, whatever the files, and dependenciesOf gives -// each a packages edge to every module built from it; -// - built-by (route-proxy on build-agent) and worker-of (build-agent on the controller) make it three -// tiers. -// -// These follow ADR 0238 §3 as written ("the whole repository for a module built from its root, and a -// repository a recipe names"), so they are not failures; when the decision on issue 338 lands, these -// expectations change with it. The edges are the ones dependenciesOf derives from this catalogue — held -// to that by TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday, which derives them from the store. -func TestASharedRepositoryMovesWhatPackagesItAsItDoesToday(t *testing.T) { +// The build sources are this repository's own programs as GoBuildSource reads them (held to that by +// TestThisRepositorysProgramsHaveBuildSourcesOfTheirOwn in internal/builder), cut to what the rows need. +func TestASharedRepositoryMovesOnlyWhatItsBuildSourceHolds(t *testing.T) { const catalogueRepo = "http://forge.internal:20000/novox/mesh-catalog.git" const controllerRepo = "http://forge.internal:20000/novox/mesh-controller.git" entries := []inventory.Entry{ @@ -249,7 +245,26 @@ func TestASharedRepositoryMovesWhatPackagesItAsItDoesToday(t *testing.T) { fromRepo("route-proxy", catalogueRepo, "modules/route-proxy"), fromRepo("gitea", catalogueRepo, "modules/gitea"), } + gomod := []string{"go.mod", "go.sum", "vendor/modules.txt"} + with := func(paths ...string) []string { return append(append([]string{}, gomod...), paths...) } read := map[string][]inventory.ReadRepository{ + "mesh-controller": {{Own: true, Paths: with("module.json", "cmd/mesh-controller/", "internal/conditions/", + "internal/broker/", "internal/builder/", "internal/inventory/", "internal/inventory/migrations/**", + "vendor/github.com/nats-io/nats.go/")}}, + "build-agent": { + {Repository: "novox/mesh-controller", Ref: "main", Paths: with("cmd/mesh-builder/", "internal/broker/", + "internal/builder/", "internal/inventory/", "internal/inventory/migrations/**", "vendor/github.com/nats-io/nats.go/")}, + {Own: true, Paths: []string{"modules/build-agent/Dockerfile", "modules/build-agent/module.json"}}, + }, + "route-proxy": { + {Repository: "novox/mesh-controller", Ref: "main", Paths: with("examples/route-proxy/", "internal/broker/", + "vendor/github.com/nats-io/nats.go/")}, + {Own: true, Paths: []string{"modules/route-proxy/Dockerfile", "modules/route-proxy/module.json"}}, + }, + } + // With no build source said — before each module's first trunk build under ADR 0267, or while an + // earlier merge's build of it is pending — a module is read as before. + unsaid := map[string][]inventory.ReadRepository{ "build-agent": {{Repository: "novox/mesh-controller", Ref: "main"}}, "route-proxy": {{Repository: "novox/mesh-controller", Ref: "main"}}, } @@ -257,51 +272,122 @@ func TestASharedRepositoryMovesWhatPackagesItAsItDoesToday(t *testing.T) { for _, c := range []struct { what, repo string paths []string + read map[string][]inventory.ReadRepository want string + unread string }{ - // The live three-tier plan of 2026-10-08 (issue 338), in the worker-of order (issue 206) that - // TestAMergeIsPlannedInTiersAlongTheThreeKindsOfDependency's controller case holds too. - {"a README of the controller's repository moves all three, in three tiers", "mesh-controller", - []string{"README.md"}, "mesh-controller | build-agent | route-proxy"}, - {"the controller's own code: the same", "mesh-controller", - []string{"cmd/mesh-controller/main.go"}, "mesh-controller | build-agent | route-proxy"}, - {"the route proxy's program alone: the same, the controller with it", "mesh-controller", - []string{"examples/route-proxy/main.go"}, "mesh-controller | build-agent | route-proxy"}, - // In the catalogue, where they live, the rule is path-precise. - {"the route proxy's directory in the catalogue: it alone", "mesh-catalog", - []string{"modules/route-proxy/module.json"}, "route-proxy"}, - {"the build agent's directory: it alone, nothing it builds", "mesh-catalog", - []string{"modules/build-agent/module.json"}, "build-agent"}, + // The operator's acceptance: a merge of the controller's own code plans the controller alone. + {"the controller's own command: the controller alone (338)", "mesh-controller", + []string{"cmd/mesh-controller/main.go"}, read, "mesh-controller", ""}, + {"a package only the controller builds from: the controller alone (338)", "mesh-controller", + []string{"internal/conditions/condition.go", "internal/conditions/bus.go"}, read, "mesh-controller", ""}, + {"a test beside the controller's command: nothing is built from it", "mesh-controller", + []string{"cmd/mesh-controller/main_test.go"}, read, "", "cmd/mesh-controller/main_test.go"}, + {"a README of the controller's repository: no module (338)", "mesh-controller", + []string{"README.md"}, read, "", "README.md"}, + {"the route proxy's program alone: the route proxy alone (338)", "mesh-controller", + []string{"examples/route-proxy/main.go"}, read, "route-proxy", ""}, + {"the build seat's program alone: its holder alone", "mesh-controller", + []string{"cmd/mesh-builder/main.go"}, read, "build-agent", ""}, + {"a package the build seat's program and the controller build from: both", "mesh-controller", + []string{"internal/builder/builder.go"}, read, "mesh-controller | build-agent", ""}, + {"a migration the controller and the build seat's program embed: both", "mesh-controller", + []string{"internal/inventory/migrations/0088-a-build-says-its-build-source.sql"}, read, + "mesh-controller | build-agent", ""}, + // A package all three build from: all three; the build seat's holder after the controller whose worker + // it binds (worker-of, issue 206), the proxy after the holder that builds it (built-by). + {"a package all three build from: all three", "mesh-controller", + []string{"internal/broker/broker.go"}, read, "mesh-controller | build-agent | route-proxy", ""}, + {"a vendored package all three build from: all three", "mesh-controller", + []string{"vendor/github.com/nats-io/nats.go/nats.go"}, read, "mesh-controller | build-agent | route-proxy", ""}, + {"go.sum: all three", "mesh-controller", + []string{"go.sum"}, read, "mesh-controller | build-agent | route-proxy", ""}, + {"a file added to the proxy's package: the proxy", "mesh-controller", + []string{"examples/route-proxy/new.go"}, read, "route-proxy", ""}, + // Before any build source is said: as before. + {"no build source said: a README moves all three, as before", "mesh-controller", + []string{"README.md"}, unsaid, "mesh-controller | build-agent | route-proxy", ""}, + // In the catalogue, where they live, each by its own build source. + {"the route proxy's recipe: it alone", "mesh-catalog", + []string{"modules/route-proxy/Dockerfile"}, read, "route-proxy", ""}, + {"the route proxy's manifest: it alone", "mesh-catalog", + []string{"modules/route-proxy/module.json"}, read, "route-proxy", ""}, + {"the route proxy's README: nothing", "mesh-catalog", + []string{"modules/route-proxy/README.md"}, read, "", "modules/route-proxy/README.md"}, + {"the build agent's manifest: it alone, nothing it builds", "mesh-catalog", + []string{"modules/build-agent/module.json"}, read, "build-agent", ""}, {"another module of the catalogue: neither", "mesh-catalog", - []string{"modules/gitea/index.ts"}, "gitea"}, + []string{"modules/gitea/index.ts"}, read, "gitea", ""}, } { - r, got := planMerge(t, c.repo, c.paths, entries, read, edges) + r, got := planMerge(t, c.repo, c.paths, entries, c.read, edges) if got != c.want { - t.Errorf("%s: planned %q, wanted %q (as today; issue 338)", c.what, got, c.want) + t.Errorf("%s: planned %q, wanted %q", c.what, got, c.want) } - if c.repo == "mesh-controller" && strings.Join(r.Unread, ",") != "" { - t.Errorf("%s: a root-built module reads every file, and %v were said unread", c.what, r.Unread) + if u := strings.Join(r.Unread, ","); u != c.unread { + t.Errorf("%s: unread %q, wanted %q", c.what, u, c.unread) } } + // Files not all said: everything the repository builds, as before. + m := link.SourceMoved{Owner: "novox", Repo: "mesh-controller", Base: "main", Commit: "head", + Paths: []string{"README.md"}, PathsTruncated: true} + if got := tiered(reachOfMerge(m, entries, read, edges).Plan.Tiers); got != "mesh-controller | build-agent | route-proxy" { + t.Errorf("a merge whose files were not all said: planned %q, wanted all three", got) + } +} + +// **A module an open plan has yet to build is read whole** (novox/hq ADR 0267): its build source is about to +// change. A merge that added an import to the route proxy is planned; before that build lands, a merge +// changing only the package newly imported must still move the proxy — the build source its last build +// said does not hold it yet. +func TestAModuleAPlanHasYetToBuildIsReadWhole(t *testing.T) { + read := map[string][]inventory.ReadRepository{ + "route-proxy": { + {Repository: "novox/mesh-controller", Ref: "main", Paths: []string{"examples/route-proxy/", "go.mod"}}, + {Own: true, Paths: []string{"modules/route-proxy/module.json"}}, + }, + "mesh-controller": {{Own: true, Paths: []string{"module.json", "cmd/mesh-controller/"}}}, + } + open := []inventory.Plan{{State: inventory.PlanBuilding, Modules: map[string]*inventory.PlanModule{ + "route-proxy": {State: "building"}, "gitea": {State: "built"}}}} + pending := pendingIn(open) + if !pending["route-proxy"] || pending["gitea"] { + t.Fatalf("pending %v", pending) + } + m := link.SourceMoved{Owner: "novox", Repo: "mesh-controller", Base: "main", Paths: []string{"internal/newly/imported.go"}} + if readsFrom(read["route-proxy"], m) { + t.Fatal("the said build source holds the new package: the fixture is wrong") + } + whole := unnarrowed(read, pending) + if !readsFrom(whole["route-proxy"], m) { + t.Error("a module a plan has yet to build was read through the build source it is about to replace") + } + if ownSource(whole["route-proxy"]) != nil { + t.Error("a pending module kept its own build source") + } + if ownSource(whole["mesh-controller"]) == nil { + t.Error("a module no plan is building lost its build source") + } + // A closed plan holds nothing back. + if len(pendingIn([]inventory.Plan{{State: inventory.PlanDone, Modules: open[0].Modules}})) != 0 { + t.Error("a plan that is done held a module back") + } } // sharedRepositoryEdges is what dependenciesOf derives for the catalogue of the test above, sorted as it -// sorts them. +// sorts them: no packages edge (novox/hq ADR 0267 rule 4). var sharedRepositoryEdges = []inventory.Edge{ - dep("build-agent", inventory.EdgePackages, "mesh-controller"), dep("build-agent", inventory.EdgeWorkerOf, "mesh-controller"), dep("gitea", inventory.EdgeBuiltBy, "build-agent"), dep("mesh-controller", inventory.EdgeBuiltBy, "build-agent"), dep("route-proxy", inventory.EdgeBuiltBy, "build-agent"), - dep("route-proxy", inventory.EdgePackages, "mesh-controller"), } // **The planner's invariant, over random catalogues.** For any catalogue whose dependencies form no cycle // and any set of changed files in one repository: // // - the plan is exactly the modules of that repository whose directory holds a changed file (every file, -// for a module built from the root), and everything reachable from them along stands-on, declared and -// packages — never along built-by or worker-of; +// for a module built from the root), and everything reachable from them along stands-on and declared — +// never along packages (novox/hq ADR 0267), built-by or worker-of; // - every stands-on, declared, built-by and worker-of edge with both ends in the plan has the module // depended on in an earlier tier; // - no cycle is said. @@ -310,7 +396,7 @@ var sharedRepositoryEdges = []inventory.Edge{ func TestAPlanIsTheTouchedModulesAndWhatIsReachableAlongTheWideningEdges(t *testing.T) { kinds := []string{inventory.EdgeStandsOn, inventory.EdgeDeclared, inventory.EdgePackages, inventory.EdgeBuiltBy, inventory.EdgeWorkerOf} - widens := map[string]bool{inventory.EdgeStandsOn: true, inventory.EdgeDeclared: true, inventory.EdgePackages: true} + widens := map[string]bool{inventory.EdgeStandsOn: true, inventory.EdgeDeclared: true} orders := map[string]bool{inventory.EdgeStandsOn: true, inventory.EdgeDeclared: true, inventory.EdgeBuiltBy: true, inventory.EdgeWorkerOf: true} // Directory names drawn from one pool, so two repositories hold directories of the same name, and one @@ -445,3 +531,43 @@ func describe(entries []inventory.Entry) []string { } return out } + +// **The merge gate reads the build sources the snapshot carries** (novox/hq ADR 0267): the gate's plan of a +// change is the merge handler's, so a snapshot taken by a controller that records build sources narrows the +// gate's plan as it narrows the merge's; one without them reads every module as before. +func TestTheGatePlansFromTheBuildSourcesTheSnapshotCarries(t *testing.T) { + manifest := func(name string) json.RawMessage { return json.RawMessage(`{"module":"` + name + `","version":"1"}`) } + facts := snapshot.Facts{Modules: []snapshot.Module{ + {Name: "mesh-controller", Repository: "novox/mesh-controller", Manifest: manifest("mesh-controller"), + Sources: []snapshot.BuildSource{{Own: true, Paths: []string{"module.json", "cmd/mesh-controller/", "internal/broker/"}}}}, + {Name: "route-proxy", Repository: "novox/mesh-catalog", Path: "modules/route-proxy", Manifest: manifest("route-proxy"), + Reads: []string{"novox/mesh-controller"}, + Sources: []snapshot.BuildSource{{Repository: "novox/mesh-controller", Paths: []string{"examples/route-proxy/", "internal/broker/"}}, + {Own: true, Paths: []string{"modules/route-proxy/module.json"}}}}, + }} + for paths, want := range map[string]string{ + "cmd/mesh-controller/main.go": "mesh-controller", + "examples/route-proxy/main.go": "route-proxy", + "internal/broker/broker.go": "mesh-controller,route-proxy", + "README.md": "", + } { + r, err := reachOfChange(facts, "novox/mesh-controller", []string{paths}, "") + if err != nil { + t.Fatal(err) + } + if got := tiered(r.Plan.Tiers); got != want { + t.Errorf("%s: the gate planned %q, wanted %q", paths, got, want) + } + } + // A snapshot without build sources: as before. + for i := range facts.Modules { + facts.Modules[i].Sources = nil + } + r, err := reachOfChange(facts, "novox/mesh-controller", []string{"README.md"}, "") + if err != nil { + t.Fatal(err) + } + if got := tiered(r.Plan.Tiers); got != "mesh-controller,route-proxy" { + t.Errorf("a snapshot without build sources: the gate planned %q for a README", got) + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 31c90340..039d5a22 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -138,9 +138,10 @@ func reachableFrom(moved []string, edges []inventory.Edge) []string { grew = false for _, e := range edges { // Built-by and worker-of order a plan; neither widens it. A new build machine changes - // nothing it builds, and a new controller changes nothing about the holder it orders — - // what packages the controller's source is already a code edge. - if e.Kind == inventory.EdgeBuiltBy || e.Kind == inventory.EdgeWorkerOf { + // nothing it builds, and a new controller changes nothing about the holder it orders. A + // packages edge, read from a record made before novox/hq ADR 0267, widens nothing either: + // a shared file moves each module whose build source holds it, directly. + if e.Kind == inventory.EdgeBuiltBy || e.Kind == inventory.EdgeWorkerOf || e.Kind == inventory.EdgePackages { continue } if in[e.To] && !in[e.From] { diff --git a/cmd/mesh-controller/release_plan_test.go b/cmd/mesh-controller/release_plan_test.go index 118a495b..d26bcf57 100644 --- a/cmd/mesh-controller/release_plan_test.go +++ b/cmd/mesh-controller/release_plan_test.go @@ -59,17 +59,18 @@ func TestAMergeIsPlannedInTiersAlongTheThreeKindsOfDependency(t *testing.T) { t.Fatalf("no cycle here: %v", tiers) } - // The controller alone moved: the proxy with it, nothing else. - small := reachableFrom([]string{"mesh-controller"}, edges) - if len(small) != 3 { - t.Fatalf("a controller merge rebuilds the controller and what packages it: %v", small) + // The controller alone moved: the controller alone — what packages its repository moves only when the + // change is in its own build source (novox/hq ADR 0267), so a packages edge widens nothing. + if alone := reachableFrom([]string{"mesh-controller"}, edges); len(alone) != 1 { + t.Fatalf("a controller merge rebuilds the controller alone: %v", alone) } - // The builder and the proxy package the controller's source, which orders nothing. The builder holds + // A change to a package all three build from moves all three. The builder holds // the build seat, whose worker the controller defines, so it follows the controller (worker-of, // novox/hq issue 206), and the controller's built-by edge to it yields: the controller is built by the // build machine that is running. The proxy is built by the new builder: the controller, the builder, // the proxy — the live plan of every controller merge. (This read "the builder, then the controller // and the proxy together" before issue 206, and the fixture had no worker-of edge.) + small := reachableFrom([]string{"mesh-controller", "builder", "route-proxy"}, edges) smallTiers := tiersOf(small, edges) if got := tiered(smallTiers); got != "mesh-controller | builder | route-proxy" { t.Fatalf("the controller, then the builder, then the proxy: %v", smallTiers) @@ -243,7 +244,7 @@ func TestAChangeToTheBuildAgentRebuildsTheBuildAgentAlone(t *testing.T) { } { m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "abc", Paths: paths, ModuleDirs: []string{"modules/build-agent"}, ModuleDirsSaid: true} - touched := whatTheMergeTouched(entries, entries, m) + touched := whatTheMergeTouched(entries, entries, m, nil) if len(touched) != 1 || touched[0].Manifest.Module != "build-agent" { t.Fatalf("%v touched %v", paths, touched) } diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index d0c2215c..3288559e 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -12,6 +12,7 @@ import ( "sync" "time" + "github.com/novox/mesh-controller/internal/builder" "github.com/novox/mesh-controller/internal/catalogue" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" @@ -313,7 +314,7 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { if err != nil { return notNow(err) } - read, err := inv.ReadRepositories(ctx) + read, err := readForAMerge(ctx, inv) if err != nil { return notNow(err) } @@ -360,7 +361,7 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { e.Manifest.Module, m.Owner, m.Repo, e.Source.Ref, m.Base) } } - touched, added, _ := touchedBy(from, entries, m) + touched, added, _ := touchedBy(from, entries, m, read) // **A module the merge deleted is not built** (novox/hq ADR 0236): its manifest is gone, so the build // seat finds nothing saying what it is, and the plan failed on it (`has no module.json at …`) with // every other module of its tier left unsent. It is forgotten where nothing holds it, said otherwise. @@ -587,7 +588,7 @@ func mergeCandidates(m link.SourceMoved, entries []inventory.Entry, func wouldMove(m link.SourceMoved, entries []inventory.Entry, read map[string][]inventory.ReadRepository) []inventory.Entry { from, _, _ := mergeCandidates(m, entries, read) - touched, _ := splitDeleted(whatTheMergeTouched(from, entries, m), m) + touched, _ := splitDeleted(whatTheMergeTouched(from, entries, m, read), m) return touched } @@ -672,18 +673,112 @@ func sameRepository(repository string, m link.SourceMoved) bool { (m.CloneURL != "" && repo == strings.ToLower(strings.TrimSuffix(m.CloneURL, ".git"))) } -// readsFrom is whether a module's build read the repository a merge names: the second repository its -// recipe packages source from. Its ref must be the branch that moved, or unset — the same rule a -// module's own source follows. +// readsFrom is whether a merge changed what a module's build read in another repository: the second +// repository its recipe packages source from. Its ref must be the branch that moved, or unset — the same +// rule a module's own source follows. +// +// **Only a changed file in what the build read there** (novox/hq ADR 0267 rule 2): where the module's +// newest trunk build said its build source in that repository, a merge touching none of it is no change +// to the module (issue 338: every merge to the controller's repository moved the route proxy and the +// build seat's holder). Where it said none, or the merge's files are not all said, the whole repository is +// read, as before. func readsFrom(read []inventory.ReadRepository, m link.SourceMoved) bool { for _, r := range read { - if sameRepository(r.Repository, m) && (r.Ref == "" || r.Ref == m.Base) { + if r.Own || !sameRepository(r.Repository, m) || (r.Ref != "" && r.Ref != m.Base) { + continue + } + if len(r.Paths) == 0 || len(m.Paths) == 0 || m.PathsTruncated { return true } + for _, p := range m.Paths { + if builder.SourceHolds(r.Paths, p) { + return true + } + } } return false } +// ownSource is the build source a module's newest trunk build said it read in its own repository; nil +// when it said none, and the module's own directory — or, built from the root, its whole repository — is +// its build source, as before (novox/hq ADR 0267). +func ownSource(read []inventory.ReadRepository) []string { + for _, r := range read { + if r.Own && len(r.Paths) > 0 { + return r.Paths + } + } + return nil +} + +// readsFile is whether a module built from the merged repository reads one of its changed files: in its +// build source where its newest trunk build said one, else anywhere in its directory, or anywhere at all +// for a module built from the repository's root. +func readsFile(e inventory.Entry, read []inventory.ReadRepository, p string) bool { + if own := ownSource(read); own != nil { + return builder.SourceHolds(own, p) + } + return strings.Trim(e.Source.Path, "/") == "" || inside(p, e.Source.Path) +} + +// pendingIn is the modules an open plan has yet to build (novox/hq ADR 0267): what such a module was built +// from is about to change, so the build source its last build said is not the one a merge now meets — an +// earlier merge that added an import to it would otherwise hide a change to what that import names. Each +// is read whole, as before, and planned again with the merge; the newer plan supersedes the older. +func pendingIn(plans []inventory.Plan) map[string]bool { + pending := map[string]bool{} + for _, p := range plans { + if !p.Open() { + continue + } + for name, s := range p.Modules { + if s == nil || (s.State != "built" && s.State != planDeleted) { + pending[name] = true + } + } + } + return pending +} + +// readForAMerge is what each module's build read, as a merge acting now maps its files onto: the build +// sources the newest trunk builds said, but for the modules an open plan has yet to build (pendingIn). +func readForAMerge(ctx context.Context, inv *inventory.Inventory) (map[string][]inventory.ReadRepository, error) { + read, err := inv.ReadRepositories(ctx) + if err != nil { + return nil, err + } + open, err := inv.OpenPlans(ctx) + if err != nil { + return nil, err + } + return unnarrowed(read, pendingIn(open)), nil +} + +// unnarrowed is what modules read, with the build sources of the pending ones set aside: each of them is +// read whole. +func unnarrowed(read map[string][]inventory.ReadRepository, pending map[string]bool) map[string][]inventory.ReadRepository { + if len(pending) == 0 { + return read + } + out := make(map[string][]inventory.ReadRepository, len(read)) + for name, rs := range read { + if !pending[name] { + out[name] = rs + continue + } + var whole []inventory.ReadRepository + for _, r := range rs { + if r.Own { + continue + } + r.Paths = nil + whole = append(whole, r) + } + out[name] = whole + } + return out +} + // lastLookAt is the most recent look at this repository by anything built from it. func lastLookAt(entries []inventory.Entry, m link.SourceMoved) time.Time { var newest time.Time @@ -698,8 +793,9 @@ 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 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) +func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved, + read map[string][]inventory.ReadRepository) []inventory.Entry { + touched, _, _ := touchedBy(candidates, known, m, read) return touched } @@ -707,8 +803,11 @@ func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved // 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. // -// **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, +// **A changed file touches exactly the modules whose build reads it.** What a build reads is its build +// source, where the module's newest trunk build said one (novox/hq ADR 0267): a Go program's import closure, +// an archive's directory, a recipe, its manifest — so a README at the root of a repository whose module is +// built from its root, or another program's package beside it, touches nothing. Where none was said, it 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 @@ -728,7 +827,8 @@ func whatTheMergeTouched(candidates, known []inventory.Entry, m link.SourceMoved // // 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) { +func touchedBy(candidates, known []inventory.Entry, m link.SourceMoved, + read map[string][]inventory.ReadRepository) (touched []inventory.Entry, added, unread []string) { knownDirs := map[string]bool{} for _, e := range known { if !e.Provided && sameRepository(e.Source.Repository, m) { @@ -763,19 +863,27 @@ func touchedBy(candidates, known []inventory.Entry, m link.SourceMoved) (touched 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 { + if readsFile(e, read[e.Manifest.Module], p) { + touched = append(touched, e) + break + } } } for _, p := range m.Paths { - read := newDir["."] + isRead := newDir["."] for _, e := range candidates { - read = read || strings.Trim(e.Source.Path, "/") == "" || inside(p, e.Source.Path) + isRead = isRead || readsFile(e, read[e.Manifest.Module], p) } for d := range newDir { - read = read || inside(p, d) + isRead = isRead || inside(p, d) } - if !read { + // A file a module packages from this repository is read too, by that module's build. + for _, e := range known { + isRead = isRead || readsFrom(read[e.Manifest.Module], link.SourceMoved{Owner: m.Owner, Repo: m.Repo, + Base: m.Base, CloneURL: m.CloneURL, Paths: []string{p}}) + } + if !isRead { unread = append(unread, p) } } @@ -834,7 +942,7 @@ func (r mergeReach) Dependents() []string { 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) + touched, added, unread := touchedBy(from, entries, m, read) kept, deleted := splitDeleted(touched, m) r := mergeReach{Touched: kept, Deleted: deleted, Packaging: packaging, Already: already, Added: added, Unread: unread} var building []string diff --git a/cmd/mesh-controller/worker_order_test.go b/cmd/mesh-controller/worker_order_test.go index f5b43f20..c633ca55 100644 --- a/cmd/mesh-controller/worker_order_test.go +++ b/cmd/mesh-controller/worker_order_test.go @@ -19,10 +19,12 @@ func TestTheBuildSeatsHolderFollowsTheControllerThatDefinesItsWorker(t *testing. {From: "route-proxy", To: "mesh-controller", Kind: inventory.EdgePackages}, {From: "route-proxy", To: "build-agent", Kind: inventory.EdgeBuiltBy}, } - set := reachableFrom([]string{"mesh-controller"}, edges) - if len(set) != 3 { - t.Fatalf("the controller, what packages it, and nothing more: %v", set) + // A packages edge recorded before novox/hq ADR 0267 widens nothing: the controller moved alone moves + // alone, and a change to a package all three build from moves all three, each by its own build source. + if alone := reachableFrom([]string{"mesh-controller"}, edges); len(alone) != 1 { + t.Fatalf("the controller alone, whatever packages its repository: %v", alone) } + set := reachableFrom([]string{"mesh-controller", "build-agent", "route-proxy"}, edges) tiers := tiersOf(set, edges) pos := map[string]int{} for i, tier := range tiers { diff --git a/internal/facts/facts.go b/internal/facts/facts.go index e010916d..be55f445 100644 --- a/internal/facts/facts.go +++ b/internal/facts/facts.go @@ -226,10 +226,21 @@ type Module struct { RollOut bool `json:"roll-out,omitempty"` // Reads are the other repositories its build read source from. Reads []string `json:"reads,omitempty"` + // Sources are the build source its newest build said it read, per repository (novox/hq ADR 0267): a + // context of Reads by name with its paths, or the module's own repository (Own). Absent, the module's + // build reads every file of each of Reads and of its own directory, as before. + Sources []BuildSource `json:"sources,omitempty"` // Manifest is the module as the mesh holds it: artifacts resolved to the builds it runs. Manifest json.RawMessage `json:"manifest"` } +// BuildSource is the build source a module's build read in one repository (novox/hq ADR 0267). +type BuildSource struct { + Repository string `json:"repository,omitempty"` + Own bool `json:"own,omitempty"` + Paths []string `json:"paths"` +} + // Edge is one build dependency: From is built standing on To. type Edge struct { From string `json:"from"` diff --git a/internal/inventory/builds.go b/internal/inventory/builds.go index 08a48235..178c32d8 100644 --- a/internal/inventory/builds.go +++ b/internal/inventory/builds.go @@ -45,6 +45,9 @@ type Build struct { // Read is every repository this build read source from besides the module's own (novox/hq // 04-ISSUES/131), at the ref it read. Read []ReadRepository + // Sources are what the build was made from, as files, per repository (novox/hq ADR 0267): said by the + // builder for a build of a trunk commit; nil where it said none. + Sources []BuildSource // SourceFingerprint is what the build was made from, hashed, as its builder said it (novox/hq // issue 280); empty from a builder that predates it, or where the source does not pin the build. SourceFingerprint string @@ -75,9 +78,27 @@ func (b Build) AskedOrAt() time.Time { const newestRequestFirst = `coalesce(asked, at) desc, at desc` // ReadRepository is a repository a build read source from besides the module's own. +// +// Paths and Own are never stored in a build's `built_contexts` (a controller that predates them would read +// an own entry as a context, and every module of a repository as packaging every other): ReadRepositories +// lays them over what it reads, from the build's build sources (novox/hq ADR 0267). type ReadRepository struct { Repository string `json:"repository"` Ref string `json:"ref,omitempty"` + // Paths are the build source the build read in this repository (builder.SourceHolds): a changed file + // outside them is no change to the module. Empty is the whole repository, as before. + Paths []string `json:"paths,omitempty"` + // Own is the module's own repository, whose Paths narrow what its own directory — or, for a module + // built from its repository's root, the whole repository — would otherwise be. + Own bool `json:"own,omitempty"` +} + +// BuildSource is the build source a build read in one repository (novox/hq ADR 0267): Repository and Ref +// a context's, empty for the module's own. +type BuildSource struct { + Repository string `json:"repository,omitempty"` + Ref string `json:"ref,omitempty"` + Paths []string `json:"paths"` } // Artifact is one thing a build published. @@ -108,6 +129,14 @@ func (i *Inventory) RecordBuild(ctx context.Context, b Build) error { if err != nil { return err } + var sources any + if len(b.Sources) > 0 { + raw, err := json.Marshal(b.Sources) + if err != nil { + return err + } + sources = raw + } var module *string if b.Module != "" { module = &b.Module @@ -118,11 +147,12 @@ func (i *Inventory) RecordBuild(ctx context.Context, b Build) error { } _, err = i.store.Pool().Exec(ctx, `insert into build (id, repository, ref, module, commit_hash, built_on, failed, made, - source_path, manifest, built_against, built_contexts, asked, source_fingerprint) - values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) + source_path, manifest, built_against, built_contexts, asked, source_fingerprint, + build_sources) + values ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15) on conflict (id) do nothing`, b.ID, b.Repository, b.Ref, module, b.Commit, b.On, b.Failed, made, - b.Path, manifestOrNil(b.Manifest), against, read, asked, b.SourceFingerprint) + b.Path, manifestOrNil(b.Manifest), against, read, asked, b.SourceFingerprint, sources) if err != nil { return err } @@ -302,9 +332,14 @@ func (i *Inventory) BuiltAgainst(ctx context.Context) (map[string][]string, erro // The mirror of BuiltAgainst, and derived the same way and for the same reason: a merge into a // repository a module only packages is a change to that module, and the manifest the mesh keeps // carries nothing that would say so (novox/hq 04-ISSUES/131). +// +// **With the build source that build said** (novox/hq ADR 0267): a context's entry carries the paths the +// build read there, and the module's own repository an entry marked Own with its paths. A build that said +// none — off the trunk, from a builder that predates it, or of a source not known — leaves its module read +// as before: every file of a context, and its own directory or root. func (i *Inventory) ReadRepositories(ctx context.Context) (map[string][]ReadRepository, error) { rows, err := i.store.Pool().Query(ctx, - `select distinct on (module) module, built_contexts + `select distinct on (module) module, built_contexts, build_sources from build where module is not null and module <> '' and failed = '' order by module, `+newestRequestFirst) @@ -316,24 +351,52 @@ func (i *Inventory) ReadRepositories(ctx context.Context) (map[string][]ReadRepo read := map[string][]ReadRepository{} for rows.Next() { var module string - var raw []byte - if err := rows.Scan(&module, &raw); err != nil { + var raw, rawSources []byte + if err := rows.Scan(&module, &raw, &rawSources); err != nil { return nil, err } - if len(raw) == 0 { - continue - } var of []ReadRepository - if err := json.Unmarshal(raw, &of); err != nil { - continue + if len(raw) > 0 { + if err := json.Unmarshal(raw, &of); err != nil { + of = nil + } } - if len(of) > 0 { + var sources []BuildSource + if len(rawSources) > 0 { + if err := json.Unmarshal(rawSources, &sources); err != nil { + sources = nil + } + } + if of = WithBuildSources(of, sources); len(of) > 0 { read[module] = of } } return read, rows.Err() } +// WithBuildSources lays a build's build sources over the repositories it read: a context's paths on its +// entry, and the module's own as an entry of its own. A context the build read that its sources do not +// name stays whole; a source naming a context the build did not say it read is dropped, since nothing +// moves a module through a repository it is not recorded as reading. +func WithBuildSources(read []ReadRepository, sources []BuildSource) []ReadRepository { + out := make([]ReadRepository, 0, len(read)+1) + for _, r := range read { + r.Paths, r.Own = nil, false + for _, s := range sources { + if s.Repository != "" && s.Repository == r.Repository && s.Ref == r.Ref && len(s.Paths) > 0 { + r.Paths = append([]string(nil), s.Paths...) + } + } + out = append(out, r) + } + for _, s := range sources { + if s.Repository == "" && len(s.Paths) > 0 { + out = append(out, ReadRepository{Ref: s.Ref, Paths: append([]string(nil), s.Paths...), Own: true}) + } + } + return out +} + // manifestOrNil keeps the difference between "declared nothing" and "predates this being kept". // // A build recorded before the mesh kept manifests has no manifest, and that is not the same as one diff --git a/internal/inventory/builds_test.go b/internal/inventory/builds_test.go index a2513335..fda48b9e 100644 --- a/internal/inventory/builds_test.go +++ b/internal/inventory/builds_test.go @@ -2,6 +2,7 @@ package inventory import ( "context" + "reflect" "strings" "testing" ) @@ -169,3 +170,47 @@ func TestWhatABuildReadComesBackForTheNewestBuildOfEachModule(t *testing.T) { t.Fatal("a failed build's reading was kept as what that module reads") } } + +// **What a build said it was made from comes back laid over what it read** (novox/hq ADR 0267): a context's +// paths on its entry, the module's own as an entry marked Own — and never in the stored `built_contexts`, +// where a controller that predates them would read an own entry as a context of its own repository. +func TestABuildsBuildSourceComesBackOverWhatItRead(t *testing.T) { + inv := fresh(t) + ctx := context.Background() + said := aBuild("said", "route-proxy", "") + said.Read = []ReadRepository{{Repository: "novox/mesh-controller", Ref: "main"}, {Repository: "novox/other"}} + said.Sources = []BuildSource{ + {Paths: []string{"modules/route-proxy/Dockerfile", "modules/route-proxy/module.json"}}, + {Repository: "novox/mesh-controller", Ref: "main", Paths: []string{"examples/route-proxy/", "go.mod"}}, + {Repository: "novox/unread", Paths: []string{"x"}}, + } + unsaid := aBuild("unsaid", "build-agent", "") + unsaid.Read = []ReadRepository{{Repository: "novox/mesh-controller", Ref: "main"}} + for _, b := range []Build{said, unsaid} { + if err := inv.RecordBuild(ctx, b); err != nil { + t.Fatal(err) + } + } + read, err := inv.ReadRepositories(ctx) + if err != nil { + t.Fatal(err) + } + want := []ReadRepository{ + {Repository: "novox/mesh-controller", Ref: "main", Paths: []string{"examples/route-proxy/", "go.mod"}}, + {Repository: "novox/other"}, + {Paths: []string{"modules/route-proxy/Dockerfile", "modules/route-proxy/module.json"}, Own: true}, + } + if !reflect.DeepEqual(read["route-proxy"], want) { + t.Fatalf("read back %+v\nwanted %+v", read["route-proxy"], want) + } + if got := read["build-agent"]; len(got) != 1 || got[0].Paths != nil || got[0].Own { + t.Fatalf("a build that said no build source reads as %+v", got) + } + var stored string + if err := inv.store.Pool().QueryRow(ctx, `select built_contexts::text from build where id = 'said'`).Scan(&stored); err != nil { + t.Fatal(err) + } + if strings.Contains(stored, "paths") || strings.Contains(stored, "own") { + t.Fatalf("the build source was stored among what the build read: %s", stored) + } +} diff --git a/internal/inventory/dependencies.go b/internal/inventory/dependencies.go index 7384ceca..26d07385 100644 --- a/internal/inventory/dependencies.go +++ b/internal/inventory/dependencies.go @@ -13,7 +13,10 @@ import ( const ( // EdgeStandsOn: the module's artifact is built on the other's. EdgeStandsOn = "stands-on" - // EdgePackages: the module's build reads the other's repository. + // EdgePackages: the module's build read the other's repository. **No longer drawn** (novox/hq ADR 0267 + // rule 4): sharing a repository is no dependency, and a changed file moves every module whose build + // source holds it, directly. Kept so an edge recorded or snapshotted before is read, and then neither + // widens nor orders a plan. EdgePackages = "packages" // EdgeBuiltBy: the module is built by the holder of the build-machine seat. EdgeBuiltBy = "built-by" @@ -42,10 +45,10 @@ type Edge struct { // nothing else computes an edge (novox/hq ADR 0162) — the merge handler, `build --on` and the // overview all read this. // -// Four sources, one relation: a manifest's `build.on`; the artifacts the latest build was made -// against (an `artifact-store:///…` reference is an edge to that module); the repositories -// the latest build read (an edge to the module whose source that is); and the build machine, which -// every source-built module is built by. +// Three sources, one relation: a manifest's `build.on`; the artifacts the latest build was made +// against (an `artifact-store:///…` reference is an edge to that module); and the build machine, +// which every source-built module is built by. The repositories a build read are no edge (novox/hq ADR +// 0267 rule 4). func (i *Inventory) Dependencies(ctx context.Context) ([]Edge, error) { entries, err := i.Catalogued(ctx) if err != nil { @@ -55,24 +58,19 @@ func (i *Inventory) Dependencies(ctx context.Context) ([]Edge, error) { if err != nil { return nil, err } - read, err := i.ReadRepositories(ctx) - if err != nil { - return nil, err - } - return dependenciesOf(entries, against, read), nil + return dependenciesOf(entries, against, nil), nil } // dependenciesOf is Dependencies over what was read, so a test can hand it a catalogue. -func dependenciesOf(entries []Entry, against map[string][]string, read map[string][]ReadRepository) []Edge { +// +// `read` draws no edge (novox/hq ADR 0267 rule 4): what a build read moves it through its build source, in +// the planner, never through the relation. +func dependenciesOf(entries []Entry, against map[string][]string, _ map[string][]ReadRepository) []Edge { known := map[string]bool{} - byRepository := map[string][]string{} var builders []string for _, e := range entries { name := e.Manifest.Module known[name] = true - if r := repositoryKey(e.Source.Repository); r != "" { - byRepository[r] = append(byRepository[r], name) - } if e.Manifest.ClaimsSeat("node-build-agent") || e.Manifest.ClaimsSeat("mesh-build-machine") { builders = append(builders, name) } @@ -125,11 +123,6 @@ func dependenciesOf(entries []Entry, against map[string][]string, read map[strin } } } - for _, r := range read[name] { - for _, other := range byRepository[repositoryKey(r.Repository)] { - add(name, other, EdgePackages) - } - } if e.Source.Repository != "" { for _, b := range builders { add(name, b, EdgeBuiltBy) @@ -154,8 +147,3 @@ func dependenciesOf(entries []Entry, against map[string][]string, read map[strin }) return out } - -// repositoryKey is a repository as compared: lower-cased, without a trailing `.git`. -func repositoryKey(repository string) string { - return strings.ToLower(strings.TrimSuffix(strings.TrimSpace(repository), ".git")) -} diff --git a/internal/inventory/dependencies_test.go b/internal/inventory/dependencies_test.go index cda50102..39286193 100644 --- a/internal/inventory/dependencies_test.go +++ b/internal/inventory/dependencies_test.go @@ -44,7 +44,6 @@ func TestDependenciesAreOneRelationWithTheirKinds(t *testing.T) { {"shop", "mesh-tools", EdgeStandsOn}, {"builder", "mesh-tools", EdgeStandsOn}, {"shop-plugin", "shop", EdgeDeclared}, - {"route-proxy", "mesh-controller", EdgePackages}, {"shop", "builder", EdgeBuiltBy}, {"mesh-controller", "builder", EdgeBuiltBy}, {"mesh-tools", "builder", EdgeBuiltBy}, @@ -53,6 +52,10 @@ func TestDependenciesAreOneRelationWithTheirKinds(t *testing.T) { t.Errorf("missing %+v in %+v", want, got) } } + // Sharing a repository is no dependency (novox/hq ADR 0267 rule 4): what a build read draws no edge. + if has("route-proxy", "mesh-controller", EdgePackages) { + t.Error("a packages edge was drawn from what a build read") + } if has("builder", "builder", EdgeBuiltBy) { t.Error("the builder is not built by itself") } diff --git a/internal/inventory/migrations/0088-a-build-says-its-build-source.sql b/internal/inventory/migrations/0088-a-build-says-its-build-source.sql new file mode 100644 index 00000000..319ae571 --- /dev/null +++ b/internal/inventory/migrations/0088-a-build-says-its-build-source.sql @@ -0,0 +1,9 @@ +-- A build says what it was made from, as files (novox/hq ADR 0267, issue 363). +-- +-- A merge to a repository moved every module whose build read it, whatever the files: the controller, +-- built from its repository's root, and the two images whose context is that repository (issue 338). +-- A build of a trunk commit now says its build source per repository — a Go program's import closure, +-- an archive's directory, an image's recipe — and the planner maps the next merge's changed files onto +-- the build source of the module's newest build. Null for a build that said none (one off the trunk, +-- one from a builder that predates this, one whose source is not known): its module is read as before. +alter table build add column build_sources jsonb;