cmd/stacks: match predicates against individual configurations

claimStacks currently concatenates all configurations associated with a
stack before evaluating a predicate. As a result, different terms of a
conjunction may match different configurations. For example, a stack
observed on linux/amd64 and darwin/arm64 incorrectly matches a predicate
requiring linux/arm64.

Evaluate the predicate separately for each stack/configuration pair, and
claim the stack if any individual configuration matches. Add coverage
for cross-configuration combinations.

Fixes golang/go#80660

Change-Id: Ie5200bab1d238dc85ef11c778eb682ce63b348e3
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/808383
Reviewed-by: race quite <quiterace@gmail.com>
Reviewed-by: Hongxiang Jiang <hxjiang@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Hongxiang Jiang <hxjiang@golang.org>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

cmd/stacks/stacks.go

diff --git a/cmd/stacks/stacks.go b/cmd/stacks/stacks.go
index 453a215..7141b5c 100644
--- a/cmd/stacks/stacks.go
+++ b/cmd/stacks/stacks.go
@@ -41,9 +41,9 @@
 //     >       | expr && expr
 //     >       | expr || expr
 //
-//     Each string literal must match complete words in the text,
-//     which consists of stack followed by one or more configuration
-//     lines of the form
+//     Each string literal must match complete words in the text.
+//     The predicate is evaluated separately for each configuration,
+//     against text consisting of the stack followed by one line of the form
 //
 //     GOTOOLCHAIN=go1.23.0 GOOS=darwin GOARCH=arm64 golang.org/x/tools/gopls@v0.22.0
 //
@@ -588,33 +588,24 @@
 //     that appear in the body by chance.
 //
 //  2. if the issue body contains a ```#!stacks``` predicate that
-//     matches the text, consisting of stack and configuration lines.
+//     matches the stack together with at least one of its configurations.
 //
 // We log an error if two different issues attempt to claim
 // the same stack.
 func claimStacks(issues []*Issue, stacks map[string]map[Info]int64) map[string]*Issue {
 	log.Println("Processing claims...")
 
-	// This is O(new stacks x existing issues).
+	// This is O(new stacks x existing issues x stack configurations).
 	claimedBy := make(map[string]*Issue)
 	claimType := make(map[string]bool) // records whether claim was due to predicate (true) or ID (false) in issue body
 	for stack, counts := range stacks {
 		id := stackID(stack)
 
-		// Construct the stack + configuration text to be matched.
-		var buf strings.Builder
-		buf.WriteString(stack)
-		for info := range counts {
-			fmt.Fprintf(&buf, "\nGOTOOLCHAIN=%s GOOS=%s GOARCH=%s %s@%s",
-				info.GoVersion, info.GOOS, info.GOARCH, info.Program, info.ProgramVersion)
-		}
-		text := buf.String()
-
 		for _, issue := range issues {
 			byPredicate := false
 			if strings.Contains(issue.Body, id) {
 				// nop
-			} else if issue.matches != nil && issue.matches(text) {
+			} else if issue.matches != nil && matchesAnyConfig(issue.matches, stack, counts) {
 				byPredicate = true
 				if false {
 					log.Printf("predicate %s matches stack %s", issue.predicate, id)
@@ -649,6 +640,19 @@
 	return claimedBy
 }
 
+// matchesAnyConfig reports whether match accepts the stack in at least one of
+// the configurations in counts.
+func matchesAnyConfig(match func(string) bool, stack string, counts map[Info]int64) bool {
+	for info := range counts {
+		text := fmt.Sprintf("%s\nGOTOOLCHAIN=%s GOOS=%s GOARCH=%s %s@%s",
+			stack, info.GoVersion, info.GOOS, info.GOARCH, info.Program, info.ProgramVersion)
+		if match(text) {
+			return true
+		}
+	}
+	return false
+}
+
 // updateIssues updates existing issues that claimed new stacks by predicate.
 func updateIssues(cli *githubClient, repo string, issues []*Issue, stacks map[string]map[Info]int64, stackToURL map[string]string) {
 	log.Println("Updating issues...")

cmd/stacks/stacks_test.go

diff --git a/cmd/stacks/stacks_test.go b/cmd/stacks/stacks_test.go
index 2af7b49..c191d38 100644
--- a/cmd/stacks/stacks_test.go
+++ b/cmd/stacks/stacks_test.go
@@ -387,3 +387,36 @@
 		t.Errorf("issue #2 newStacks = %v, want [%q]", issues[1].newStacks, stack2)
 	}
 }
+
+func TestClaimStacksMultipleConfigs(t *testing.T) {
+	const stack = "runtime.main"
+	stacks := map[string]map[Info]int64{
+		stack: {
+			{GOOS: "linux", GOARCH: "amd64"}:  1,
+			{GOOS: "darwin", GOARCH: "arm64"}: 1,
+		},
+	}
+
+	for _, test := range []struct {
+		name      string
+		predicate string
+		want      bool
+	}{
+		{"linux/amd64", `"GOOS=linux" && "GOARCH=amd64"`, true},
+		{"darwin/arm64", `"GOOS=darwin" && "GOARCH=arm64"`, true},
+		{"linux/arm64", `"GOOS=linux" && "GOARCH=arm64"`, false},
+		{"darwin/amd64", `"GOOS=darwin" && "GOARCH=amd64"`, false},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			match, err := parsePredicate(test.predicate)
+			if err != nil {
+				t.Fatal(err)
+			}
+			issue := &Issue{Number: 1, predicate: test.predicate, matches: match}
+			claimed := claimStacks([]*Issue{issue}, stacks)
+			if got := claimed[stackID(stack)] == issue; got != test.want {
+				t.Errorf("claimStacks with predicate %q claimed stack: %t, want %t", test.predicate, got, test.want)
+			}
+		})
+	}
+}