internal: support "all" in BuildContext

Handle BuildContexts where one or more parts are "all".

These should only occur when searching for a matching Documentation,
not sorting. (Because we sort only when there is more than one
Documentation, and if there is an all/all Documentation then there
aren't any others.)

For golang/go#37232

Change-Id: I898aba8f73d2682798d56c47cc6773709f10c702
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/290094
Trust: Jonathan Amsterdam <[email protected]>
Run-TryBot: Jonathan Amsterdam <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Julie Qiu <[email protected]>

Author: jba

internal/build_context.go

@@ -11,6 +11,9 @@ type BuildContext struct {
 	GOOS, GOARCH string
 }
 
+// All represents all values for a build context element (GOOS or GOARCH).
+const All = "all"
+
 // BuildContexts are the build contexts we check when loading a package (see
 // internal/fetch/load.go).
 // We store documentation for all of the listed contexts.
@@ -25,6 +28,14 @@ var BuildContexts = []BuildContext{
 // CompareBuildContexts returns a negative number, 0, or a positive number depending on
 // the relative positions of c1 and c2 in BuildContexts.
 func CompareBuildContexts(c1, c2 BuildContext) int {
+	if c1 == c2 {
+		return 0
+	}
+	// We should never see a BuildContext with "all" here.
+	if c1.GOOS == All || c1.GOARCH == All || c2.GOOS == All || c2.GOARCH == All {
+		panic("BuildContext with 'all'")
+	}
+
 	pos := func(c BuildContext) int {
 		for i, d := range BuildContexts {
 			if c == d {
@@ -44,12 +55,12 @@ func (d *Documentation) BuildContext() BuildContext {
 // DocumentationForBuildContext returns the first Documentation the list that
 // matches the BuildContext, or nil if none does. A Documentation matches if its
 // GOOS and GOARCH fields are the same as those of the BuildContext, or if the
-// BuildContext field is empty. That is, empty BuildContext fields act as
-// wildcards. So the zero BuildContext will match the first element of docs, if
-// there is one.
+// Documentation field is "all", or if the BuildContext field is empty. That is,
+// empty BuildContext fields act as wildcards. So the zero BuildContext will
+// match the first element of docs, if there is one.
 func DocumentationForBuildContext(docs []*Documentation, bc BuildContext) *Documentation {
 	for _, d := range docs {
-		if (bc.GOOS == "" || bc.GOOS == d.GOOS) && (bc.GOARCH == "" || bc.GOARCH == d.GOARCH) {
+		if (bc.GOOS == "" || d.GOOS == All || bc.GOOS == d.GOOS) && (bc.GOARCH == "" || d.GOARCH == All || bc.GOARCH == d.GOARCH) {
 			return d
 		}
 	}

internal/build_context_test.go

@@ -7,21 +7,33 @@ package internal
 import "testing"
 
 func TestCompareBuildContexts(t *testing.T) {
-	for i, c1 := range BuildContexts {
-		if got := CompareBuildContexts(c1, c1); got != 0 {
-			t.Errorf("%v: got %d, want 0", c1, got)
-		}
-		for _, c2 := range BuildContexts[i+1:] {
-			if got := CompareBuildContexts(c1, c2); got >= 0 {
-				t.Errorf("%v, %v: got %d, want < 0", c1, c2, got)
+	check := func(c1, c2 BuildContext, want int) {
+		t.Helper()
+		got := CompareBuildContexts(c1, c2)
+		switch want {
+		case 0:
+			if got != 0 {
+				t.Errorf("%v vs. %v: got %d, want 0", c1, c2, got)
 			}
-			if got := CompareBuildContexts(c2, c1); got <= 0 {
-				t.Errorf("%v, %v: got %d, want > 0", c2, c1, got)
+		case 1:
+			if got <= 0 {
+				t.Errorf("%v vs. %v: got %d, want > 0", c1, c2, got)
+			}
+		case -1:
+			if got >= 0 {
+				t.Errorf("%v vs. %v: got %d, want < 0", c1, c2, got)
 			}
 		}
 	}
-	got := CompareBuildContexts(BuildContext{"?", "?"}, BuildContexts[len(BuildContexts)-1])
-	if got <= 0 {
-		t.Errorf("unknown vs. last: got %d, want > 0", got)
+
+	for i, c1 := range BuildContexts {
+		check(c1, c1, 0)
+		for _, c2 := range BuildContexts[i+1:] {
+			check(c1, c2, -1)
+			check(c2, c1, 1)
+		}
 	}
+
+	// Special cases.
+	check(BuildContext{"?", "?"}, BuildContexts[len(BuildContexts)-1], 1) // unknown is last
 }

internal/fetch/fetchdata_test.go

@@ -54,6 +54,8 @@ var moduleOnePackage = &testModule{
 						Path: "github.com/basic/foo",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "package foo exports a helpful constant.",
 					}},
 					Imports: []string{"net/http"},
@@ -124,6 +126,8 @@ var moduleMultiPackage = &testModule{
 						Contents: "Another README FILE FOR TESTING.",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "package bar",
 					}},
 				},
@@ -133,6 +137,8 @@ var moduleMultiPackage = &testModule{
 						Path: "github.com/my/module/foo",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "package foo",
 					}},
 					Imports: []string{"fmt", "github.com/my/module/bar"},
@@ -178,6 +184,8 @@ var moduleNoGoMod = &testModule{
 						Path: "no.mod/module/p",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "Package p is inside a module where a go.mod file hasn't been explicitly added yet.",
 					}},
 				},
@@ -241,6 +249,8 @@ var moduleBadPackages = &testModule{
 						Path: "bad.mod/module/good",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "Package good is inside a module that has bad packages.",
 					}},
 				},
@@ -397,6 +407,8 @@ var moduleNonRedist = &testModule{
 						Path: "nonredistributable.mod/module/bar",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "package bar",
 					}},
 				},
@@ -406,6 +418,8 @@ var moduleNonRedist = &testModule{
 						Path: "nonredistributable.mod/module/bar/baz",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "package baz",
 					}},
 				},
@@ -419,6 +433,8 @@ var moduleNonRedist = &testModule{
 						Contents: "README FILE SHOW UP HERE BUT WILL BE REMOVED BEFORE DB INSERT",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "package foo",
 					}},
 					Imports: []string{"fmt", "github.com/my/module/bar"},
@@ -468,7 +484,7 @@ var moduleBadImportPath = &testModule{
 						Name: "foo",
 						Path: "bad.import.path.com/good/import/path",
 					},
-					Documentation: []*internal.Documentation{{}},
+					Documentation: []*internal.Documentation{{GOOS: internal.All, GOARCH: internal.All}},
 				},
 			},
 		},
@@ -527,6 +543,8 @@ var moduleDocTest = &testModule{
 						Path: "doc.test/permalink",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "Package permalink is for testing the heading permalink documentation rendering feature.",
 					}},
 				},
@@ -566,6 +584,8 @@ var moduleDocTooLarge = &testModule{
 						Path: "bigdoc.test",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "This documentation is big.",
 					}},
 				},
@@ -674,6 +694,8 @@ var moduleStd = &testModule{
 						Path: "builtin",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "Package builtin provides documentation for Go's predeclared identifiers.",
 					}},
 				},
@@ -720,6 +742,8 @@ var moduleStd = &testModule{
 						Path: "context",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "Package context defines the Context type, which carries deadlines, cancelation signals, and other request-scoped values across API boundaries and between processes.",
 					}},
 					Imports: []string{"errors", "fmt", "reflect", "sync", "time"},
@@ -735,6 +759,8 @@ var moduleStd = &testModule{
 						Path: "encoding/json",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "Package json implements encoding and decoding of JSON as defined in RFC 7159.",
 					}},
 					Imports: []string{
@@ -761,6 +787,8 @@ var moduleStd = &testModule{
 						Path: "errors",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "Package errors implements functions to manipulate errors.",
 					}},
 				},
@@ -771,6 +799,8 @@ var moduleStd = &testModule{
 					},
 					Imports: []string{"errors", "fmt", "io", "os", "reflect", "sort", "strconv", "strings", "time"},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "Package flag implements command-line flag parsing.",
 					}},
 				},
@@ -807,6 +837,8 @@ var moduleMaster = &testModule{
 						Path: "github.com/my/module/foo",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "package foo exports a helpful constant.",
 					}},
 				},
@@ -843,6 +875,8 @@ var moduleLatest = &testModule{
 						Path: "github.com/my/module/foo",
 					},
 					Documentation: []*internal.Documentation{{
+						GOOS:     internal.All,
+						GOARCH:   internal.All,
 						Synopsis: "package foo exports a helpful constant.",
 					}},
 				},
@@ -892,6 +926,8 @@ package example_test
 							Path: path + "/example",
 						},
 						Documentation: []*internal.Documentation{{
+							GOOS:     internal.All,
+							GOARCH:   internal.All,
 							Synopsis: "Package example contains examples.",
 						}},
 					},

internal/fetch/load.go

@@ -112,8 +112,8 @@ func loadPackage(ctx context.Context, zipGoFiles []*zip.File, innerPath string,
 				name:    name,
 				imports: imports,
 				docs: []*internal.Documentation{{
-					GOOS:     bc.GOOS,
-					GOARCH:   bc.GOARCH,
+					GOOS:     internal.All,
+					GOARCH:   internal.All,
 					Synopsis: synopsis,
 					Source:   source,
 				}},
@@ -149,6 +149,14 @@ func loadPackage(ctx context.Context, zipGoFiles []*zip.File, innerPath string,
 			pkg.docs = append(pkg.docs, doc)
 		}
 	}
+	// If all the build contexts succeeded and had the same set of files, then
+	// assume that the package doc is valid for all build contexts. Represent
+	// this with a single Documentation whose GOOS and GOARCH are both "all".
+	if len(docsByFiles) == 1 && len(pkg.docs) == len(internal.BuildContexts) {
+		pkg.docs = pkg.docs[:1]
+		pkg.docs[0].GOOS = internal.All
+		pkg.docs[0].GOARCH = internal.All
+	}
 	return pkg, nil
 }
 

internal/postgres/unit_test.go

@@ -449,13 +449,21 @@ func TestGetUnit(t *testing.T) {
 	// Add a module that has documentation for two Go build contexts.
 	m = sample.Module("a.com/twodoc", "v1.2.3", "p")
 	pkg := m.Packages()[0]
-	doc2 := &internal.Documentation{
-		GOOS:     "windows",
-		GOARCH:   "amd64",
-		Synopsis: pkg.Documentation[0].Synopsis + " 2",
-		Source:   pkg.Documentation[0].Source,
+	docs2 := []*internal.Documentation{
+		{
+			GOOS:     "linux",
+			GOARCH:   "amd64",
+			Synopsis: sample.Synopsis + " for linux",
+			Source:   sample.Documentation.Source,
+		},
+		{
+			GOOS:     "windows",
+			GOARCH:   "amd64",
+			Synopsis: sample.Synopsis + " for windows",
+			Source:   sample.Documentation.Source,
+		},
 	}
-	pkg.Documentation = append(pkg.Documentation, doc2)
+	pkg.Documentation = docs2
 	if err := testDB.InsertModule(ctx, m); err != nil {
 		t.Fatal(err)
 	}
@@ -578,7 +586,8 @@ func TestGetUnit(t *testing.T) {
 				u := unit("a.com/twodoc/p", "a.com/twodoc", "v1.2.3", "p",
 					nil,
 					[]string{"p"})
-				u.Documentation = append(u.Documentation, doc2)
+				u.Documentation = docs2
+				u.Subdirectories[0].Synopsis = docs2[0].Synopsis
 				return u
 			}(),
 		},

internal/testing/sample/sample.go

@@ -62,8 +62,8 @@ var (
 	Synopsis       = "This is a package synopsis"
 	ReadmeFilePath = "README.md"
 	ReadmeContents = "readme"
-	GOOS           = "linux"
-	GOARCH         = "amd64"
+	GOOS           = internal.All
+	GOARCH         = internal.All
 )
 
 // LicenseCmpOpts are options to use when comparing licenses with the cmp package.