From 140d723d60887791938a7fe6fc3dde0ddd515ce9 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Fri, 8 Apr 2022 16:30:52 -0700 Subject: [PATCH] cmd/gopherbot: autosubmit stacks in the right order When a change in a stack is labelled with Auto-Submit+1, only submit the change if all of the changes below have already been merged (or been abandoned.) Additionally, if any of the parent changes are based on a revision of their parent which is no longer the current revision (i.e. in the Gerrit UI it is marked as 'Not current'), we will not submit the change. This makes sure that changes are submitted in the expected order. Updates #48021 Change-Id: I9a178551843f64f8b752468ce0a091cb3cf2577d Reviewed-on: https://go-review.googlesource.com/c/build/+/399043 Reviewed-by: Russ Cox Reviewed-by: Bryan Mills Run-TryBot: Roland Shoemaker TryBot-Result: Gopher Robot --- cmd/gopherbot/gopherbot.go | 35 +++++++++++++++++++++++++++++++++++ gerrit/gerrit.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/cmd/gopherbot/gopherbot.go b/cmd/gopherbot/gopherbot.go index 5d2c13b7..28c4c297 100644 --- a/cmd/gopherbot/gopherbot.go +++ b/cmd/gopherbot/gopherbot.go @@ -2462,6 +2462,41 @@ func (b *gopherbot) autoSubmitCLs(ctx context.Context) error { return nil } + // If this change is part of a stack, we'd like to merge the stack + // in the correct order (i.e. from the bottom of the stack to the + // top), so we'll only merge the current change if every change + // below it in the stack is either merged, or abandoned. If a parent + // change is no longer current (the revision of the change that the + // child change is based on is no longer the current revision of + // that change) we won't merge the child. GetRelatedChanges gives us + // the stack from top to bottom (the order of the git commits, from + // newest to oldest, see Gerrit documentation for + // RelatedChangesInfo), so first we find our change in the stack, + // then check everything below it. + relatedChanges, err := b.gerrit.GetRelatedChanges(ctx, fmt.Sprint(cl.Number), "current") + if err != nil { + return err + } + if len(relatedChanges.Changes) > 0 { + var parentChanges bool + for _, ci := range relatedChanges.Changes { + if !parentChanges { + // Skip everything before the change we are checking, as + // they are the children of this change, and we only care + // about the parents. + parentChanges = ci.ChangeNumber == cl.Number + continue + } + if ci.Status != gerrit.ChangeStatusAbandoned && + ci.Status != gerrit.ChangeStatusMerged { + return nil + } + if ci.CurrentRevisionNumber != ci.RevisionNumber { + return nil + } + } + } + if *dryRun { log.Printf("[dry-run] would've submitted CL https://golang.org/cl/%d ...", cl.Number) return nil diff --git a/gerrit/gerrit.go b/gerrit/gerrit.go index 4b6cccce..93de7486 100644 --- a/gerrit/gerrit.go +++ b/gerrit/gerrit.go @@ -1045,3 +1045,31 @@ func (c *Client) GetRevisionActions(ctx context.Context, changeID, revision stri err := c.do(ctx, &actions, "GET", "/changes/"+changeID+"/revisions/"+revision+"/actions") return actions, err } + +// RelatedChangeAndCommitInfo contains information about a particular +// change at a particular commit. +// +// See https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#related-change-and-commit-info. +type RelatedChangeAndCommitInfo struct { + Project string `json:"project"` + ChangeID string `json:"change_id"` + ChangeNumber int32 `json:"_change_number"` + Commit CommitInfo `json:"commit"` + Status string `json:"status"` + RevisionNumber int32 `json:"_revision_number"` + CurrentRevisionNumber int32 `json:"_current_revision_number"` +} + +// RelatedChangesInfo contains information about a set of related changes. +// +// See https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#related-changes-info. +type RelatedChangesInfo struct { + Changes []RelatedChangeAndCommitInfo `json:"changes"` +} + +// GetRelatedChanges retrieves information about a set of related changes. +func (c *Client) GetRelatedChanges(ctx context.Context, changeID, revision string) (*RelatedChangesInfo, error) { + var changes *RelatedChangesInfo + err := c.do(ctx, &changes, "GET", "/changes/"+changeID+"/revisions/"+revision+"/related") + return changes, err +}