From 891e3b67e3bfc42021ccf399dba2300c097bbed3 Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Fri, 24 Jan 2025 14:35:43 -0500 Subject: [PATCH] internal/telemetry/cmd/stacks: cmd/compile reopen Support re-opening issues for the compiler. Fixes golang/go#71045. Change-Id: I6e5dad81220c74923919b4c72f7bc1089af6c37d Reviewed-on: https://go-review.googlesource.com/c/tools/+/644018 Reviewed-by: Alan Donovan Auto-Submit: Jonathan Amsterdam LUCI-TryBot-Result: Go LUCI TryBot-Result: Gopher Robot --- gopls/internal/telemetry/cmd/stacks/stacks.go | 49 ++++++++++++++++--- .../telemetry/cmd/stacks/stacks_test.go | 34 ++++++++++--- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/gopls/internal/telemetry/cmd/stacks/stacks.go b/gopls/internal/telemetry/cmd/stacks/stacks.go index fca230e3acd..64b71606272 100644 --- a/gopls/internal/telemetry/cmd/stacks/stacks.go +++ b/gopls/internal/telemetry/cmd/stacks/stacks.go @@ -626,13 +626,23 @@ func shouldReopen(issue *Issue, stacks map[string]map[Info]int64) bool { if !ok { return false } - // TODO(jba?): handle other programs - if issueProgram != "gopls" { - return false + + matchProgram := func(infoProg string) bool { + switch issueProgram { + case "gopls": + return path.Base(infoProg) == issueProgram + case "go": + // At present, we only care about compiler stacks. + // Issues should have milestones like "Go1.24". + return infoProg == "cmd/compile" + default: + return false + } } + for _, stack := range issue.newStacks { for info := range stacks[stack] { - if path.Base(info.Program) == issueProgram && semver.Compare(info.ProgramVersion, issueVersion) >= 0 { + if matchProgram(info.Program) && semver.Compare(semVer(info.ProgramVersion), issueVersion) >= 0 { log.Printf("reopening issue #%d: purportedly fixed in %s@%s, but found a new stack from version %s", issue.Number, issueProgram, issueVersion, info.ProgramVersion) return true @@ -647,12 +657,23 @@ func (i *Issue) isFixed() bool { return i.State == "closed" && i.StateReason == "completed" } -// parseMilestone parses a the title of a GitHub milestone that is in the format -// PROGRAM/VERSION. For example, "gopls/v0.17.0". +// parseMilestone parses a the title of a GitHub milestone. +// If it is in the format PROGRAM/VERSION (for example, "gopls/v0.17.0"), +// then it returns PROGRAM and VERSION. +// If it is in the format Go1.X, then it returns "go" as the program and +// "v1.X" or "v1.X.0" as the version. +// Otherwise, the last return value is false. func parseMilestone(m *Milestone) (program, version string, ok bool) { if m == nil { return "", "", false } + if strings.HasPrefix(m.Title, "Go") { + v := semVer(m.Title) + if !semver.IsValid(v) { + return "", "", false + } + return "go", v, true + } program, version, ok = morestrings.CutLast(m.Title, "/") if !ok || program == "" || version == "" || version[0] != 'v' { return "", "", false @@ -660,6 +681,22 @@ func parseMilestone(m *Milestone) (program, version string, ok bool) { return program, version, true } +// semVer returns a semantic version for its argument, which may already be +// a semantic version, or may be a Go version. +// +// v1.2.3 => v1.2.3 +// go1.24 => v1.24 +// Go1.23.5 => v1.23.5 +// goHome => vHome +// +// It returns "", false if the go version is in the wrong format. +func semVer(v string) string { + if strings.HasPrefix(v, "go") || strings.HasPrefix(v, "Go") { + return "v" + v[2:] + } + return v +} + // stackID returns a 32-bit identifier for a stack // suitable for use in GitHub issue titles. func stackID(stack string) string { diff --git a/gopls/internal/telemetry/cmd/stacks/stacks_test.go b/gopls/internal/telemetry/cmd/stacks/stacks_test.go index 1f3cbef1f29..452113a1581 100644 --- a/gopls/internal/telemetry/cmd/stacks/stacks_test.go +++ b/gopls/internal/telemetry/cmd/stacks/stacks_test.go @@ -269,7 +269,8 @@ func TestMarshalUpdateIssueFields(t *testing.T) { func TestShouldReopen(t *testing.T) { const stack = "stack" const gopls = "golang.org/x/tools/gopls" - const milestoneVersion = "v0.2.0" + goplsMilestone := &Milestone{Title: "gopls/v0.2.0"} + goMilestone := &Milestone{Title: "Go1.23"} for _, tc := range []struct { name string @@ -279,44 +280,61 @@ func TestShouldReopen(t *testing.T) { }{ { "issue open", - Issue{State: "open"}, + Issue{State: "open", Milestone: goplsMilestone}, Info{Program: gopls, ProgramVersion: "v0.2.0"}, false, }, { "issue closed but not fixed", - Issue{State: "closed", StateReason: "not_planned"}, + Issue{State: "closed", StateReason: "not_planned", Milestone: goplsMilestone}, Info{Program: gopls, ProgramVersion: "v0.2.0"}, false, }, { "different program", - Issue{State: "closed", StateReason: "completed"}, + Issue{State: "closed", StateReason: "completed", Milestone: goplsMilestone}, Info{Program: "other", ProgramVersion: "v0.2.0"}, false, }, { "later version", - Issue{State: "closed", StateReason: "completed"}, + Issue{State: "closed", StateReason: "completed", Milestone: goplsMilestone}, Info{Program: gopls, ProgramVersion: "v0.3.0"}, true, }, { "earlier version", - Issue{State: "closed", StateReason: "completed"}, + Issue{State: "closed", StateReason: "completed", Milestone: goplsMilestone}, Info{Program: gopls, ProgramVersion: "v0.1.0"}, false, }, { "same version", - Issue{State: "closed", StateReason: "completed"}, + Issue{State: "closed", StateReason: "completed", Milestone: goplsMilestone}, Info{Program: gopls, ProgramVersion: "v0.2.0"}, true, }, + { + "compiler later version", + Issue{State: "closed", StateReason: "completed", Milestone: goMilestone}, + Info{Program: "cmd/compile", ProgramVersion: "go1.24"}, + true, + }, + { + "compiler earlier version", + Issue{State: "closed", StateReason: "completed", Milestone: goMilestone}, + Info{Program: "cmd/compile", ProgramVersion: "go1.22"}, + false, + }, + { + "compiler same version", + Issue{State: "closed", StateReason: "completed", Milestone: goMilestone}, + Info{Program: "cmd/compile", ProgramVersion: "go1.23"}, + true, + }, } { t.Run(tc.name, func(t *testing.T) { tc.issue.Number = 1 - tc.issue.Milestone = &Milestone{Title: "gopls/" + milestoneVersion} tc.issue.newStacks = []string{stack} got := shouldReopen(&tc.issue, map[string]map[Info]int64{stack: map[Info]int64{tc.info: 1}}) if got != tc.want {