Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion cmd/gopherbot/gopherbot.go
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,29 @@ func (b *gopherbot) labelBuildIssues(ctx context.Context) error {
})
}

// longestPrefixMatch finds the compiler/runtime package from crtPackages
// that is the longest prefix of pkg. It resolves the issue where sub-packages
// like "cmd/compile/internal/noder" are not explicitly listed in the owners
// file but clearly belong to the compiler team (because "cmd/compile" is).
//
// Returns true for exact matches, and for prefix matches where the candidate
// is a proper ancestor of pkg (pkg has a '/' immediately after the prefix).
func longestPrefixMatch(pkg string, crtPackages map[string]struct{}) bool {
if _, ok := crtPackages[pkg]; ok {
return true
}
var best string
for candidate := range crtPackages {
if strings.HasPrefix(pkg, candidate) &&
(len(candidate) == len(pkg) || pkg[len(candidate)] == '/') {
if len(candidate) > len(best) {
best = candidate
}
}
}
return best != ""
}

func (b *gopherbot) labelCompilerRuntimeIssues(ctx context.Context) error {
entries, err := getAllCodeOwners(ctx)
if err != nil {
Expand Down Expand Up @@ -1140,7 +1163,7 @@ func (b *gopherbot) labelCompilerRuntimeIssues(ctx context.Context) error {
return nil
}
for p := range strings.SplitSeq(strings.TrimSpace(components[0]), ",") {
if _, ok := crtPackages[strings.TrimSpace(p)]; !ok {
if !longestPrefixMatch(strings.TrimSpace(p), crtPackages) {
continue
}
// TODO(mknyszek): Add this issue to the GitHub project as well.
Expand Down
89 changes: 89 additions & 0 deletions cmd/gopherbot/gopherbot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -669,3 +669,92 @@ func TestForeachIssue(t *testing.T) {
t.Errorf("got %+v, want all true", got)
}
}

func TestLongestPrefixMatch(t *testing.T) {
// Simulate the crtPackages set as built by labelCompilerRuntimeIssues.
crtPackages := map[string]struct{}{
"cmd/compile": {},
"cmd/compile/internal/amd64": {},
"cmd/compile/internal/ssa": {},
"cmd/compile/internal/syntax": {},
"cmd/compile/internal/types": {},
"cmd/link": {},
"runtime": {},
"runtime/metrics": {},
"runtime/pprof": {},
"internal/abi": {},
"internal/runtime/atomic": {},
"x/sys/unix": {},
"cmd/asm": {},
}

tests := []struct {
pkg string
want bool
desc string
}{
// Exact matches should still work.
{"cmd/compile", true, "exact match: cmd/compile"},
{"runtime", true, "exact match: runtime"},
{"runtime/metrics", true, "exact match: runtime/metrics"},
{"x/sys/unix", true, "exact match: x/sys/unix"},
{"internal/abi", true, "exact match: internal/abi"},

// Prefix matches: sub-packages not explicitly in owners but under known packages.
{"cmd/compile/internal/noder", true, "prefix match: cmd/compile/internal/noder -> cmd/compile"},
{"cmd/compile/internal/ir", true, "prefix match: cmd/compile/internal/ir -> cmd/compile"},
{"cmd/compile/internal/noder/reader", true, "prefix match: cmd/compile/internal/noder/reader -> cmd/compile"},
{"cmd/link/internal/ld", true, "prefix match: cmd/link/internal/ld -> cmd/link"},
{"runtime/internal/sys", true, "prefix match: runtime/internal/sys -> runtime"},
{"runtime/internal/atomic", true, "prefix match: runtime/internal/atomic -> runtime"},
{"internal/runtime/atomic/value", true, "prefix match: internal/runtime/atomic/value -> internal/runtime/atomic"},
{"runtime/metric", true, "prefix match: runtime/metric -> runtime (1 level below)"},
{"x/sys/unix/foo", true, "prefix match: x/sys/unix/foo -> x/sys/unix"},
{"cmd/asm/internal/arch", true, "prefix match: cmd/asm/internal/arch -> cmd/asm"},
{"runtime/pprof/internal", true, "prefix match: runtime/pprof/internal -> runtime/pprof"},
{"cmd/compile/internal/noder/a/b/c", true, "prefix match: deeply nested (6 levels) -> cmd/compile"},

// No match: packages not under any compiler/runtime-owned prefix.
{"net/http", false, "no match: net/http not in crtPackages"},
{"fmt", false, "no match: fmt not in crtPackages"},
{"cmd/go", false, "no match: cmd/go not in crtPackages"},
{"os", false, "no match: os not in crtPackages"},
{"cmd/vet", false, "no match: cmd/vet not in crtPackages"},

// No match: package that shares prefix with a known package but is NOT a child.
// "cmd/compilefoo" starts with "cmd/compile" but "cmd/compile" is not an ancestor
// (no '/' after "cmd/compile" in "cmd/compilefoo").
{"cmd/compilefoo", false, "no match: cmd/compilefoo is not under cmd/compile (no trailing /)"},

// Empty string.
{"", false, "no match: empty string"},

// Prefer longer prefix match.
{"runtime/metrics/something", true, "prefix match: prefers runtime/metrics over runtime"},
{"internal/runtime/atomic/something", true, "prefix match: prefers internal/runtime/atomic over internal/abi"},
}

for _, tt := range tests {
got := longestPrefixMatch(tt.pkg, crtPackages)
if got != tt.want {
t.Errorf("longestPrefixMatch(%q) = %v, want %v — %s", tt.pkg, got, tt.want, tt.desc)
}
}
}

func TestLongestPrefixMatchEmpty(t *testing.T) {
// Edge case: empty crtPackages should never match.
empty := map[string]struct{}{}
tests := []string{
"cmd/compile",
"runtime",
"net/http",
"",
"cmd/compile/internal/noder",
}
for _, pkg := range tests {
if longestPrefixMatch(pkg, empty) {
t.Errorf("longestPrefixMatch(%q, empty) = true, want false", pkg)
}
}
}
97 changes: 91 additions & 6 deletions cmd/updatestd/updatestd.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// updatestd is an experimental program that has been used to update
// the standard library modules as part of golang.org/issue/36905 in
// CL 255860 and CL 266898. It's expected to be modified to meet the
// ongoing needs of that recurring maintenance work.
// updatestd is a command used to update dependency versions
// in the Go standard library as part of go.dev/issue/36905.
// It's expected to be modified to meet the ongoing needs of
// that recurring maintenance work.
package main

import (
Expand All @@ -17,6 +17,7 @@ import (
"flag"
"fmt"
"go/ast"
"go/format"
"go/parser"
"go/token"
"io"
Expand Down Expand Up @@ -139,6 +140,41 @@ func main() {
//
log.Println("updating bundles in", r.dir)
r.run(goCmd, "generate", "-run=bundle", "std", "cmd")

// Update the golang.org/x/pkgsite/cmd/internal/doc version used
// as the underlying implementation of 'go doc -http'.
if goVersion == 25 { // TODO: Delete this case after Go 1.25 stops being supported.
log.Println(`skipping updating x/pkgsite/cmd/internal/doc version:

on Go 1.25 release branch, this maintenance is done manually by editing the version constant in doPkgsite:

$EDITOR src/cmd/internal/doc/main.go
(see https://cs.opensource.google/go/go/+/release-branch.go1.25:src/cmd/internal/doc/main.go;l=246;drc=b4309ece66ca989a38ed65404850a49ae8f92742)

automated updatestd maintenance support currently applies to Go 1.27+`)
} else if goVersion == 26 { // TODO: Delete this case after Go 1.26 stops being supported.
log.Println(`skipping updating x/pkgsite/cmd/internal/doc version:

on Go 1.26 release branch, this maintenance is done manually by editing the version constant in doPkgsite:

$EDITOR src/cmd/go/internal/doc/pkgsite.go
(see https://cs.opensource.google/go/go/+/release-branch.go1.26:src/cmd/go/internal/doc/pkgsite.go;l=74;drc=866e461b9689d03dbbf2df19b86cace21270865b)

automated updatestd maintenance support currently applies to Go 1.27+`)
} else if pkgsiteHash := hashes["pkgsite"]; pkgsiteHash != "" {
log.Println("updating x/pkgsite/cmd/internal/doc version to", pkgsiteHash)
out := r.runOut(goCmd, "list", "-mod=readonly", "-m", "-json", "golang.org/x/pkgsite/cmd/internal/doc@"+pkgsiteHash)
var mod struct{ Version string }
if err := json.Unmarshal(out, &mod); err != nil {
log.Fatalf("error parsing go list -m -json output: %v", err)
}
err := editPkgsiteVersion(*goroot, mod.Version)
if err != nil {
log.Fatalln(err)
}
} else {
log.Println("skipping updating x/pkgsite/cmd/internal/doc version because x/pkgsite doesn't have a branch named", *branch)
}
}

type Work struct {
Expand Down Expand Up @@ -235,7 +271,7 @@ func gorootVersion(goroot string) (int, error) {
fset := token.NewFileSet()
f, err := parser.ParseFile(fset, filepath.Join(goroot, "src", "internal", "goversion", "goversion.go"), nil, 0)
if os.IsNotExist(err) {
return 0, fmt.Errorf("did not find goversion.go file (%v); wrong goroot or did internal/goversion package change?", err)
return 0, fmt.Errorf("did not find goversion.go file (%v); wrong goroot or internal/goversion package changed", err)
} else if err != nil {
return 0, err
}
Expand All @@ -253,10 +289,59 @@ func gorootVersion(goroot string) (int, error) {
if !ok || l.Kind != token.INT {
continue
}
// Found it.
return strconv.Atoi(l.Value)
}
}
return 0, fmt.Errorf("did not find Version declaration in %s; wrong goroot or did internal/goversion package change?", fset.File(f.Pos()).Name())
return 0, fmt.Errorf("did not find Version declaration in %s; wrong goroot or internal/goversion package changed", fset.File(f.Pos()).Name())
}

// editPkgsiteVersion reads the GOROOT/src/cmd/go/internal/doc/pkgsite.go
// file and edits the pkgsiteCmdInternalDocVersion constant found therein.
func editPkgsiteVersion(goroot, version string) error {
// Parse the pkgsite.go file, extract the declaration from the AST,
// edit it in place and overwrite the file.
//
// This is a pragmatic approach that relies on the trajectory of the
// cmd/go/internal/doc package being predictable and in our control.
// This small helper is easy to replace with something else if it
// becomes desirable to start using another approach for this task.
//
filename := filepath.Join(goroot, "src", "cmd", "go", "internal", "doc", "pkgsite.go")
fset := token.NewFileSet()
f, err := parser.ParseFile(fset, filename, nil, parser.ParseComments)
if os.IsNotExist(err) {
return fmt.Errorf("did not find pkgsite.go file (%v); wrong goroot or cmd/go/internal/doc package changed", err)
} else if err != nil {
return err
}
for _, d := range f.Decls {
g, ok := d.(*ast.GenDecl)
if !ok {
continue
}
for _, s := range g.Specs {
v, ok := s.(*ast.ValueSpec)
if !ok || len(v.Names) != 1 || v.Names[0].String() != "pkgsiteCmdInternalDocVersion" || len(v.Values) != 1 {
continue
}
l, ok := v.Values[0].(*ast.BasicLit)
if !ok || l.Kind != token.STRING {
continue
}

// Found it.
// Edit its value and overwrite the existing file.
l.Value = fmt.Sprintf("%q", version)
var buf bytes.Buffer
err := format.Node(&buf, fset, f)
if err != nil {
return err
}
return os.WriteFile(filename, buf.Bytes(), 0) // file already exists, so perm is unused
}
}
return fmt.Errorf("did not find pkgsiteCmdInternalDocVersion declaration in %s; wrong goroot or cmd/go/internal/doc package changed", fset.File(f.Pos()).Name())
}

type runner struct{ dir string }
Expand Down