]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/relnote: fix API relnote check
authorJonathan Amsterdam <jba@google.com>
Thu, 1 Feb 2024 21:00:51 +0000 (16:00 -0500)
committerJonathan Amsterdam <jba@google.com>
Mon, 5 Feb 2024 16:31:53 +0000 (16:31 +0000)
Fix the check for release note files that correspond to API files
to look in the right directory, doc/next/*stdlib/*minor. Previously
the test looked in doc/next.

Improve the error messages when the test fails to explain the problem
better and refer to further documentation.

(These changes are actually in the x/build repo; this CL vendors
the latest version.)

Lastly, re-enable the check.

For #64169.

Change-Id: I8bba845e9bd12afbe269ce42d6d4b17b1e3c0252
Reviewed-on: https://go-review.googlesource.com/c/go/+/560516
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/cmd/go.mod
src/cmd/go.sum
src/cmd/relnote/relnote_test.go
src/cmd/vendor/golang.org/x/build/relnote/relnote.go
src/cmd/vendor/modules.txt

index 41194f39d935ef97a9b976d931b815a79b7a2ec7..7b548c79bd180473bd83d7269517050b4d5f9ab4 100644 (file)
@@ -5,7 +5,7 @@ go 1.23
 require (
        github.com/google/pprof v0.0.0-20230811205829-9131a7e9cc17
        golang.org/x/arch v0.7.0
-       golang.org/x/build v0.0.0-20240122184708-c291ad69d6be
+       golang.org/x/build v0.0.0-20240201175143-3ee44a092755
        golang.org/x/mod v0.14.0
        golang.org/x/sync v0.6.0
        golang.org/x/sys v0.16.1-0.20240110015235-f69d32aa924f
index 86dd83bd8ae98b566ff7f2f25d0f552655fb7509..572492d22fc9110b0aa38d0562867885209e97f5 100644 (file)
@@ -8,8 +8,8 @@ github.com/yuin/goldmark v1.6.0 h1:boZcn2GTjpsynOsC0iJHnBWa4Bi0qzfJjthwauItG68=
 github.com/yuin/goldmark v1.6.0/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
 golang.org/x/arch v0.7.0 h1:pskyeJh/3AmoQ8CPE95vxHLqp1G1GfGNXTmcl9NEKTc=
 golang.org/x/arch v0.7.0/go.mod h1:FEVrYAQjsQXMVJ1nsMoVVXPZg6p2JE2mx8psSWTDQys=
-golang.org/x/build v0.0.0-20240122184708-c291ad69d6be h1:h1qJlb1MudWuUMYotaFX+nSdSgv6zrBBDNojV68uqCA=
-golang.org/x/build v0.0.0-20240122184708-c291ad69d6be/go.mod h1:RHSzqFUzT4+buJlGik6WptO5NxLQiR/ewD2uz3fgWuA=
+golang.org/x/build v0.0.0-20240201175143-3ee44a092755 h1:irSM9p93GT4I3+Pu/grZlkwIjrXA3GfyKwlSosVbmtU=
+golang.org/x/build v0.0.0-20240201175143-3ee44a092755/go.mod h1:RHSzqFUzT4+buJlGik6WptO5NxLQiR/ewD2uz3fgWuA=
 golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0=
 golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
 golang.org/x/sync v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ=
index 74b785923a655afc01b27a30df9698a563153da9..c20f80efc4857aa325423e45cd539a66b651e1c3 100644 (file)
@@ -19,7 +19,6 @@ var flagCheck = flag.Bool("check", false, "run API release note checks")
 
 // Check that each file in api/next has corresponding release note files in doc/next.
 func TestCheckAPIFragments(t *testing.T) {
-       t.Skip("impossibly confusing error messages")
        if !*flagCheck {
                t.Skip("-check not specified")
        }
@@ -33,7 +32,7 @@ func TestCheckAPIFragments(t *testing.T) {
        docFS := os.DirFS(filepath.Join(root, "doc", "next"))
        // Check that each api/next file has a corresponding release note fragment.
        for _, apiFile := range files {
-               if err := relnote.CheckAPIFile(rootFS, apiFile, docFS); err != nil {
+               if err := relnote.CheckAPIFile(rootFS, apiFile, docFS, "doc/next"); err != nil {
                        t.Errorf("%s: %v", apiFile, err)
                }
        }
index 63791a703660245bff33e46671ff3e8292d778e6..5ac4d7a843e013a851adade14f6cb9a0ccf6cb5d 100644 (file)
@@ -37,13 +37,13 @@ func NewParser() *md.Parser {
 // CheckFragment reports problems in a release-note fragment.
 func CheckFragment(data string) error {
        doc := NewParser().Parse(data)
-       if len(doc.Blocks) == 0 {
-               return errors.New("empty content")
-       }
        // Check that the content of the document contains either a TODO or at least one sentence.
-       txt := text(doc)
+       txt := ""
+       if len(doc.Blocks) > 0 {
+               txt = text(doc)
+       }
        if !strings.Contains(txt, "TODO") && !strings.ContainsAny(txt, ".?!") {
-               return errors.New("needs a TODO or a sentence")
+               return errors.New("File must contain a complete sentence or a TODO.")
        }
        return nil
 }
@@ -382,7 +382,9 @@ func GroupAPIFeaturesByFile(fs []APIFeature) (map[string][]APIFeature, error) {
 // CheckAPIFile reads the api file at filename in apiFS, and checks the corresponding
 // release-note files under docFS. It checks that the files exist and that they have
 // some minimal content (see [CheckFragment]).
-func CheckAPIFile(apiFS fs.FS, filename string, docFS fs.FS) error {
+// The docRoot argument is the path from the repo or project root to the root of docFS.
+// It is used only for error messages.
+func CheckAPIFile(apiFS fs.FS, filename string, docFS fs.FS, docRoot string) error {
        features, err := parseAPIFile(apiFS, filename)
        if err != nil {
                return err
@@ -396,21 +398,47 @@ func CheckAPIFile(apiFS fs.FS, filename string, docFS fs.FS) error {
                filenames = append(filenames, fn)
        }
        slices.Sort(filenames)
+       mcDir, err := minorChangesDir(docFS)
+       if err != nil {
+               return err
+       }
        var errs []error
-       for _, filename := range filenames {
+       for _, fn := range filenames {
+               // Use path.Join for consistency with io/fs pathnames.
+               fn = path.Join(mcDir, fn)
                // TODO(jba): check that the file mentions each feature?
-               if err := checkFragmentFile(docFS, filename); err != nil {
-                       errs = append(errs, fmt.Errorf("%s: %v", filename, err))
+               if err := checkFragmentFile(docFS, fn); err != nil {
+                       errs = append(errs, fmt.Errorf("%s: %v\nSee doc/README.md for more information.", path.Join(docRoot, fn), err))
                }
        }
        return errors.Join(errs...)
 }
 
+// minorChangesDir returns the unique directory in docFS that corresponds to the
+// "Minor changes to the standard library" section of the release notes.
+func minorChangesDir(docFS fs.FS) (string, error) {
+       dirs, err := fs.Glob(docFS, "*stdlib/*minor")
+       if err != nil {
+               return "", err
+       }
+       var bad string
+       if len(dirs) == 0 {
+               bad = "No"
+       } else if len(dirs) > 1 {
+               bad = "More than one"
+       }
+       if bad != "" {
+               return "", fmt.Errorf("%s directory matches *stdlib/*minor.\nThis shouldn't happen; please file a bug at https://go.dev/issues/new.",
+                       bad)
+       }
+       return dirs[0], nil
+}
+
 func checkFragmentFile(fsys fs.FS, filename string) error {
        f, err := fsys.Open(filename)
        if err != nil {
                if errors.Is(err, fs.ErrNotExist) {
-                       err = fs.ErrNotExist
+                       err = errors.New("File does not exist. Every API change must have a corresponding release note file.")
                }
                return err
        }
index f5ffc67a026d6da1bbc4d7f816b13039ba419a92..abafaf30ba5fb7aaa858b3daf429b8835bdf4038 100644 (file)
@@ -23,7 +23,7 @@ golang.org/x/arch/arm/armasm
 golang.org/x/arch/arm64/arm64asm
 golang.org/x/arch/ppc64/ppc64asm
 golang.org/x/arch/x86/x86asm
-# golang.org/x/build v0.0.0-20240122184708-c291ad69d6be
+# golang.org/x/build v0.0.0-20240201175143-3ee44a092755
 ## explicit; go 1.21
 golang.org/x/build/relnote
 # golang.org/x/mod v0.14.0