From 6784efae756721a335eb0496ebca35964bed8f09 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 3 Oct 2026 22:22:08 +0200 Subject: [PATCH] A commit is never a branch to follow (hq issue 215) A build asked at a commit recorded that commit as the module's ref. Every merge after it failed to match the module and its plan left it out without a word, and every plan that rebuilt it asked for the same old commit again. Registration now keeps the branch the module followed (the default branch for a new one); matching and re-asking read a recorded commit as the default branch, which heals records already pinned this way; and a merge says which modules of its repository it leaves out because they follow another branch. --- cmd/mesh-controller/build.go | 9 +++++++ cmd/mesh-controller/build_test.go | 37 +++++++++++++++++++++++++++++ cmd/mesh-controller/order_test.go | 24 +++++++++++++++++++ cmd/mesh-controller/release_plan.go | 3 ++- cmd/mesh-controller/upgrades.go | 28 +++++++++++++++++++++- 5 files changed, 99 insertions(+), 2 deletions(-) diff --git a/cmd/mesh-controller/build.go b/cmd/mesh-controller/build.go index 56f7afb..452a74e 100644 --- a/cmd/mesh-controller/build.go +++ b/cmd/mesh-controller/build.go @@ -530,6 +530,15 @@ func takeIn(ctx context.Context, inv *inventory.Inventory, result link.BuildResu if result.Source != nil && result.Source.Seat != "" { recorded.Repository, recorded.Seat = result.Source.Repository, result.Source.Seat } + // **A build at a commit does not change the branch a module follows** (novox/hq 04-ISSUES/215): + // the commit is built and recorded as what it was built from, and the module keeps following + // what it followed before — the repository's default branch for one new to the catalogue. + if followedBranch(result.Ref) == "" && result.Ref != "" { + recorded.Ref = "" + if was, err := inv.SourceOf(ctx, manifest.Module); err == nil { + recorded.Ref = followedBranch(was.Ref) + } + } if err := namesNoInstallation(manifest); err != nil { return manifest, kept, fmt.Errorf("%s built %s (%s), and the mesh does not register it: %w", result.On, result.Repository, short(result.Commit), err) diff --git a/cmd/mesh-controller/build_test.go b/cmd/mesh-controller/build_test.go index a39ef6e..29d0d73 100644 --- a/cmd/mesh-controller/build_test.go +++ b/cmd/mesh-controller/build_test.go @@ -63,3 +63,40 @@ func TestABuildHeardIsRecordedAndRegistered(t *testing.T) { t.Fatalf("a failure is said in the builder's words: %v", err) } } + +// novox/hq 04-ISSUES/215: a build asked at a commit is recorded as built from that commit, and the +// module keeps following the branch it followed — a new one, the default branch. +func TestABuildAtACommitKeepsTheBranchTheModuleFollows(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + manifest, _ := json.Marshal(map[string]any{"module": "unifi", "version": "1"}) + result := func(id, ref, commit string) link.BuildResult { + return link.BuildResult{ID: id, Repository: "http://forge.internal:20000/novox/mesh-catalog.git", + Path: "modules/unifi", Ref: ref, On: "anchor", Commit: commit, Manifest: manifest, + Source: &link.SourceOnSeat{Seat: "git", Repository: "novox/mesh-catalog"}} + } + if _, _, err := takeIn(ctx, open.inventory, result("b-1", "main", "1111111aaaa")); err != nil { + t.Fatal(err) + } + if _, _, err := takeIn(ctx, open.inventory, result("b-2", "9c97a8a", "9c97a8a1d2c3")); err != nil { + t.Fatal(err) + } + src, err := open.inventory.SourceOf(ctx, "unifi") + if err != nil { + t.Fatal(err) + } + if src.Ref != "main" || src.BuiltFrom != "9c97a8a1d2c3" { + t.Errorf("after a build at a commit the module follows %q, built from %q; want main, 9c97a8a1d2c3", src.Ref, src.BuiltFrom) + } + + // One new to the catalogue, first built at a commit, follows the default branch. + other, _ := json.Marshal(map[string]any{"module": "letta", "version": "1"}) + r := result("b-3", "deadbeef", "deadbeefcafe") + r.Manifest, r.Path = other, "modules/letta" + if _, _, err := takeIn(ctx, open.inventory, r); err != nil { + t.Fatal(err) + } + if src, _ := open.inventory.SourceOf(ctx, "letta"); src.Ref != "" { + t.Errorf("a module first built at a commit follows %q, want the default branch", src.Ref) + } +} diff --git a/cmd/mesh-controller/order_test.go b/cmd/mesh-controller/order_test.go index be2c4d0..2acac30 100644 --- a/cmd/mesh-controller/order_test.go +++ b/cmd/mesh-controller/order_test.go @@ -211,3 +211,27 @@ func TestWhatAHandedOverModuleRecordsAboutItsSource(t *testing.T) { } } } + +// novox/hq 04-ISSUES/215: a module once built at a commit still follows its branch — a merge into it +// matches the module, and a plan re-asks the branch, not the old commit. +func TestAModuleBuiltAtACommitStillFollowsItsBranch(t *testing.T) { + m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main"} + pinned := inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Ref: "9c97a8a"} + if !sourceIs(pinned, m) { + t.Error("a module whose record names a commit is left out of a merge into its branch") + } + full := inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Ref: "9c97a8a1d2c3b4a5f60718293a4b5c6d7e8f9012"} + if !sourceIs(full, m) { + t.Error("a full commit hash is read as a branch") + } + if got := followedBranch("9c97a8a"); got != "" { + t.Errorf("a plan would re-ask the old commit %q", got) + } + if got := followedBranch("release"); got != "release" { + t.Errorf("a branch is not followed as named: %q", got) + } + // A module that follows another branch is still not this merge's. + if sourceIs(inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Ref: "release"}, m) { + t.Error("a module following another branch was matched") + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 438ea38..3cf4c0c 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -272,7 +272,8 @@ func askTier(ctx context.Context, inv *inventory.Inventory, p *inventory.Plan) e } source := buildSource{Repository: e.Source.Repository, Seat: e.Source.Seat} fmt.Printf(" tier %d: ", p.Tier) - if err := buildOne(ctx, source, e.Source.Path, e.Source.Ref, 0); err != nil { + // The branch it follows, never a commit a build once named (novox/hq 04-ISSUES/215). + if err := buildOne(ctx, source, e.Source.Path, followedBranch(e.Source.Ref), 0); err != nil { state.State = "failed" state.Why = err.Error() p.State = inventory.PlanFailed diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index 0aa9ffd..b9d9cd2 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -5,6 +5,7 @@ import ( "errors" "flag" "fmt" + "regexp" "strings" "time" @@ -283,6 +284,14 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { if isHistory(m.MergedAt, lastLookAt(entries, m)) { packaging = nil } + // Said, never silent (novox/hq 04-ISSUES/215): a module built from this repository that follows + // another branch is not part of this merge, and whoever is waiting for its change should read why. + for _, e := range entries { + if sameRepository(e.Source.Repository, m) && !sourceIs(e.Source, m) { + fmt.Printf(" %s is built from %s/%s and follows %s, not %s; this merge leaves it out\n", + e.Manifest.Module, m.Owner, m.Repo, e.Source.Ref, m.Base) + } + } touched := whatTheMergeTouched(from, entries, m) for _, e := range touched { if err := inv.SourceMoved(ctx, e.Manifest.Module, m.Commit); err != nil { @@ -345,7 +354,24 @@ func sourceIs(s inventory.Source, m link.SourceMoved) bool { if !sameRepository(s.Repository, m) { return false } - return s.Ref == "" || s.Ref == m.Base + ref := followedBranch(s.Ref) + return ref == "" || ref == m.Base +} + +// commitRef is a ref that names a commit rather than a branch: what `build --ref ` asks for. +var commitRef = regexp.MustCompile(`^[0-9a-f]{7,40}$`) + +// followedBranch is the branch a recorded ref means a module follows (novox/hq 04-ISSUES/215). **A +// commit is never a branch to follow.** A build asked at a commit — to try one, or to pin it during a +// fix — recorded that commit as the module's ref; every merge after it then failed to match the +// module, its plan left it out without saying so, and every plan that rebuilt it asked for that same +// old commit again. A commit recorded so is read as the repository's default branch, which is what +// the module followed before it; a branch is followed as named. +func followedBranch(ref string) string { + if commitRef.MatchString(strings.TrimSpace(ref)) { + return "" + } + return ref } // sameRepository is whether a recorded repository is the one a merge names, in either spelling it