cmd/api: disallow silent API additions after api/go1.n.txt is created

At this time, the golang.org/s/release process arranges such that the
api/go1.n.txt file is created when a Go 1.N Beta 1 release is being cut.
The API check is currently configured so that tests don't fail visibly
even if api/go1.n.txt becomes a subset of the actual API additions in
the upcoming Go 1.N release as long as 'go version' has "devel" in it.
The first time that 'go version' output drops the "devel" substring
during testing is after the release-branch.go1.N branch is created
as part of the process to cut a Go 1.N Release Candidate 1 release.

The month or so between Beta 1 and RC 1 is well into the freeze and
deliberate API changes are rare and very intentional. There seems to
be agreement that it's healthy to make the API check stricter during
that time period. Doing so will ensure that api/go1.n.txt cannot get
stale after creation without anyone noticing, and may catch CLs that
don't have the intended diff on the API.

This CL changes behavior to be simple and clear: from the moment
an api/go1.n.txt file corresponding to the current Go version in
development is added to the tree, silent API additions stop being
permitted.

This CL also moves the magical "override the value of -allow_new flag
if runtime.Version() contains 'devel' string" behavior from cmd/api
command to the run.go script that calls it, making the CLI of cmd/api
itself less surprising.

Fixes #43956.

Change-Id: I89468207573f7ccdbc9f12625dcdd3ef2bcf8f10
Reviewed-on: https://go-review.googlesource.com/c/go/+/315350
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Trust: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>

diff --git a/src/cmd/api/goapi.go b/src/cmd/api/goapi.go
index efc2696f8fecc72bfdf6ad27ffc99ec814cdaae0..b07a238d679a1636657fb7618b8c0d7e7ea34a72 100644 (file)
--- a/src/cmd/api/goapi.go
+++ b/src/cmd/api/goapi.go
@@ -215,8 +215,7 @@ func main() {
        }
        optional := fileFeatures(*nextFile)
        exception := fileFeatures(*exceptFile)
-       fail = !compareAPI(bw, features, required, optional, exception,
-               *allowNew && strings.Contains(runtime.Version(), "devel"))
+       fail = !compareAPI(bw, features, required, optional, exception, *allowNew)
 }
 
 // export emits the exported package features.

diff --git a/src/cmd/api/run.go b/src/cmd/api/run.go
index 8c9fb723a583b251027a22aff54ba3894063ee07..81979de191abc2f13263a467fc7e34aab31b8026 100644 (file)
--- a/src/cmd/api/run.go
+++ b/src/cmd/api/run.go
@@ -10,8 +10,11 @@
 package main
 
 import (
+       "errors"
        "fmt"
        exec "internal/execabs"
+       "internal/goversion"
+       "io/fs"
        "log"
        "os"
        "path/filepath"
@@ -43,6 +46,7 @@ func main() {
        apiDir := filepath.Join(goroot, "api")
        out, err := exec.Command(goCmd(), "tool", "api",
                "-c", findAPIDirFiles(apiDir),
+               allowNew(apiDir),
                "-next", filepath.Join(apiDir, "next.txt"),
                "-except", filepath.Join(apiDir, "except.txt")).CombinedOutput()
        if err != nil {
@@ -71,3 +75,35 @@ func findAPIDirFiles(apiDir string) string {
        }
        return strings.Join(apiFiles, ",")
 }
+
+// allowNew returns the -allow_new flag to use for the 'go tool api' invocation.
+func allowNew(apiDir string) string {
+       // Verify that the api/go1.n.txt for previous Go version exists.
+       // It definitely should, otherwise it's a signal that the logic below may be outdated.
+       if _, err := os.Stat(filepath.Join(apiDir, fmt.Sprintf("go1.%d.txt", goversion.Version-1))); err != nil {
+               log.Fatalln("Problem with api file for previous release:", err)
+       }
+
+       // See whether the api/go1.n.txt for this Go version has been created.
+       // (As of April 2021, it gets created during the release of the first Beta.)
+       _, err := os.Stat(filepath.Join(apiDir, fmt.Sprintf("go1.%d.txt", goversion.Version)))
+       if errors.Is(err, fs.ErrNotExist) {
+               // It doesn't exist, so we're in development or before Beta 1.
+               // At this stage, unmentioned API additions are deemed okay.
+               // (They will be quietly shown in API check output, but the test won't fail).
+               return "-allow_new=true"
+       } else if err == nil {
+               // The api/go1.n.txt for this Go version has been created,
+               // so we're definitely past Beta 1 in the release cycle.
+               //
+               // From this point, enforce that api/go1.n.txt is an accurate and complete
+               // representation of what's going into the release by failing API check if
+               // there are API additions (a month into the freeze, there shouldn't be many).
+               //
+               // See golang.org/issue/43956.
+               return "-allow_new=false"
+       } else {
+               log.Fatal(err)
+       }
+       panic("unreachable")
+}