Packages which normalize casing in import path comment don't show #507
Comments
|
I got a local development environment running, and I have this error in the logs: Which is due to this check: https://github.com/golang/gddo/blob/master/gosrc/github.go#L130 Commenting that check out fixes it. I'm not sure what the best way to fix it is, as this is before any Go code is parsed. |
|
This is a valid issue that we need to find a good solution to. I simply want to ask this question first. Is it a viable option for you to rename your GitHub organization from |
This is good question, but tricky because there is a lot tied in to our GitHub account (not just Go, also other languages/environments). Predicting what will break is not so easy. Even worse, it's not easy to test if everything works, and failures related to the rename may not be noticed for days, weeks, or even months, and many of these errors would likely be confusing. Another alternative would be to use It's do-able, but would be fairly time-consuming/costly across the entire company. It would be more cost-effective if I spend time fixing this bug :-) |
|
I understand, thank you for providing that information. I agree, although it's a bit unfortunate, I don't think what you're doing is breaking any rules, and as such, godoc.org should be able to display documentation with the current setup. The reason some of those checks exist is because we don't want a normal
That would cause an huge artificial increase in the number of packages available. At the same time, a non-github import path such as |
|
I'm not entirely sure how to fix this; to get the canonical import path from the comment we need to clone the code and parse it; but to clone the code we first want to normalize the path. It's a bit of a catch-22 :-/ Any suggestions would be appreciated. |
Our GitHub organisation is named Teamwork, but for various hard to change reasons we import packages as teamwork (lower case t). To enforce this we add an import enforcement for our packages. This works well, but godoc.org enforces users to use the canonical GitHub path (Teamwork), but with the import statement the Go compiler enforces a different path, leading to an error. Fixing this is a bit of a catch-22: to get the canonical import path from the Go comment need to clone the code and parse it; but to clone the code we first want to normalize the path. As a fix, this patch will allow package authors to specify the canonical import path in the repositories README file as: <!-- import "github.com/canonical/path" --> I have added this to github.com/teamwork/validate as a test: Teamwork/validate@4b8ee77 It seems to me that using the README for this is the easiest way to achieve this;. Another way to fix this might be to use a different file with metadata for gddo (i.e. .gddo.yaml). I don't know if that would make sense. Fixes golang#507
Our GitHub organisation is named Teamwork, but for various hard to change reasons we import packages as teamwork (lower case t). To enforce this we add an import enforcement for our packages. This works well, but godoc.org enforces users to use the canonical GitHub path (Teamwork), but with the import statement the Go compiler enforces a different path, leading to an error. Fixing this is a bit of a catch-22: to get the canonical import path from the Go comment need to clone the code and parse it; but to clone the code we first want to normalize the path. As a fix, this patch will allow package authors to specify the canonical import path in the repositories README file as: <!-- import "github.com/canonical/path" --> I have added this to github.com/teamwork/validate as a test: Teamwork/validate@4b8ee77 It seems to me that using the README for this is the easiest way to achieve this;. Another way to fix this might be to use a different file with metadata for gddo (i.e. .gddo.yaml). I don't know if that would make sense. Fixes golang#507
|
I have an idea for how to deal with the catch-22. If my memory of how gddo works is right, I recall there's github.com-specific logic that first checks if the To solve this, we can consider delaying the redirect when the casing doesn't match, and instead fetch the package as normal and see if it happens to have a matching import path comment. If yes, display the documentation. If not, then proceed to redirect to canonical casing, as done currently. Does that make sense? It seems that it would resolve this bug, but not cause the alternative-casing repos to show up unnecessarily. |
|
Thanks for your feedback @shurcooL! Simply removing this check would implement your suggestion, I think. Since redirection to the canonical import path is already done in I had assumed that this check was added to save disk space, network, or some such, but if that's not an issue, then it's an easy fix. |
|
Oh wait, that's only in cases where an import path is specified. Perhaps adding a |
|
I changed the PR. Let me know what you think! |
Problem: Our GitHub organisation is named Teamwork, but for various hard to change reasons we import packages as teamwork (lower case t). To enforce this we add an import enforcement for our packages: package duck // import "github.com/teamwork/duck" This works well, but godoc.org enforces users to use the canonical GitHub path (Teamwork), but with the import statement the Go compiler enforces a different path, leading to an error. Fix: Instead of immediately redirecting in the gosrc package, it will now only store the canonical import path there. This is before the Go source code is actually scanned, so we don't *actually* know what the real canonical path should be. In the doc.newPackage() function it will check for the canonical import path, taking both the Go import path as well as the reported path from the gosrc package in to account, and redirect as needed. This seems to work for all the cases I can think of: github.com/Teamwork/validate -> github.com/teamwork/validate github.com/pkg/ERRORS -> github.com/pkg/errors github.com/Carpetsmoker/sconfig -> arp242.net/sconfig arp242.net/SCONFIG -> not found (expected) bitbucket.org/pkg/inflect -> works bitbucket.org/pkg/INFLECT -> works (should probably redirect too) github.com/docker/docker/client -> works (golang#534) github.com/moby/moby/client -> redirects (on master this actually seems to error out?) It should also be easy to add a similar check for for some other repo providers, such as BitBucket, GitLab, etc. Fixes golang#507
|
We've merged the fix (PR #560) and deployed the new version. This is fixed now, and the following page works: |
Our organisation name is
Teamwork, and our packages aregithub.com/Teamwork/<name>.For historical reasons, we've always used lowercase import paths:
github.com/teamwork/<name>; I recently added some import comments to enforce this to prevent confusion down the line (to prevent e.g the recent logrus confusion):It seems that this confuses godoc.org. Any packages with such an import don't have working documentation pages.
For example for the github.com/teamwork/test example I fixed it with:
Teamwork/test@c3703d0
A still-broken example is
https://godoc.org/github.com/Teamwork/mailaddress, which has the import path enforcement, but doesn't show as either https://godoc.org/github.com/Teamwork/mailaddress or https://godoc.org/github.com/teamwork/mailaddressThe text was updated successfully, but these errors were encountered: