godoc.org/github.com/docker/docker/* reports 404

[dnephin]

The godocs for docker/docker/ (all subpackages) are returning 404 (https://github.com/moby/moby/issues/36252).

For context, github.com/docker/docker redirects to github.com/moby/moby. We recently added a canonical import path to the go packages in this repo with // import "github.com/docker/docker" since all the internal imports still use this path. We believe this is what broke godoc.

I started a gddo-server locally to debug the issue, and I noticed it reported

notfound: Github import path has incorrect case. github.com/docker/docker/client

However I was using the correct case (all lowercase).

I believe the issue is this line: https://github.com/golang/gddo/blob/9b12a26f3fbd7397dee4e20939ddca719d840d2a/gosrc/github.go#L131

By changing it to use strings.EqualFold instead of != the issue is resolved.

--- a/gosrc/github.go
+++ b/gosrc/github.go
@@ -128,7 +128,7 @@ func getGitHubDir(ctx context.Context, client *http.Client, match map[string]str

    // GitHub owner and repo names are case-insensitive. Redirect if requested
    // names do not match the canonical names in API response.
-   if m := ownerRepoPat.FindStringSubmatch(contents[0].GitURL); m != nil && (m[1] != match["owner"] || m[2] != match["repo"]) {
+   if m := ownerRepoPat.FindStringSubmatch(contents[0].GitURL); m != nil && incorrectCase(match, m) {
        match["owner"] = m[1]
        match["repo"] = m[2]
        return nil, NotFoundError{
@@ -180,6 +180,13 @@ func getGitHubDir(ctx context.Context, client *http.Client, match map[string]str
    }, nil
 }

+func incorrectCase(expected map[string]string, actual []string) bool {
+   if actual[1] == expected["owner"] && actual[2] == expected["repo"] {
+       return false
+   }
+   return strings.EqualFold(actual[1], expected["owner"]) || strings.EqualFold(actual[2], expected["repo"])
+}
+
 // isQuickFork reports whether the repository is a "quick fork":
 // it has fewer than 3 commits, all within a week of the repo creation, createdAt.
 // Commits must be in reverse chronological order by Commit.Committer.Date.

If this sounds like the correct fix I can submit a patch.

With this match the godoc for moby/moby correctly redirects to the canonical import path as well (docker/docker)

[dmitshur]

For context, github.com/docker/docker redirects to github.com/moby/moby. We recently added a canonical import path to the go packages in this repo with // import "github.com/docker/docker" since all the internal imports still use this path.

That's a tricky situation. But, I don't see a reason it shouldn't be supported.

The general approach of the proposed fix sounds valid to me. That code was meant to check only if the case was mismatched, but it was actually checking if the strings differed in any way.

Although I don't quite understand why it's serving package not found rather than actually redirecting somewhere. Do you know why? Edit: Never mind, it does redirect from https://godoc.org/github.com/docker/docker/client to https://godoc.org/github.com/moby/moby/client. I missed that somehow earlier.

(I hope the final code of incorrectCase can be made a little more clear; those arguments are hard to track now.)

[dmitshur]

Also, this issue intersects with #507 in some way. Please see that your fix doesn't make the situation there worse. (I think it should be orthogonal, but better to check anyway.)

Edit: Be mindful of https://github.com/Teamwork/test/commit/c3703d09f4f6ee38d5aa06054007c149bd050937, that repo might no longer be representative of the problem.

[dnephin]

That's a tricky situation.

Yes it is...

Although I don't quite understand why it's serving package not found rather than actually redirecting somewhere.

I believe that line causes a redirectly loop, that is eventually terminated with a not found here: https://github.com/golang/gddo/blob/master/gddo-server/main.go#L239-L243

My understanding is that it will fetch moby/moby, see the canonical import path docker/docker, so re-fetch docker/docker, fail because of that incorrect case check, and the loop continues. I haven't traced that entire code path yet. but there were a bunch of these in the logs:

2018/02/09 15:52:00 web   fetch: 1728 notfound: not at canonical import path github.com/moby/moby/api/types
2018/02/09 15:52:01 web   fetch: 998 notfound: Github import path has incorrect case. github.com/docker/docker/api/types
2018/02/09 15:52:07 web   fetch: 503 notfound: Github import path has incorrect case. github.com/docker/docker/api/types
2018/02/09 15:52:07 web   fetch: 596 notfound: not at canonical import path github.com/moby/moby/api/types
2018/02/09 15:52:13 web   fetch: 577 notfound: Github import path has incorrect case. github.com/docker/docker/api/types
2018/02/09 15:52:14 web   fetch: 681 notfound: not at canonical import path github.com/moby/moby/api/types

I hope the final code of incorrectCase can be made a little more clear;

Absolutely, that was just a quick patch, it needs cleanup.

[dnephin]

https://go-review.googlesource.com/c/gddo/+/93176

[dnephin]

Thanks for merging my patch for this issue!

Is there a regular deployment schedule for godoc.org? Or how is a new deploy trigger?

[shantuo]

Thank you for the fix! Just did a new deployment, it works for both moby and docker paths. It might take some time for the crawler to update these doc pages.

[dnephin]

Thanks!