-
Notifications
You must be signed in to change notification settings - Fork 18k
x/pkgsite: support different GOOS/GOARCH values when displaying package documentation #37232
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
Comments
Here is a package where documentation rendering is GOARCH dependent: |
This is trickier to implement than it might seem, because we want to do as much preprocessing as possible on the worker. If we just generated doc from source at serving time, this would be easy, but that's too slow. We would like to at least parse the files on the worker, and generate doc from the ASTs on the frontend. A
is perfectly fine and won't interfere with compiling, unless the That means that we need a list of valid GOOS/GOARCH combinations. We could parse a file if it matches at least one combination. That list can be large, but not unbounded, so you won't be able to get docs for an arbitrary combination. You could still have two disjoint sets of files, each with a different package name. So what do we record as the name of the package? You could argue that that's not a reasonable setup, and we could just take the first name we find. |
Note that you can obtain such a list by running |
Yep. We bounced a few ideas on what the next iteration could be; mainly, just focusing on Edit: the retracted proposal was #39005. |
Change https://golang.org/cl/258737 mentions this issue: |
Package records the GOOS/GOARCH combination it was created with; Render checks it. Someday we'll allow multiple combos, but it requires a bit of refactoring in internal/fetch, and could conceivably cause us to fail on packages that we now succeed on (by parsing a file from a build context that we don't currently get to). So we'll leave it for later. See golang/go#37232. Change-Id: I3d5e608fefebf4a9e825d1f8f8a5d1166b7e573b Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/258737 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>
A decent intermediate/minimum viable change would be a predefined set of tag sets. On a related note, it would be nice if there was a way to indicate what the default GOOS should be. For example, https://pkg.go.dev/github.com/keybase/go-keychain is pretty useless for anything other than |
Change https://golang.org/cl/279457 mentions this issue: |
Change https://golang.org/cl/279458 mentions this issue: |
loadPackage has two return values: a *goPackage and an error. Before this CL, if a package's documentation was too large, loadPackage would return the package and a non-nil error. With this CL, that error is stored in the goPackage struct in a new field, and loadPackage returns a nil error. Aside from being more idiomatic, this change will enable future changes needed to support multiple GOOS/GOARCH combinations. For golang/go#37232 Change-Id: If73adde76239046eb22409659d6795c09bbb1fd4 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/279457 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>
Handle BuildContexts where one or more parts are "all". These should only occur when searching for a matching Documentation, not sorting. (Because we sort only when there is more than one Documentation, and if there is an all/all Documentation then there aren't any others.) For golang/go#37232 Change-Id: I898aba8f73d2682798d56c47cc6773709f10c702 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/290094 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>
If all the build contexts for a package produce the same documentation, then return a single Documentation with build context all/all. This will save a lot of space in the documentation table, since most packages will fit into this case. For golang/go#37232 Change-Id: I15237242e0c3ca3c7b8f8c8944b227afebd23785 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/289680 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>
If it is not all/all, show the build context at the bottom of the documentation. This is just a preliminary implementation, so users can experience the feature before we provide a proper UI. For golang/go#37232 Change-Id: Ic48a246a8793e61917a31f66580249997255ba77 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/290095 Trust: Jonathan Amsterdam <jba@google.com> Run-TryBot: Jonathan Amsterdam <jba@google.com> Reviewed-by: Jamal Carvalho <jamal@golang.org>
UI design: top right corner of Documentation section will show:
|
Just to clarify, did you really mean this dropdown will include all 41 platform combinations listed by |
This seems to not be working as expected: https://pkg.go.dev/github.com/kardianos/service?GOOS=windows
|
@tianon We currently only check a small subset of that list. Currently we recognize the three cases I mentioned. In particular, if the docs differ for any of our checked build contexts, then we'll record them all. That's not ideal but with our short list it's manageable. |
@firelizzard18 This feature isn't officially live yet. We need to reprocess all packages to make it work. That will happen soon. |
I know that this is still an active thing, I just hit the same problem and would like to reinforce what looks like the solution you are already thinking about. I just added module support for my Windows GUI library wui and in my readme linked to https://pkg.go.dev/github.com/gonutz/wui/v2 which does not show much because almost all my files are tagged Also a heuristic for a sensible default GOOS and GOARCH could be a good thing. In my case 3 of 30 files are tagged Windows, in this case the site could suggest what it considers the common combination somewhere visible at the top when you go to the default (unspecified GOOS and GOARCH) version of the site? |
We're actively working on a better UI. It should be out soon. |
Nice, that is what I was guessing from seeing this and other issues and discussions. I just wanted to give my two cents about it. Good job, keep up the good work and I am looking forward to it :-) |
Change https://golang.org/cl/297550 mentions this issue: |
Adds documentation build context data to support the build context dropdown on the doc pages. For golang/go#37232 Change-Id: I8ff333f811612b89db95064cf387b12e0316ed13 Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/297550 Trust: Julie Qiu <julie@golang.org> Trust: Jamal Carvalho <jamal@golang.org> Run-TryBot: Julie Qiu <julie@golang.org> Reviewed-by: Julie Qiu <julie@golang.org> TryBot-Result: kokoro <noreply+kokoro@google.com>
Maybe I'm missing something, but what exactly is preventing this? I would imagine it would work like this:
I would very much like to see support for Go tags, which would more or less require such a more flexible scheme. |
Change https://golang.org/cl/297552 mentions this issue: |
@aykevl, that's an interesting thought. The problem is that we use We could probably extract the logic of that function and save just the build constraints of each file, but that would be brittle, especially in light of the new build constraint syntax. |
I see, that complicates it a lot. I agree that copying the logic from the build package is way too brittle, there are many special exceptions there. One hacky workaround would be to save just the first part of all the *.go files (up to the Other than that, I think the only solution would involve a significant refactor in how build tags are handled in the build package. For example, it could provide a method to read the build tags from a Go file in a structure that can be serialized and deserialized. Later (on page load, when the build tags are known) it could use this structure to determine which files to include. Basically, the refactor would need to separate reading the build tags from determining which build tags apply to a given file. |
The go/build/constraint package, which is new in Go 1.16, can help with some of this. |
We still need a way to go from a go source file to a Given the structure of the |
That's interesting! That would certainly simplify this.
There is also the Currently of course the But of course I'm not really involved in this. I would just like to see support for build tags on pkg.go.dev, which are not feasible with the current design. |
What did you do?
os/exec.LookPath
has different docs on Linux and Windows. CompareWhat did you expect to see?
An easy to discover and use way to see OS specific package documentation on the pkg.go.dev discovery site.
What did you see instead?
No documented or otherwise discoverable solution on the pkg.go.dev discovery site.
Notes
This idea extends to GOARCH specific or build tag specific docs.
The text was updated successfully, but these errors were encountered: