Skip to content

Commit 0a8055e

Browse files
rscgopherbot
authored andcommitted
cmd/api: rewrite as package test
No one ever runs 'go tool api', because the invocation has gotten unwieldy enough that it's not practical. And we don't support it as a standalone tool for other packages - it's not even in the distribution. Making it an ordinary package test lets us invoke it more easily from cmd/dist (as go test cmd/api -check) and avoids the increasingly baroque code in run.go to build a command line. Left in cmd/api even though it's no longer a command because (1) it uses a package from cmd/vendor and (2) it uses internal/testenv. Otherwise it could be misc/api. Fixes #56845. Change-Id: I00a13d9c19b1e259fa0e6bb93d1a4dca25f0e8c1 Reviewed-on: https://go-review.googlesource.com/c/go/+/453258 Auto-Submit: Russ Cox <rsc@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com> Run-TryBot: Russ Cox <rsc@golang.org>
1 parent 76ec47e commit 0a8055e

File tree

5 files changed

+87
-218
lines changed

5 files changed

+87
-218
lines changed

src/cmd/api/goapi.go renamed to src/cmd/api/api.go

Lines changed: 54 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
// Api computes the exported API of a set of Go packages.
6-
package main
5+
// Package api computes the exported API of a set of Go packages.
6+
// It is only a test, not a command, nor a usefully importable package.
7+
package api
78

89
import (
910
"bufio"
@@ -16,6 +17,7 @@ import (
1617
"go/parser"
1718
"go/token"
1819
"go/types"
20+
"internal/testenv"
1921
"io"
2022
"log"
2123
"os"
@@ -27,33 +29,23 @@ import (
2729
"strconv"
2830
"strings"
2931
"sync"
32+
"testing"
3033
)
3134

35+
const verbose = false
36+
3237
func goCmd() string {
3338
var exeSuffix string
3439
if runtime.GOOS == "windows" {
3540
exeSuffix = ".exe"
3641
}
37-
if goroot := build.Default.GOROOT; goroot != "" {
38-
path := filepath.Join(goroot, "bin", "go"+exeSuffix)
39-
if _, err := os.Stat(path); err == nil {
40-
return path
41-
}
42+
path := filepath.Join(testenv.GOROOT(nil), "bin", "go"+exeSuffix)
43+
if _, err := os.Stat(path); err == nil {
44+
return path
4245
}
4346
return "go"
4447
}
4548

46-
// Flags
47-
var (
48-
checkFiles = flag.String("c", "", "optional comma-separated filename(s) to check API against")
49-
requireApproval = flag.String("approval", "", "require approvals in comma-separated list of `files`")
50-
allowNew = flag.Bool("allow_new", true, "allow API additions")
51-
exceptFile = flag.String("except", "", "optional filename of packages that are allowed to change without triggering a failure in the tool")
52-
nextFiles = flag.String("next", "", "comma-separated list of `files` for upcoming API features for the next release. These files can be lazily maintained. They only affects the delta warnings from the -c file printed on success.")
53-
verbose = flag.Bool("v", false, "verbose debugging")
54-
forceCtx = flag.String("contexts", "", "optional comma-separated list of <goos>-<goarch>[-cgo] to override default contexts.")
55-
)
56-
5749
// contexts are the default contexts which are scanned, unless
5850
// overridden by the -contexts flag.
5951
var contexts = []*build.Context{
@@ -117,36 +109,25 @@ func parseContext(c string) *build.Context {
117109
return bc
118110
}
119111

120-
func setContexts() {
121-
contexts = []*build.Context{}
122-
for _, c := range strings.Split(*forceCtx, ",") {
123-
contexts = append(contexts, parseContext(c))
124-
}
125-
}
126-
127112
var internalPkg = regexp.MustCompile(`(^|/)internal($|/)`)
128113

129114
var exitCode = 0
130115

131-
func main() {
132-
log.SetPrefix("api: ")
133-
log.SetFlags(0)
134-
flag.Parse()
135-
136-
if build.Default.GOROOT == "" {
137-
log.Fatalf("GOROOT not found. (If binary was built with -trimpath, $GOROOT must be set.)")
116+
func Check(t *testing.T) {
117+
checkFiles, err := filepath.Glob(filepath.Join(testenv.GOROOT(t), "api/go1*.txt"))
118+
if err != nil {
119+
t.Fatal(err)
138120
}
139121

140-
if !strings.Contains(runtime.Version(), "weekly") && !strings.Contains(runtime.Version(), "devel") {
141-
if *nextFiles != "" {
142-
fmt.Printf("Go version is %q, ignoring -next %s\n", runtime.Version(), *nextFiles)
143-
*nextFiles = ""
122+
var nextFiles []string
123+
if strings.Contains(runtime.Version(), "devel") {
124+
next, err := filepath.Glob(filepath.Join(testenv.GOROOT(t), "api/next/*.txt"))
125+
if err != nil {
126+
t.Fatal(err)
144127
}
128+
nextFiles = next
145129
}
146130

147-
if *forceCtx != "" {
148-
setContexts()
149-
}
150131
for _, c := range contexts {
151132
c.Compiler = build.Default.Compiler
152133
}
@@ -158,7 +139,7 @@ func main() {
158139
wg.Add(1)
159140
go func() {
160141
defer wg.Done()
161-
walkers[i] = NewWalker(context, filepath.Join(build.Default.GOROOT, "src"))
142+
walkers[i] = NewWalker(context, filepath.Join(testenv.GOROOT(t), "src"))
162143
}()
163144
}
164145
wg.Wait()
@@ -204,40 +185,29 @@ func main() {
204185
}
205186

206187
bw := bufio.NewWriter(os.Stdout)
207-
defer func() {
208-
bw.Flush()
209-
if exitCode != 0 {
210-
os.Exit(exitCode)
211-
}
212-
}()
213-
214-
if *checkFiles == "" {
215-
sort.Strings(features)
216-
for _, f := range features {
217-
fmt.Fprintln(bw, f)
218-
}
219-
return
220-
}
188+
defer bw.Flush()
221189

222190
var required []string
223-
for _, file := range strings.Split(*checkFiles, ",") {
224-
required = append(required, fileFeatures(file)...)
191+
for _, file := range checkFiles {
192+
required = append(required, fileFeatures(file, needApproval(file))...)
225193
}
226194
var optional []string
227-
if *nextFiles != "" {
228-
for _, file := range strings.Split(*nextFiles, ",") {
229-
optional = append(optional, fileFeatures(file)...)
230-
}
195+
for _, file := range nextFiles {
196+
optional = append(optional, fileFeatures(file, true)...)
231197
}
232-
exception := fileFeatures(*exceptFile)
233-
if !compareAPI(bw, features, required, optional, exception, *allowNew) {
234-
exitCode = 1
198+
exception := fileFeatures(filepath.Join(testenv.GOROOT(t), "api/except.txt"), false)
199+
200+
if exitCode == 1 {
201+
t.Errorf("API database problems found")
202+
}
203+
if !compareAPI(bw, features, required, optional, exception, false) {
204+
t.Errorf("API differences found")
235205
}
236206
}
237207

238208
// export emits the exported package features.
239209
func (w *Walker) export(pkg *types.Package) {
240-
if *verbose {
210+
if verbose {
241211
log.Println(pkg)
242212
}
243213
pop := w.pushScope("pkg " + pkg.Path())
@@ -353,17 +323,7 @@ var aliasReplacer = strings.NewReplacer(
353323
"os.PathError", "fs.PathError",
354324
)
355325

356-
func fileFeatures(filename string) []string {
357-
if filename == "" {
358-
return nil
359-
}
360-
needApproval := false
361-
for _, name := range strings.Split(*requireApproval, ",") {
362-
if filename == name {
363-
needApproval = true
364-
break
365-
}
366-
}
326+
func fileFeatures(filename string, needApproval bool) []string {
367327
bs, err := os.ReadFile(filename)
368328
if err != nil {
369329
log.Fatal(err)
@@ -406,6 +366,11 @@ func fileFeatures(filename string) []string {
406366
exitCode = 1
407367
}
408368
line = strings.TrimSpace(feature)
369+
} else {
370+
if strings.Contains(line, " #") {
371+
log.Printf("%s:%d: unexpected approval\n", filename, i+1)
372+
exitCode = 1
373+
}
409374
}
410375
nonblank = append(nonblank, line)
411376
}
@@ -1140,7 +1105,20 @@ func (w *Walker) emitf(format string, args ...any) {
11401105
}
11411106
w.features[f] = true
11421107

1143-
if *verbose {
1108+
if verbose {
11441109
log.Printf("feature: %s", f)
11451110
}
11461111
}
1112+
1113+
func needApproval(filename string) bool {
1114+
name := filepath.Base(filename)
1115+
if name == "go1.txt" {
1116+
return false
1117+
}
1118+
minor := strings.TrimSuffix(strings.TrimPrefix(name, "go1."), ".txt")
1119+
n, err := strconv.Atoi(minor)
1120+
if err != nil {
1121+
log.Fatalf("unexpected api file: %v", name)
1122+
}
1123+
return n >= 19 // started tracking approvals in Go 1.19
1124+
}

src/cmd/api/goapi_test.go renamed to src/cmd/api/api_test.go

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
package main
5+
package api
66

77
import (
88
"flag"
@@ -17,6 +17,8 @@ import (
1717
"testing"
1818
)
1919

20+
var flagCheck = flag.Bool("check", false, "run API checks")
21+
2022
func TestMain(m *testing.M) {
2123
if !testenv.HasExec() {
2224
os.Stdout.WriteString("skipping test: platform cannot exec")
@@ -40,7 +42,7 @@ func TestMain(m *testing.M) {
4042
wg.Add(1)
4143
go func() {
4244
defer wg.Done()
43-
_ = NewWalker(context, filepath.Join(build.Default.GOROOT, "src"))
45+
_ = NewWalker(context, filepath.Join(testenv.GOROOT(nil), "src"))
4446
}()
4547
}
4648
wg.Wait()
@@ -53,6 +55,10 @@ var (
5355
)
5456

5557
func TestGolden(t *testing.T) {
58+
if *flagCheck {
59+
// slow, not worth repeating in -check
60+
t.Skip("skipping with -check set")
61+
}
5662
td, err := os.Open("testdata/src/pkg")
5763
if err != nil {
5864
t.Fatal(err)
@@ -194,7 +200,7 @@ func TestSkipInternal(t *testing.T) {
194200
func BenchmarkAll(b *testing.B) {
195201
for i := 0; i < b.N; i++ {
196202
for _, context := range contexts {
197-
w := NewWalker(context, filepath.Join(build.Default.GOROOT, "src"))
203+
w := NewWalker(context, filepath.Join(testenv.GOROOT(b), "src"))
198204
for _, name := range w.stdPackages {
199205
pkg, _ := w.Import(name)
200206
w.export(pkg)
@@ -205,6 +211,10 @@ func BenchmarkAll(b *testing.B) {
205211
}
206212

207213
func TestIssue21181(t *testing.T) {
214+
if *flagCheck {
215+
// slow, not worth repeating in -check
216+
t.Skip("skipping with -check set")
217+
}
208218
for _, context := range contexts {
209219
w := NewWalker(context, "testdata/src/issue21181")
210220
pkg, err := w.Import("p")
@@ -217,6 +227,10 @@ func TestIssue21181(t *testing.T) {
217227
}
218228

219229
func TestIssue29837(t *testing.T) {
230+
if *flagCheck {
231+
// slow, not worth repeating in -check
232+
t.Skip("skipping with -check set")
233+
}
220234
for _, context := range contexts {
221235
w := NewWalker(context, "testdata/src/issue29837")
222236
_, err := w.Import("p")
@@ -227,9 +241,13 @@ func TestIssue29837(t *testing.T) {
227241
}
228242

229243
func TestIssue41358(t *testing.T) {
244+
if *flagCheck {
245+
// slow, not worth repeating in -check
246+
t.Skip("skipping with -check set")
247+
}
230248
context := new(build.Context)
231249
*context = build.Default
232-
context.Dir = filepath.Join(context.GOROOT, "src")
250+
context.Dir = filepath.Join(testenv.GOROOT(t), "src")
233251

234252
w := NewWalker(context, context.Dir)
235253
for _, pkg := range w.stdPackages {
@@ -238,3 +256,10 @@ func TestIssue41358(t *testing.T) {
238256
}
239257
}
240258
}
259+
260+
func TestCheck(t *testing.T) {
261+
if !*flagCheck {
262+
t.Skip("-check not specified")
263+
}
264+
Check(t)
265+
}

src/cmd/api/goapi_boring_test.go renamed to src/cmd/api/boring_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
//go:build boringcrypto
66

7-
package main
7+
package api
88

99
import (
1010
"fmt"

0 commit comments

Comments
 (0)