Skip to content

Commit

Permalink
cmd/cueckoo: do not over-run trybots
Browse files Browse the repository at this point in the history
In case we already have a trybot result for the requested change, there
is no real point requesting them again, unless in exceptional
circumstances or we want to force the run.

We can use labels to conditionally trigger the trybots.

This short-circuit can be overridden by passing the --force (or -f for
short) flag to runtrybot.

Otherwise, the trybots are skipped if the revision against which we have
requested the trybots is the current revision, and Run-TryBot == +1.
Otherwise they are run.

Tested against this CL!

Signed-off-by: Paul Jolly <paul@myitcv.io>
Change-Id: Ia8ad5b61b6e3754918b73cd0aad39b5818c4fab6
Reviewed-on: https://review.gerrithub.io/c/cue-sh/tools/+/1169892
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
  • Loading branch information
myitcv committed Sep 25, 2023
1 parent caf725e commit eab1552
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
28 changes: 27 additions & 1 deletion cmd/cueckoo/cmd/cltrigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func (c *cltrigger) triggerBuilds(revs []revision) error {

func (c *cltrigger) triggerBuild(rev revision) error {
in, _, err := c.cfg.gerritClient.Changes.GetChange(rev.changeID, &gerrit.ChangeOptions{
AdditionalFields: []string{"ALL_REVISIONS"},
AdditionalFields: []string{"ALL_REVISIONS", "LABELS"},
})
if err != nil {
// Note that this may be a "change not found" error when the changeID is
Expand All @@ -262,6 +262,32 @@ func (c *cltrigger) triggerBuild(rev revision) error {
return fmt.Errorf("change %q does not know about revision %q; did you forget to run git codereview mail?", rev.changeID, commit)
}

// If we do not have the --force flag, only trigger trybots when we do not
// have a result for the trybots.
//
// Labels are attached to the change itself, not revisions. There is logic
// to reset labels when new revisions are added, logic that removes the
// TryBot-Result when a new patchset is added.
//
// So to be safe, we can only skip the trybots if the revision we requested
// is the current (latest) revision, and the change in question has
// TryBotResult == +1.
isCurrent := rev.revision == in.CurrentRevision
if isCurrent && !flagForce.Bool(c.cmd) {
// Order is significant here; check the request for a trybot first
if tbResult, ok := in.Labels["TryBot-Result"]; ok {
for _, approval := range tbResult.All {
// We are looking for a score of 1. Repo config limits the
// people who can vote on this label, hence we don't care
// who actually voted because it can only have been someone
// with permission to do so.
if approval.Value == 1 {
return nil
}
}
}
}

return c.builder(repositoryDispatchPayload{
CL: in.Number,
Patchset: revision.Number,
Expand Down
2 changes: 2 additions & 0 deletions cmd/cueckoo/cmd/runtrybot.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
const (
flagChange flagName = "change"
flagRunTrybotNoUnity flagName = "nounity"
flagForce flagName = "force"
)

// newRuntrybotCmd creates a new runtrybot command
Expand Down Expand Up @@ -60,6 +61,7 @@ If the --nounity flag is provided, only a trybot run is triggered.
}
cmd.Flags().Bool(string(flagChange), false, "interpret arguments as change numbers or IDs")
cmd.Flags().Bool(string(flagRunTrybotNoUnity), false, "do not simultaenously trigger unity build")
cmd.Flags().BoolP(string(flagForce), string(flagForce[0]), false, "force the trybots to run, ignoring any results")
return cmd
}

Expand Down

0 comments on commit eab1552

Please sign in to comment.