-
Notifications
You must be signed in to change notification settings - Fork 17.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
x/pkgsite: linking to module source code #40477
Comments
Please can you provide some details on why? Because in implementing #39559 others (i.e. people/tools/whatever other than pkgsite) can benefit from the surfaced information. The two don't appear to be mutually exclusive either; i.e. we can pursue this approach in the short term with #39559 for the longer term? |
I agree with Paul; if I host a personal source code hosting site, I shouldn't need to add it to a global list of well-known code hosting sites. The HTTP meta tags should be plenty for the URLs to describe what the setup looks like. |
I think there is a benefit to using this approach as a next step to support linking to module source code. Iterating on this approach in an internal package can help inform the design of an HTTP meta tag, if that were to be pursued in the future. If something about the internal API turns out to work poorly, or an opportunity to improve or simplify appears, code in the internal package can be refactored atomically. Once the internal package stabilizes, someone can decide it'd be worth creating an equivalent HTTP meta tag design that websites can choose to implement instead, and the existing entries in the internal package can serve as test cases. |
Right, agreed that keeping a list is good in the short term. Even if a meta tag was accepted and announced today, it could take months or years for it to see any widespread use. What I think we were aiming for is to have a tracking issue for what the long-term solution to the problem would be, so that anyone running their own git server can follow that issue and trust that it won't be closed until there's a proper solution. I think we can all agree that a very very long list of sites can be a decent solution for most people in the short term, but it will never be enough for the practically endless amount of source code hosting sites out there. All this to say - @jba has reopened the meta tag issue, so that's a good start :) I only ask that, if the proposal gets rejected, we open a new issue about the problem statement, which is "how do we support any code hosting site without hard-coding it into one big global list". |
We identified a few important problems with the go-source-v2 idea, most notably:
This makes us pretty confident that go-source-v2 as proposed in the other issue is not the right long-term solution. It's a little bit more module-friendly in that it knows what a version is, but it's not module-friendly enough. More thought is clearly needed. And even if we added that go-source-v2 support today, we'd need every For now, instead of defining a new tag that will require widespread adoption but still not be completely right, it seems best to get the most common sites working by making changes to pkg.go.dev directly, and then revisit the topic when we've had more time to think about the right path forward. |
I tried to create a CL that would add support for packages hosted on dmitri.shuralyov.com, a custom server that hosts some Go packages, but it didn't seem to be possible to do by just adding a pattern. So, as you asked, describing it here. The server itself doesn't support displaying source code in the browser, so it defers to another website that can display source code of Go packages. It was possible to implement this with the following $ curl -s 'https://dmitri.shuralyov.com/gpu/mtl?go-get=1' | grep 'go-source'
<meta name="go-source" content="dmitri.shuralyov.com/gpu/mtl https://dmitri.shuralyov.com/gpu/mtl/... https://gotools.org/dmitri.shuralyov.com/gpu/mtl https://gotools.org/dmitri.shuralyov.com/gpu/mtl#{file}-L{line}"> The go-source tag includes 4 parts (as documented): the import path corresponding to the repository root, the repository's home page, a URL pointing to the directory corresponding to the Go package, and a URL template for a specific line in a specific file. This makes it work on godoc.org (e.g., see https://godoc.org/dmitri.shuralyov.com/gpu/mtl). I learned there are differences in how the templates work in pkgsite, which contributed to me not seeing a way this package could be made to work without additional modifications:
I've sent a DNS/DNR CL 246239 to show my local prototype in case it's easier to look at some code to understand some details and/or give feedback, but I certainly don't expect it to be merged as is. I'm not sure if it's in scope of pkgsite to support what's needed to make that package work, or how to best do it, but I wanted to share what I learned from my experience. If it is in scope, please let me know what you think is a good next step. Thank you. |
@dmitshur We definitely want to make your site work. Your CL looks like it's on the right track. We can continue the discussion there. |
Change https://golang.org/cl/246239 mentions this issue: |
Add support for a set of packages that defer to another website to display their source code. To be able to produce expected URLs, we need to add two more URL template variables: • {importPath} - Package import path ("example.com/myrepo/mypkg"). • {base} - Base name of file containing the identifier, including file extension ("file.go"). Also add a new optional Repo URL template for overriding the home page of the repository, which is something that was possible with the original go-source meta tag. Document the existing URL template variables, so that it's easier to understand how to use them. For golang/go#40477. Change-Id: I70b857155f69c5c3ed41e78daccb90153a927290 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/246239 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: kokoro <noreply+kokoro@google.com> Reviewed-by: Jonathan Amsterdam <jba@google.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
It's better for the home page of a repository to be a page that displays all packages contained by that repository, rather than a page that shows the single package whose import path corresponds to the repository root. Make that change by using an import path pattern with the "/..." suffix as the home page URL. Updates golang/go#40477.
Change https://golang.org/cl/285312 mentions this issue: |
Previously, when we encountered a repo whose URL doesn't match an existing pattern, we did not generate any URL templates for it, meaning we could not render source links in the documentation. This CL uses the templates in the go-source meta tag to guess the version-aware templates that are likely to work for the repo. For golang/go#40477 Change-Id: I2d1978da5a6de1af19284233dbab9ac1ae2cb582 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/285312 Trust: Jonathan Amsterdam <jba@google.com> Run-TryBot: Jonathan Amsterdam <jba@google.com> Reviewed-by: Julie Qiu <julie@golang.org>
We discussed this issue in the last golang-tools meeting, and wanted to post and update on discussions from that meeting. We’re sorry that the communication has been spread across multiple threads, which may have made the conversation hard to follow. This thread should be the main source of discussion moving forward. We still support the go-source spec (https://github.com/golang/gddo/wiki/Source-Code-Links). Any source link that worked on godoc.org should continue to work on pkg.go.dev. However, you may be taken to master (as on godoc.org) instead of the specific version you are viewing on pkg.go.dev, if that URL pattern is not supported. The code for this is at internal/source. We will not be moving forward with the new source meta tag proposal. #39559 is slated to be declined by the proposal review committee, for the reasons mentioned in #39559 (comment). However, if you come across a v2+ source link that does not work, please let us know. We will add code to the internal/source package as needed to resolve any issues providing source links to code-hosting sites. |
Hi @jba, may I ask what is current state with pkgsite/internal/source package? I think pkgsite/internal/source is exactly what I want to use. Is it ready to be moved out of the internal package so other tools can rely on it? Or would you suggest letting me vendoring it for now? |
@Bobgy, I don't see that package leaving internal any time soon. But it is fairly stable and you're welcome to vendor it. |
Understood, will do so. Thank you for the great work! It can save me a lot of headache : ) |
This issue tracks requests to provide source links to code-hosting sites.
We won't be pursuing a new go-source meta tag (#39559). Instead, we will add code to the internal/source package as needed.
Use this issue to request that we add a site. Your request can take a few different forms. In order of preference:
Write a CL. Examples: https://golang.org/cl/245379, https://golang.org/cl/245380. If the site requires more than just adding a pattern, describe your changes here first.
Provide the pattern and templates that should be added to the
patterns
variable in source.go, or describe the linking scheme if it doesn't fit our patterns.Provide a link to a repo on the site. We'll determine the patterns.
No need to file a separate issue for your site; just comment here. An exception is the existing issue for launchpad.net: #39627.
The text was updated successfully, but these errors were encountered: