doc: don't redirect vanity packages to GitHub
This change fixes a regression from CL 120059. We started to redirect
to GitHub's reported canonical case when the request path didn't
match too aggressively. It should only be done if the request path is
already on GitHub; otherwise we're redirecting a vanity import path to
its GitHub hosting location whenever it doesn't have an import path
comment.
Add tests for this newPackage redirection behavior to increase
confidence in the fix, and help avoid future regressions.
Fix typos in gosrc package comments.
Fixes golang/gddo#579.
Change-Id: I7ed0787a125d2f39ca979d01983e971729e181fd
Reviewed-on: https://go-review.googlesource.com/134355
Reviewed-by: Tuo Shan <shantuo@google.com>
diff --git a/doc/builder.go b/doc/builder.go
index 75ec863..e372d3d 100644
--- a/doc/builder.go
+++ b/doc/builder.go
@@ -579,16 +579,20 @@
switch {
case bpkg.ImportComment != "":
- // Redirect to import path comment.
+ // Redirect to import path comment, if not already there.
if dir.ImportPath != bpkg.ImportComment {
return nil, gosrc.NotFoundError{
Message: "not at canonical import path",
Redirect: bpkg.ImportComment,
}
}
- case bpkg.ImportComment == "" && dir.ResolvedGitHubPath != "":
- // Make sure to redirect to GitHub's reported canonical casing when
- // there's no import path comment.
+
+ // Redirect to GitHub's reported canonical casing when there's no import path comment,
+ // and the import path differs from resolved GitHub path only in case.
+ case bpkg.ImportComment == "" && dir.ResolvedGitHubPath != "" &&
+ strings.EqualFold(dir.ImportPath, dir.ResolvedGitHubPath):
+
+ // Redirect to resolved GitHub path, if not already there.
if dir.ImportPath != dir.ResolvedGitHubPath {
return nil, gosrc.NotFoundError{
Message: "not at canonical import path",
diff --git a/doc/builder_test.go b/doc/builder_test.go
index 7160cda..763f9e4 100644
--- a/doc/builder_test.go
+++ b/doc/builder_test.go
@@ -9,6 +9,8 @@
import (
"go/ast"
"testing"
+
+ "github.com/golang/gddo/gosrc"
)
var badSynopsis = []string{
@@ -112,3 +114,108 @@
}
}
}
+
+// TestNewPackageRedirect tests that newPackage redirects
+// and does not redirect as expected, in various situations.
+// See https://github.com/golang/gddo/issues/507
+// and https://github.com/golang/gddo/issues/579.
+func TestNewPackageRedirect(t *testing.T) {
+ // robpike.io/ivy package.
+ // Vanity import path, hosted on GitHub, with import path comment.
+ ivy := gosrc.Directory{
+ Files: []*gosrc.File{
+ {Name: "main.go", Data: []byte("package main // import \"robpike.io/ivy\"\n")},
+ },
+ ResolvedGitHubPath: "github.com/robpike/ivy",
+ }
+
+ // go4.org/sort package.
+ // Vanity import path, hosted on GitHub, without import path comment.
+ go4sort := gosrc.Directory{
+ Files: []*gosrc.File{
+ {Name: "main.go", Data: []byte("package sort\n")},
+ },
+ ResolvedGitHubPath: "github.com/go4org/go4/sort",
+ }
+
+ // github.com/teamwork/validate package.
+ // Hosted on GitHub, with import path comment that doesn't match canonical GitHub case.
+ // See issue https://github.com/golang/gddo/issues/507.
+ gtv := gosrc.Directory{
+ Files: []*gosrc.File{
+ {Name: "main.go", Data: []byte("package validate // import \"github.com/teamwork/validate\"\n")},
+ },
+ ResolvedGitHubPath: "github.com/Teamwork/validate", // Note that this differs from import path comment.
+ }
+
+ tests := []struct {
+ name string
+ repo gosrc.Directory
+ requestPath string
+ wantRedirect string // Empty string means no redirect.
+ }{
+ // ivy.
+ {
+ repo: ivy, name: "ivy repo: access canonical path -> no redirect",
+ requestPath: "robpike.io/ivy",
+ },
+ {
+ repo: ivy, name: "ivy repo: access GitHub path -> redirect to import comment",
+ requestPath: "github.com/robpike/ivy",
+ wantRedirect: "robpike.io/ivy",
+ },
+ {
+ repo: ivy, name: "ivy repo: access GitHub path with weird casing -> redirect to import comment",
+ requestPath: "github.com/RoBpIkE/iVy",
+ wantRedirect: "robpike.io/ivy",
+ },
+
+ // go4sort.
+ {
+ repo: go4sort, name: "go4sort repo: access canonical path -> no redirect",
+ requestPath: "go4.org/sort",
+ },
+ {
+ repo: go4sort, name: "go4sort repo: access GitHub path -> no redirect",
+ requestPath: "github.com/go4org/go4/sort",
+ },
+ {
+ repo: go4sort, name: "go4sort repo: access GitHub path with weird casing -> redirect to resolved GitHub case",
+ requestPath: "github.com/gO4oRg/Go4/sort",
+ wantRedirect: "github.com/go4org/go4/sort",
+ },
+
+ // gtv.
+ {
+ repo: gtv, name: "gtv repo: access canonical path -> no redirect",
+ requestPath: "github.com/teamwork/validate",
+ },
+ {
+ repo: gtv, name: "gtv repo: access canonical GitHub path -> redirect to import comment",
+ requestPath: "github.com/Teamwork/validate",
+ wantRedirect: "github.com/teamwork/validate",
+ },
+ {
+ repo: gtv, name: "gtv repo: access GitHub path with weird casing -> redirect to import comment",
+ requestPath: "github.com/tEaMwOrK/VaLiDaTe",
+ wantRedirect: "github.com/teamwork/validate",
+ },
+ }
+ for _, tt := range tests {
+ dir := tt.repo
+ dir.ImportPath = tt.requestPath
+
+ var want error
+ if tt.wantRedirect != "" {
+ want = gosrc.NotFoundError{
+ Message: "not at canonical import path",
+ Redirect: tt.wantRedirect,
+ }
+ }
+
+ _, got := newPackage(&dir)
+ if got != want {
+ t.Errorf("%s: got error %v, want %v", tt.name, got, want)
+ }
+ }
+}
diff --git a/gosrc/gosrc.go b/gosrc/gosrc.go
index c4f9f4c..776bd45 100644
--- a/gosrc/gosrc.go
+++ b/gosrc/gosrc.go
@@ -455,7 +455,7 @@
return dir, nil
}
-// getStatic gets a diretory from a statically known service. getStatic
+// getStatic gets a directory from a statically known service. getStatic
// returns errNoMatch if the import path is not recognized.
func getStatic(ctx context.Context, client *http.Client, importPath, etag string) (*Directory, error) {
for _, s := range services {
diff --git a/gosrc/gosrc_test.go b/gosrc/gosrc_test.go
index d90e798..723c73c 100644
--- a/gosrc/gosrc_test.go
+++ b/gosrc/gosrc_test.go
@@ -22,7 +22,7 @@
var testWeb = map[string]string{
// Package at root of a GitHub repo.
"https://alice.org/pkg": `<head> <meta name="go-import" content="alice.org/pkg git https://github.com/alice/pkg"></head>`,
- // Package in sub-diretory.
+ // Package in sub-directory.
"https://alice.org/pkg/sub": `<head> <meta name="go-import" content="alice.org/pkg git https://github.com/alice/pkg"><body>`,
// Fallback to http.
"http://alice.org/pkg/http": `<head> <meta name="go-import" content="alice.org/pkg git https://github.com/alice/pkg">`,
