Skip to content

x/pkgsite: source links don't work for non-proxy getters #47982

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
Closed
jba opened this issue Aug 26, 2021 · 9 comments
Closed

x/pkgsite: source links don't work for non-proxy getters #47982

jba opened this issue Aug 26, 2021 · 9 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. pkgsite
Milestone
pkgsite/unplanned

Comments

@jba
Copy link
Contributor

jba commented Aug 26, 2021

If you serve a local module with cmd/pkgsite, the links to source files are wrong.

We should at least make them file: URLs. That will get you to the file, assuming you are browsing on the same machine as the directory. It won't take you to a specific line. It also won't help with zips, which are served directly from the module cache when the -cache flag is given. But it's better than the broken links we have now.
@jba jba added NeedsFix The path to resolution is known, but the work has not been done. pkgsite labels Aug 26, 2021
@jba jba added this to the pkgsite/unplanned milestone Aug 26, 2021
@jba jba self-assigned this Aug 26, 2021
@jba jba mentioned this issue Aug 27, 2021
Closed
2 tasks
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/347751 mentions this issue: internal/source: support source links to local files

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/347749 mentions this issue: internal/fetch: get source info from module getter

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/347750 mentions this issue: many: use safehtml.URLs for source links

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/347752 mentions this issue: internal/fetch: support local directory source links

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/347929 mentions this issue: internal/fetch: reorganize testdata directory

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/347931 mentions this issue: internal/fetch: put ModuleInfo into each unit

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/347932 mentions this issue: cmd/pkgsite et. al.: serve source

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/347930 mentions this issue: internal/fetch: use modcache dir for cache getter

@jba
Copy link
Contributor Author

jba commented Sep 7, 2021

The solution uses a /files endpoint on the server, for more flexibility.

gopherbot pushed a commit to golang/pkgsite that referenced this issue Sep 7, 2021
@jba
internal/fetch: get source info from module getter
7c857cb 
Make it the ModuleGetter's responsibility to provide links to source
files, instead of passing around a separate source.Client.

This will allow getters that fetch from disk to serve those local
files.

For golang/go#47982

Change-Id: I6c7cd7890b03ad61d9410850a510c1c9a63f1c9a
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/347749
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Sep 7, 2021
@jba
internal/fetch: reorganize testdata directory
b06752c 
The modcache subdirectory did not correctly resemble a true module
cache. Move the proxy part into a cache/download subdirectory, and
expand one module zip, since we'll be serving source from the expanded
files, not the zip.

For golang/go#47982

Change-Id: Ib5d49f67d2e7a47f8763958d28f44861aea9296b
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/347929
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Sep 7, 2021
@jba
internal/fetch: use modcache dir for cache getter
abd967e 
A minor change to the ModuleGetter for the module cache: it
now takes the cache directory itself, not the cache/download
subdirectory. This will facilitate serving source.

For golang/go#47982

Change-Id: I646471297e38de686586f2180608fc32cb59496d
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/347930
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Sep 7, 2021
@jba
internal/fetch: put ModuleInfo into each unit
68f9951 
Install the full ModuleInfo from the parent module into each
unit. Previously this wasn't necessary in fetch because
Unit.ModuleInfo was populated when reading from the DB. But it's
necessary now that we can serve directly from a fetched module.

For golang/go#47982

Change-Id: I0b0462148f7623ccbbbe7d4ca3bf2409e2107023
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/347931
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>
@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 NeedsFix The path to resolution is known, but the work has not been done. pkgsite
Projects
None yet
Milestone
pkgsite/unplanned
Development

No branches or pull requests
2 participants
@gopherbot @jba