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 <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
diff --git a/internal/build_context.go b/internal/build_context.go
index 76ae015..9e7db80 100644
--- a/internal/build_context.go
+++ b/internal/build_context.go
@@ -11,6 +11,9 @@
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 @@
// 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 @@
// 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
}
}
diff --git a/internal/build_context_test.go b/internal/build_context_test.go
index c4e1fc1..1c05577 100644
--- a/internal/build_context_test.go
+++ b/internal/build_context_test.go
@@ -7,21 +7,33 @@
import "testing"
func TestCompareBuildContexts(t *testing.T) {
+ 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)
+ }
+ 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)
+ }
+ }
+ }
+
for i, c1 := range BuildContexts {
- if got := CompareBuildContexts(c1, c1); got != 0 {
- t.Errorf("%v: got %d, want 0", c1, got)
- }
+ check(c1, c1, 0)
for _, c2 := range BuildContexts[i+1:] {
- if got := CompareBuildContexts(c1, c2); got >= 0 {
- t.Errorf("%v, %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)
- }
+ check(c1, c2, -1)
+ check(c2, c1, 1)
}
}
- got := CompareBuildContexts(BuildContext{"?", "?"}, BuildContexts[len(BuildContexts)-1])
- if got <= 0 {
- t.Errorf("unknown vs. last: got %d, want > 0", got)
- }
+
+ // Special cases.
+ check(BuildContext{"?", "?"}, BuildContexts[len(BuildContexts)-1], 1) // unknown is last
}
diff --git a/internal/fetch/fetchdata_test.go b/internal/fetch/fetchdata_test.go
index dc7804a..e36e9f7 100644
--- a/internal/fetch/fetchdata_test.go
+++ b/internal/fetch/fetchdata_test.go
@@ -54,6 +54,8 @@
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 @@
Contents: "Another README FILE FOR TESTING.",
},
Documentation: []*internal.Documentation{{
+ GOOS: internal.All,
+ GOARCH: internal.All,
Synopsis: "package bar",
}},
},
@@ -133,6 +137,8 @@
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 @@
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 @@
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 @@
Path: "nonredistributable.mod/module/bar",
},
Documentation: []*internal.Documentation{{
+ GOOS: internal.All,
+ GOARCH: internal.All,
Synopsis: "package bar",
}},
},
@@ -406,6 +418,8 @@
Path: "nonredistributable.mod/module/bar/baz",
},
Documentation: []*internal.Documentation{{
+ GOOS: internal.All,
+ GOARCH: internal.All,
Synopsis: "package baz",
}},
},
@@ -419,6 +433,8 @@
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 @@
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 @@
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 @@
Path: "bigdoc.test",
},
Documentation: []*internal.Documentation{{
+ GOOS: internal.All,
+ GOARCH: internal.All,
Synopsis: "This documentation is big.",
}},
},
@@ -674,6 +694,8 @@
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 @@
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 @@
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 @@
Path: "errors",
},
Documentation: []*internal.Documentation{{
+ GOOS: internal.All,
+ GOARCH: internal.All,
Synopsis: "Package errors implements functions to manipulate errors.",
}},
},
@@ -771,6 +799,8 @@
},
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 @@
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 @@
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 @@
Path: path + "/example",
},
Documentation: []*internal.Documentation{{
+ GOOS: internal.All,
+ GOARCH: internal.All,
Synopsis: "Package example contains examples.",
}},
},
diff --git a/internal/fetch/load.go b/internal/fetch/load.go
index bc13b91..f5b149f 100644
--- a/internal/fetch/load.go
+++ b/internal/fetch/load.go
@@ -112,8 +112,8 @@
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 @@
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
}
diff --git a/internal/postgres/unit_test.go b/internal/postgres/unit_test.go
index a3daca9..ea53d45 100644
--- a/internal/postgres/unit_test.go
+++ b/internal/postgres/unit_test.go
@@ -449,13 +449,21 @@
// 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 @@
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
}(),
},
diff --git a/internal/testing/sample/sample.go b/internal/testing/sample/sample.go
index 17b2f24..f284aa9 100644
--- a/internal/testing/sample/sample.go
+++ b/internal/testing/sample/sample.go
@@ -62,8 +62,8 @@
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.
