Skip to content
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

Closed
jba opened this issue Jul 29, 2020 · 15 comments
Closed

x/pkgsite: linking to module source code #40477

jba opened this issue Jul 29, 2020 · 15 comments

Comments

@jba
Copy link
Contributor

jba commented Jul 29, 2020

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:

  1. 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.

  2. 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.

  3. 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.

@gopherbot gopherbot added this to the Unreleased milestone Jul 29, 2020
@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 29, 2020
@myitcv
Copy link
Member

myitcv commented Jul 29, 2020

We won't be pursuing a new go-source meta tag (#39559)

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?

@mvdan
Copy link
Member

mvdan commented Jul 29, 2020

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.

@dmitshur
Copy link
Contributor

dmitshur commented Jul 29, 2020

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.

@mvdan
Copy link
Member

mvdan commented Jul 29, 2020

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".

@jba
Copy link
Contributor Author

jba commented Jul 29, 2020

We identified a few important problems with the go-source-v2 idea, most notably:

  • The only guarantee for a module is a zip file. There may not even be a publicly browseable source code repository available.
  • The extra meta tag is only accessible if you go back to the original redirect page. It's not proxyable like all the other module metadata, nor is it preserved if the origin goes away. (Part of why it's not proxyable is that it's only a godoc.org add-on, not something the Go toolchain has ever defined.)
  • The rules around {revision} are very specific to Git repos and bake in details about pseudo-versions that may change in the future.

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 go get redirector to be updated before any links would start working. That's a lot to ask for a design that we're not even sure is right, especially when a better design might not require any changes at all. The right answer might be to display the code directly from the zip files, or it might be to put some info in the zip file that helps find a source display, or it might be something else entirely. We don't know.

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.

@dmitshur
Copy link
Contributor

dmitshur commented Aug 3, 2020

  1. 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.

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 go-source meta tag, using the dmitri.shuralyov.com/gpu/mtl package as an example:

$ 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:

  1. There's no pattern for the repo/module home page. It's always just {repo} and isn't configurable, so I couldn't find a way to add the "/..." suffix as done in the go-source meta tag.
  2. The {repo} substitution unconditionally includes a "https://" prefix immediately followed by the repo root, and I didn't see a way to use a different domain to display the source code.
  3. The {file} substitution is not just the file name (as it is on godoc.org), but the full path to file from the module root. I didn't see a way to split apart just the name of the file, needed for my template. Similarly, I didn't see a way to compute the import path of Go package whose file (or directory) is to be viewed, as that requires combining {repo} with just the directory of {file}.
  4. On godoc.org, the go-source meta tag applies to an individual package, so it's possible for a custom server to provide templates that work for each package. The templates in pkgsite seem to apply to a repository (or module), so I couldn't work around limitations of templates on the side of the server. (This isn't a problem if there's an equivalent way of doing what I needed via the template.)

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.

@jba
Copy link
Contributor Author

jba commented Aug 3, 2020

@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.

@gopherbot
Copy link

Change https://golang.org/cl/246239 mentions this issue: internal/source: add support for dmitri.shuralyov.com/... packages

gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 28, 2020
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>
@julieqiu julieqiu removed the website label Sep 19, 2020
@julieqiu julieqiu removed the website label Sep 22, 2020
@jba

This comment has been minimized.

@mvdan

This comment has been minimized.

@jba jba removed help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 15, 2020
dmitshur added a commit to shurcooL/home that referenced this issue Jan 2, 2021
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.
@gopherbot
Copy link

Change https://golang.org/cl/285312 mentions this issue: internal/source: match old templates to find new ones

gopherbot pushed a commit to golang/pkgsite that referenced this issue Jan 21, 2021
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>
@jba
Copy link
Contributor Author

jba commented Jan 21, 2021

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.

@Bobgy
Copy link

Bobgy commented Jun 25, 2021

Hi @jba, may I ask what is current state with pkgsite/internal/source package?
I'm trying to build a go license tool that can give versioned links to licenses on code hosting websites, see google/go-licenses#73.

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?

@jba
Copy link
Contributor Author

jba commented Jun 25, 2021

@Bobgy, I don't see that package leaving internal any time soon. But it is fairly stable and you're welcome to vendor it.

@Bobgy
Copy link

Bobgy commented Jun 25, 2021

Understood, will do so. Thank you for the great work! It can save me a lot of headache : )

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants