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.

Sign up for GitHub

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

Jump to bottom

x/pkgsite: Broken links for custom go-source tags (regression) #44607

Closed
albertito opened this issue Feb 25, 2021 · 10 comments
Closed

x/pkgsite: Broken links for custom go-source tags (regression) #44607

albertito opened this issue Feb 25, 2021 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. pkgsite
Milestone
pkgsite/unplanned

Comments

@albertito
Copy link
Contributor

What did you do?

Navigate to https://pkg.go.dev/blitiri.com.ar/go/spf (or any blitiri.com.ar package).

Click on any type or function name, or any of the source files.

What did you expect to see?

I expected to be taken to the corresponding source page, e.g. https://blitiri.com.ar/git/r/spf/b/master/t/f=spf.go.html#line-49

What did you see instead?

The links are broken because {dir} is not being replaced correctly.

Context and additional information

https://blitiri.com.ar is my personal page, where I host my git and go repositories.

The git web viewer is git-arr, and has been running fine for years.

I create the https://blitiri.com.ar/go/ remote import path by hand for all my Go packages/modules. They use the meta tags as follows:

<meta name="go-import"
  content="blitiri.com.ar/go/log git https://blitiri.com.ar/repos/log" />
<meta name="go-source"
  content="blitiri.com.ar/go/log https://godoc.org/blitiri.com.ar/go/log https://blitiri.com.ar/git/r/log/b/master/t/{dir} https://blitiri.com.ar/git/r/log/b/master/t/{dir}/f={file}.html#line-{line}" />

These go-import meta tags have been working fine for years, and worked well in godoc.org and (IIRC) also in pkg.go.dev at least at some point.

I think what's happening is that {dir} is not being expanded correctly, and instead {file} contains the full path.

As an example, if you navigate to pgd's view of blitiri.com.ar/go/chasquid/internal/safeio, then click on any of the links, you can see they're broken because they are like this:

https://blitiri.com.ar/git/r/chasquid/b/master/t/%7bdir%7d/f=internal/safeio/safeio.go.html#line-26

Note how {file} is being expanded to internal/safeio/safeio.go.html, and {dir} being expanded to {dir} (sic).

Instead, {dir} should be expanding to internal/safeio/, and file to safeio.go as per the spec and historical behaviour.

This is probably related to issue #40477 in some way.

Please let me know if you need any additional information, thanks!
@gopherbot gopherbot added this to the Unreleased milestone Feb 25, 2021
@jamalc jamalc added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 25, 2021
@jamalc jamalc modified the milestones: Unreleased, pkgsite/unplanned Feb 25, 2021
@jamalc
Copy link

jamalc commented Feb 25, 2021

This does look related. @jba should we close in favor of or merge this with #40477.

@jba
Copy link
Contributor

jba commented Mar 9, 2021

@albertito I'd be happy to add templates for your hosting software. If you could just tell me what they should look like for commits other than master, like version tags and commit hashes (the latter are needed for pseudo-versions). Here is a rough spec for the templates I'm looking for: https://go.googlesource.com/pkgsite/+/master/internal/source/source.go#683.

@jba
Copy link
Contributor

jba commented Mar 9, 2021

I looked at git-arr to try to figure this out myself, but I didn't see how to link to a file or directory with a tag. I also don't see how to get the raw bytes of a file, which isn't strictly necessary but is used to generate the proper img URLs in readmes.

@albertito
Copy link
Contributor Author

albertito commented Mar 10, 2021 •
edited

Thanks for the offer!

However, in #40477 (comment) you said:

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.

The go-source metadata tags I use are compliant with go-source spec, used to work in godoc.org and have not changed, yet the source links in pkg.go.dev don't work. To me that seems like a regression, or at least a contradiction in your statement.

FWIW, I much rather have a spec that enables and encourage a more independent federation and ecosystem, without having to burden the Go team and rely on your good will and energy to customize each hosting site or software. If you tell me that's no longer supported and this is the mandated way forward, I strongly think it's the wrong direction, but I will adjust my hosting software and do the custom integration. But it would also contradict your closing statement on issue #40477, I think.

Thanks for all your help and patience with this!

@jba
Copy link
Contributor

jba commented Mar 10, 2021

Technically you are correct: that your links don't work with master is a bug. On the other hand, linking to master when the version is v1.2.3 is not a great user experience.

Why don't I make the master links work now, and when you have upgraded your software to handle tags and commits, let me know and I will generalize.

We understand the desire to have a more federated scheme, but we just don't have one that we're happy with.

@albertito
Copy link
Contributor Author

albertito commented Mar 19, 2021 •
edited

Technically you are correct: that your links don't work with master is a bug. On the other hand, linking to master when the version is v1.2.3 is not a great user experience.

Why don't I make the master links work now, and when you have upgraded your software to handle tags and commits, let me know and I will generalize.

SGTM, thanks!

Please let me know when the fix is in and I can check the links by hand to help verify it works.

I'll let you know once git-arr supports views of tags (and maybe arbitrary commits, although it being a static generator that might not be practical).

Thanks again!

@gopherbot
Copy link

Change https://golang.org/cl/303609 mentions this issue: internal/source: handle blitiri.com.ar import paths

gopherbot pushed a commit to golang/pkgsite that referenced this issue Mar 22, 2021
@jba
internal/source: handle blitiri.com.ar import paths
7d0f1a1 
The source for these import paths is served by the git-arr web
viewer, which doesn't yet handle tags. Until it does, serve
the source from master.

For golang/go#44607

Change-Id: I94047a887ea6f1038ae812864ad343876b74ff8e
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/303609
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
@jba
Copy link
Contributor

jba commented Apr 4, 2021

@albertito This should be live.

@albertito
Copy link
Contributor Author

Thanks! I can see that it works for my particular site now, thanks, I appreciate the workaround :)

Has the regression for the general case been fixed though? What I mentioned above (#44607 (comment) and #44607 (comment)) affects all sites relying on the go-source spec, and IIUC you committed to continue to support.

Thanks!

@jba
Copy link
Contributor

jba commented Aug 6, 2021

Sorry I didn't respond to your last comment, but I think I answered your question at #44607 (comment).

@jba jba closed this as completed Aug 6, 2021
@rsc rsc unassigned jba Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. pkgsite
Projects
None yet
Milestone
pkgsite/unplanned
Development

No branches or pull requests
4 participants
@albertito @gopherbot @jamalc @jba