Say no cycle for a packages edge in a plan's last tier

A packages edge orders nothing, so a module and what packages its source
share a tier by rule; hasCycle counted the edge and the merge handler said
"the last tier depends on itself" of plans with no cycle. Skip the kind as
tiersOf does. The planner tests' cycle rows and property now pass.
This commit is contained in:
jochen
2026-10-08 22:22:02 +02:00
parent 8ca4b04321
commit add807f034
2 changed files with 12 additions and 12 deletions
+6 -11
View File
@@ -218,14 +218,9 @@ func TestAPlanIsWhatTheChangeTouchedAndWhatIsBuiltOnIt(t *testing.T) {
if u := strings.Join(r.Unread, ","); u != c.unread {
t.Errorf("%s: unread %q, wanted %q", c.what, u, c.unread)
}
// BUG found by this test on main (8170fc5): hasCycle reads any edge between two modules of the last
// tier as a cycle — a packages edge too, which puts both there by rule — so the merge handler logs
// "the last tier depends on itself … built together, in no order" for a plan with no cycle (the
// packages rows and the mixed diamond fail here). ADR 0162: a cycle is one last tier, *and said*;
// one said where there is none is the rule broken. The expectation stays.
// A packages edge in the last tier is no cycle; hasCycle said one on main at 8170fc5.
if hasCycle(r.Plan.Tiers, c.edges) != c.cycle {
t.Errorf("%s: a cycle said %v, wanted %v (%v) [BUG: hasCycle counts a packages edge]",
c.what, !c.cycle, c.cycle, r.Plan.Tiers)
t.Errorf("%s: a cycle said %v, wanted %v (%v)", c.what, !c.cycle, c.cycle, r.Plan.Tiers)
}
}
}
@@ -310,7 +305,7 @@ var sharedRepositoryEdges = []inventory.Edge{
// packages — never along 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 (this one fails on main: see the BUG note below).
// - no cycle is said.
//
// Seeded, so a failure is replayed by its seed and case.
func TestAPlanIsTheTouchedModulesAndWhatIsReachableAlongTheWideningEdges(t *testing.T) {
@@ -413,10 +408,10 @@ func TestAPlanIsTheTouchedModulesAndWhatIsReachableAlongTheWideningEdges(t *test
}
}
}
// BUG found by this property on main (8170fc5); see the table's cycle check. Said once, after the
// other invariants have run over every case, so it hides none of them.
// Said once, after the other invariants have run over every case, so it hides none of them (hasCycle
// counted a packages edge on main at 8170fc5: 19 of these 500 cases).
if falseCycles > 0 {
t.Errorf("a cycle said of a graph with none in %d of 500 cases [BUG: hasCycle counts a packages edge]; "+
t.Errorf("a cycle said of a graph with none in %d of 500 cases; "+
"the first:\n%s", falseCycles, firstFalseCycle)
}
}
+6 -1
View File
@@ -157,7 +157,9 @@ func reachableFrom(moved []string, edges []inventory.Edge) []string {
return out
}
// hasCycle says whether the tiers' last tier holds modules that still depend on each other.
// hasCycle says whether the tiers' last tier holds modules that still depend on each other. A packages
// edge orders nothing (tiersOf), so a module and what packages its source share a tier by rule: that is
// no cycle, and saying one was is a false report in every such plan's log.
func hasCycle(tiers [][]string, edges []inventory.Edge) bool {
if len(tiers) == 0 {
return false
@@ -167,6 +169,9 @@ func hasCycle(tiers [][]string, edges []inventory.Edge) bool {
last[m] = true
}
for _, e := range edges {
if e.Kind == inventory.EdgePackages {
continue
}
if last[e.From] && last[e.To] {
return true
}