A newer delivered revision covers the older merges of its repository, as the forge says ancestry (issue 1946)
This commit is contained in:
@@ -94,3 +94,40 @@ func TestTheSameRevisionTwiceIsOnceAndTheGatesOwnDeliveredIsNoWord(t *testing.T)
|
||||
t.Fatalf("delivered %v, %d ready-to-verify", is.Delivered, countEvents(h, EventReadyToVerify))
|
||||
}
|
||||
}
|
||||
|
||||
// "A newer revision covers older ones": the view says each revision once and may skip one delivered together with a later one,
|
||||
// so a merged fix whose merge is an ancestor of the head the event holds for its repository is delivered too, as the forge says.
|
||||
func TestANewerRevisionCoversTheOlderMergesOfItsRepositoryAsTheForgeSaysAncestry(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
older := fixIssue(t, h, "mesh-catalog#30")
|
||||
newer := fixIssue(t, h, "mesh-catalog#31")
|
||||
unrelated := fixIssue(t, h, "mesh-catalog#32")
|
||||
mergedFix(t, h, 30, "feat/a", "1111111111110000")
|
||||
mergedFix(t, h, 31, "feat/b", "2222222222220000")
|
||||
mergedFix(t, h, 32, "feat/c", "3333333333330000")
|
||||
var asked []string
|
||||
h.Contains = func(owner, repo, commit, in string) (bool, error) {
|
||||
asked = append(asked, owner+"/"+repo+" "+commit+" in "+in)
|
||||
return commit == "1111111111110000", nil // only the first is an ancestor of the revision's head
|
||||
}
|
||||
viewSays(t, h, 12, "feat/b", map[string]string{"novox/mesh-catalog": "2222222222220000"})
|
||||
for n, want := range map[uint64]string{older: "delivered to every machine it should reach",
|
||||
newer: "delivered to every machine it should reach", unrelated: "fix merged, not delivered: mesh-catalog#32"} {
|
||||
if v, _ := h.View(n); v.Delivery != want {
|
||||
t.Errorf("issue %d reads %q, wanted %q (asked %v)", n, v.Delivery, want, asked)
|
||||
}
|
||||
}
|
||||
// A forge that cannot say leaves the fix for the next event, and a repository the revision holds no head for is not asked.
|
||||
h2 := newHarness(t)
|
||||
n := fixIssue(t, h2, "mesh-catalog#33")
|
||||
mergedFix(t, h2, 33, "feat/d", "4444444444440000")
|
||||
h2.Contains = func(string, string, string, string) (bool, error) { return false, errStuck }
|
||||
viewSays(t, h2, 13, "feat/z", map[string]string{"novox/mesh-catalog": "5555555555550000"})
|
||||
if v, _ := h2.View(n); v.Delivery != "fix merged, not delivered: mesh-catalog#33" {
|
||||
t.Fatalf("a forge that could not say delivered it: %q", v.Delivery)
|
||||
}
|
||||
viewSays(t, h2, 14, "feat/z", map[string]string{"novox/mesh-sdk": "5555555555550000"})
|
||||
if v, _ := h2.View(n); v.Delivery != "fix merged, not delivered: mesh-catalog#33" {
|
||||
t.Fatalf("another repository's head covered it: %q", v.Delivery)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -427,9 +427,9 @@ const DeliveredEvent = "mesh-delivery-view.delivered"
|
||||
|
||||
// OnDelivered is the handler of the view's `delivered` event: {revision, group, heads}. Every merged fix of a pull request the
|
||||
// group moved, or whose merge is the head the revision holds for its repository, reached the machines at that revision. The same
|
||||
// revision twice changes nothing (a delivered fix stays delivered), and a fix is matched by the group it merged in or by its own
|
||||
// merge commit, so a revision that carries a later head of a repository does not cover an earlier merge the tracker never heard
|
||||
// delivered: that waits for its own revision's event.
|
||||
// revision twice changes nothing (a delivered fix stays delivered). A fix is matched by the group it merged in, by its own merge
|
||||
// commit, or because its merge is an ancestor of the head the revision holds for its repository (coveredBy): a newer revision
|
||||
// covers the older ones, which the view may never have said.
|
||||
func (t *Tracker) OnDelivered(e stdio.Envelope) error {
|
||||
var body struct {
|
||||
Revision uint64 `json:"revision"`
|
||||
@@ -453,7 +453,7 @@ func (t *Tracker) OnDelivered(e stdio.Envelope) error {
|
||||
for n, is := range all {
|
||||
for _, pr := range liveFixes(is) {
|
||||
f := fixOf(is, pr)
|
||||
if f == nil || !f.Merged || contains(is.Delivered, pr) || !deliveredBy(f, body.Group, body.Heads) {
|
||||
if f == nil || !f.Merged || contains(is.Delivered, pr) || !(deliveredBy(f, body.Group, body.Heads) || t.coveredBy(f, body.Heads)) {
|
||||
continue
|
||||
}
|
||||
pr := pr
|
||||
@@ -482,6 +482,57 @@ func (t *Tracker) OnDelivered(e stdio.Envelope) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// busContains asks the forge's gitea_contains over the bus: whether commit is an ancestor of in, or it.
|
||||
func busContains(owner, repo, commit, in string) (bool, error) {
|
||||
raw, err := stdio.Ask("gitea.gitea_contains", map[string]any{"owner": owner, "repo": repo, "commit": commit, "in": in})
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
var out struct {
|
||||
Contains *bool `json:"contains"`
|
||||
Answer struct {
|
||||
Contains *bool `json:"contains"`
|
||||
} `json:"answer"`
|
||||
}
|
||||
if json.Unmarshal(raw, &out) != nil {
|
||||
return false, errors.New("the forge's gitea_contains is not readable")
|
||||
}
|
||||
switch {
|
||||
case out.Answer.Contains != nil:
|
||||
return *out.Answer.Contains, nil
|
||||
case out.Contains != nil:
|
||||
return *out.Contains, nil
|
||||
}
|
||||
return false, errors.New("the forge's gitea_contains answered no verdict")
|
||||
}
|
||||
|
||||
// coveredBy says whether a merged fix's merge is an ancestor of, or equal to, the head a delivered revision holds for its
|
||||
// repository: the view says each revision once and may skip one delivered together with a later one, so a newer revision
|
||||
// covers the older merges of its repository. The forge decides ancestry; a forge that cannot say leaves the fix for the next event.
|
||||
func (t *Tracker) coveredBy(f *Fix, heads map[string]string) bool {
|
||||
if t.Contains == nil || f.MergedAs == "" {
|
||||
return false
|
||||
}
|
||||
for repository, head := range heads {
|
||||
if !strings.EqualFold(repository, f.Repository) {
|
||||
continue
|
||||
}
|
||||
owner, repo, ok := strings.Cut(repository, "/")
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
yes, err := t.Contains(owner, repo, f.MergedAs, head)
|
||||
if err != nil {
|
||||
t.Logf("[mesh-issues] whether %s contains %s is not known yet: %v", shortCommit(head), shortCommit(f.MergedAs), err)
|
||||
continue
|
||||
}
|
||||
return yes
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func shortCommit(c string) string { return c[:min(len(c), 12)] }
|
||||
|
||||
// deliveredBy says whether a merged fix is one a delivered revision names: the group it merged in is the group that moved the
|
||||
// source, or its merge commit is the head the revision holds for its repository.
|
||||
func deliveredBy(f *Fix, group string, heads map[string]string) bool {
|
||||
|
||||
@@ -105,6 +105,7 @@ func main() {
|
||||
return busDeliveryState(forgeOwner())(repository, number)
|
||||
}
|
||||
t.Approval = busApproval(forgeOwner)
|
||||
t.Contains = busContains
|
||||
if len(os.Args) > 1 && os.Args[1] == "dry-run" {
|
||||
os.Exit(dryRunAtTheTerminal(t, os.Args[2:]))
|
||||
}
|
||||
|
||||
@@ -112,7 +112,7 @@ func TestItDeclaresAndHoldsTheIssueTrackerSeatOnceForTheMesh(t *testing.T) {
|
||||
// the import and a reservation read the record through the records module (issue 424).
|
||||
if !reflect.DeepEqual(m.Consumes, []string{"mesh-issues." + EventOpened, "mesh-issues." + EventMoved, "mesh-delivery-gate.transition", DeliveredEvent}) ||
|
||||
!reflect.DeepEqual(m.Invokes, []string{"claude-code.claude_code_judge", "seat:mesh-delivery.checks", "seat:operator-channel.notify",
|
||||
"gitea.gitea_pull_reviews", "records.records_status", "records.records_list", "records.records_read", "mesh-knowledge.knowledge_highest"}) {
|
||||
"gitea.gitea_pull_reviews", "gitea.gitea_contains", "records.records_status", "records.records_list", "records.records_read", "mesh-knowledge.knowledge_highest"}) {
|
||||
t.Fatalf("consumes %v, invokes %v", m.Consumes, m.Invokes)
|
||||
}
|
||||
// The tools bundle, and the archive the build packs files/ into for the templates (novox/hq ADR 0349 §2).
|
||||
|
||||
@@ -36,6 +36,9 @@ type Tracker struct {
|
||||
Judge Judgement
|
||||
// Deliveries answers a pull request's delivery (ADR 0279 §3, ADR 0281); nil asks nothing.
|
||||
Deliveries DeliveryState
|
||||
// Contains asks the forge whether a commit is contained in another (gitea_contains); nil asks nothing, and a delivered
|
||||
// revision then marks only the fixes it names (OnDelivered).
|
||||
Contains func(owner, repo, commit, in string) (bool, error)
|
||||
// Notify tells the operator's channel of a stuck fix (ADR 0281) and of a fix over its budget (ADR 0313); nil
|
||||
// says nothing there.
|
||||
Notify Notifier
|
||||
|
||||
@@ -65,6 +65,7 @@
|
||||
"seat:mesh-delivery.checks",
|
||||
"seat:operator-channel.notify",
|
||||
"gitea.gitea_pull_reviews",
|
||||
"gitea.gitea_contains",
|
||||
"records.records_status",
|
||||
"records.records_list",
|
||||
"records.records_read",
|
||||
|
||||
Reference in New Issue
Block a user