diff --git a/cmd/cueckoo/cmd/cltrigger.go b/cmd/cueckoo/cmd/cltrigger.go index 2e62a29..9f837ac 100644 --- a/cmd/cueckoo/cmd/cltrigger.go +++ b/cmd/cueckoo/cmd/cltrigger.go @@ -45,36 +45,58 @@ func newCLTrigger(cmd *Command, cfg *config, b builder) *cltrigger { } } +// Per https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#change-id, +// a change ID can be in multiple forms. +// We only really care about CL numbers and Change-ID identifiers from git commit trailers, +// since those are what a human user is most likely going to find useful. +// The long forms, like "project~branch~I1234..." are far too cumbersome. +var rxChangeID = regexp.MustCompile(`^[1-9][0-9]{3,}|I[0-9a-f]{40}$`) + func (c *cltrigger) run() (err error) { var changeIDs []revision - args := make(map[string]bool) - for _, a := range c.cmd.Flags().Args() { - args[a] = true + args := c.cmd.Flags().Args() + derive := true + for _, arg := range args { + if rxChangeID.MatchString(arg) { + derive = false + } else if !derive { + return fmt.Errorf("cannot mix change IDs and git refs") + } } - if flagChange.Bool(c.cmd) { + if derive { + changeIDs, err = c.deriveChangeIDs(args) + if err != nil { + return err + } + } else { if len(args) == 0 { return fmt.Errorf("must provide at least one change number of ID") } - for a := range args { + for _, a := range args { changeIDs = append(changeIDs, revision{ changeID: a, }) } - } else { - changeIDs, err = c.deriveChangeIDs(args) - if err != nil { - return err - } } return c.triggerBuilds(changeIDs) } +// TODO: replace once we can use slices.Contains +func slicesContains[S ~[]E, E comparable](s S, v E) bool { + for i := range s { + if v == s[i] { + return true + } + } + return false +} + // deriveChangeIDs determines a list of change IDs for the supplied args (if // there are any). See the trybot docs for an explanation of what is derived. // Essentially however we try to follow the semantics of git-codereview: // // https://pkg.go.dev/golang.org/x/review/git-codereview -func (c *cltrigger) deriveChangeIDs(args map[string]bool) (res []revision, err error) { +func (c *cltrigger) deriveChangeIDs(args []string) (res []revision, err error) { ctx := context.TODO() // Work out the branchpoint bp, err := run(ctx, "git", "codereview", "branchpoint") @@ -91,10 +113,11 @@ func (c *cltrigger) deriveChangeIDs(args map[string]bool) (res []revision, err e if len(pendingCommits) == 0 { return nil, fmt.Errorf("no pending commits") } - if args["HEAD"] && len(args) > 1 { + argHead := slicesContains(args, "HEAD") + if argHead && len(args) > 1 { return nil, fmt.Errorf("HEAD can only be supplied as an argument by itself") } - if !args["HEAD"] && len(pendingCommits) > 1 && len(args) == 0 { + if !argHead && len(pendingCommits) > 1 && len(args) == 0 { return nil, fmt.Errorf("must specify commits as arguments or use HEAD for everything") } addRevision := func(pc commit) error { @@ -124,7 +147,7 @@ func (c *cltrigger) deriveChangeIDs(args map[string]bool) (res []revision, err e }) return nil } - if args["HEAD"] || len(args) == 0 && len(pendingCommits) == 1 { + if argHead || len(args) == 0 && len(pendingCommits) == 1 { // Verify all for _, pc := range pendingCommits { if err := addRevision(pc); err != nil { @@ -137,7 +160,7 @@ func (c *cltrigger) deriveChangeIDs(args map[string]bool) (res []revision, err e // We verify each of the arguments EachArg: - for h := range args { + for _, h := range args { // Resolve the arg and ensure we have a matching pending commit // and ensure we have a single one commits, err := resolveCommits(ctx, "-1", h) @@ -177,6 +200,9 @@ func resolveCommits(ctx context.Context, args ...string) ([]commit, error) { if err != nil { return nil, err } + if commitStream == "" { + return nil, fmt.Errorf("%q resulted in no git commits", args) + } var commits []commit diff --git a/cmd/cueckoo/cmd/runtrybot.go b/cmd/cueckoo/cmd/runtrybot.go index 764d26a..0edee7a 100644 --- a/cmd/cueckoo/cmd/runtrybot.go +++ b/cmd/cueckoo/cmd/runtrybot.go @@ -22,7 +22,6 @@ import ( ) const ( - flagChange flagName = "change" flagRunTrybotNoUnity flagName = "nounity" flagForce flagName = "force" ) @@ -35,23 +34,18 @@ func newRuntrybotCmd(c *Command) *cobra.Command { Long: ` Usage of runtrybot: - runtrybot [--change] [--nounity] [ARGS...] + runtrybot [--nounity] [ARGS...] Triggers trybot and unity runs for its arguments. When run with no arguments, runtrybot derives a revision and change ID for each pending commit in the current branch. If multiple pending commits are found, -you must either specify which commits to run, or specify HEAD to run the +you must either specify which commits or CLs to run, or specify HEAD to run the trybots for all of them. -If the --change flag is provide, then the list of arguments is interpreted as -change numbers or IDs, and the latest revision from each of those changes is -assumed. - -runtrybot requires GITHUB_USER and GITHUB_PAT environment variables to be set -with your GitHub username and personal acccess token respectively. The personal -access token requires the "repo" scope, since Unity is a private repository. - +runtrybot needs your GitHub username and a personal acccess token +with the "repo" scope. You can configure them via your git credential helper, +or by setting the GITHUB_USER and GITHUB_PAT environment variables. Note that the personal access token should be "classic"; GitHub's new fine-grained tokens are still in beta and haven't been tested to work here. @@ -59,7 +53,6 @@ If the --nounity flag is provided, only a trybot run is triggered. `, RunE: mkRunE(c, runtrybotDef), } - 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 diff --git a/cmd/cueckoo/cmd/unity.go b/cmd/cueckoo/cmd/unity.go index d5e03f7..c9f709b 100644 --- a/cmd/cueckoo/cmd/unity.go +++ b/cmd/cueckoo/cmd/unity.go @@ -45,13 +45,14 @@ unity for all of them. If the --normal flag is provided, then the list of arguments is interpreted as versions understood by unity. -unity requires GITHUB_USER and GITHUB_PAT environment variables to be set with -your GitHub username and personal acccess token respectively. The personal -access token requires the "repo" scope, since Unity is a private repository. +runtrybot needs your GitHub username and a personal acccess token +with the "repo" scope. You can configure them via your git credential helper, +or by setting the GITHUB_USER and GITHUB_PAT environment variables. +Note that the personal access token should be "classic"; GitHub's new +fine-grained tokens are still in beta and haven't been tested to work here. `, RunE: mkRunE(c, unityDef), } - cmd.Flags().Bool(string(flagChange), false, "interpret arguments as change numbers or IDs") cmd.Flags().Bool(string(flagUnityVersions), false, "pass arguments to unity as versions") return cmd } @@ -62,10 +63,6 @@ func unityDef(cmd *Command, args []string) error { return err } - if flagUnityVersions.Bool(cmd) && flagChange.Bool(cmd) { - return fmt.Errorf("cannot supply --change and --versions") - } - // If we are passed --normal, interpret all args as versions to be passed to // unity if flagUnityVersions.Bool(cmd) {