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: wrong file links for submodules #56666

Open
dolanor opened this issue Nov 9, 2022 · 9 comments
Open

x/pkgsite: wrong file links for submodules #56666

dolanor opened this issue Nov 9, 2022 · 9 comments
Labels
pkgsite
Milestone
pkgsite/backlog

Comments

@dolanor
Copy link

dolanor commented Nov 9, 2022

What is the URL of the page with the issue?

https://pkg.go.dev/dagger.io/dagger@v0.3.1#section-sourcefiles

What is your user agent?

Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:106.0) Gecko/20100101 Firefox/106.0

Screenshot

Capture d’écran de 2022-11-09 12-27-37

What did you do?

Clicked on the error.go file link

What did you expect to see?

It should have led me to: https://github.com/dagger/dagger/blob/sdk/go/v0.4.0/sdk/go/error.go

What did you see instead?

https://github.com/dagger/dagger/tree/main/sdk/go/blob/v0.4.0/error.go

Fix

- https://github.com/dagger/dagger/tree/main/sdk/go/blob/v0.4.0/error.go
+ https://github.com/dagger/dagger/blob/sdk/go/v0.4.0/sdk/go/error.go

Issue

it seems when using a submodule with the correct way of tagging it (with a tag containing its subdirectory location), the pkgsite doesn't concatenate the relative to repository root path of the file, but only relative to its respective go.mod submodule.
@dolanor dolanor added the pkgsite label Nov 9, 2022
@gopherbot gopherbot added this to the Unreleased milestone Nov 9, 2022
@dolanor dolanor mentioned this issue Nov 9, 2022
Closed
@hyangah
Copy link
Contributor

hyangah commented Nov 11, 2022 •
edited

When I tried to go get dagger.io/dagger?go-get, I see

     <meta
      name="go-import"
      content="dagger.io/dagger git https://github.com/dagger/dagger-go-sdk"
    />
    <meta
      name="go-source"
      content="dagger.io/dagger     https://github.com/dagger/dagger/tree/main/sdk/go https://github.com/dagger/dagger-go-sdk/tree/main{/dir} https://github.com/dagger/dagger-go-sdk/blob/main{/dir}/{file}#L{line}"
    />

@golang/pkgsite @golang/tools-team Doesn't pkgsite use the directory root info retrieved from the go-source meta tag? Or is it still waiting for the decision on the proposal #39559 @jba ?

@jba
Copy link
Contributor

jba commented Nov 11, 2022

We do, code is at https://go.googlesource.com/pkgsite/+/refs/heads/master/internal/source.
I retracted that proposal (read the end). I just closed it to avoid confusion.

@suzmue suzmue modified the milestones: Unreleased, pkgsite/later Nov 12, 2022
@dolanor
Copy link
Author

dolanor commented Nov 14, 2022

Reading the tests from https://go.googlesource.com/pkgsite/+/refs/heads/master/internal/source, I don't see a nested example for github.com. Hence, I don't really know what to do with our use case.
Is our vanity meta tag setup wrong?

@jamalc
Copy link

jamalc commented Nov 18, 2022

What is the reason to use a different repo root in the go-import tag from the home field in the go-source tag? Can you use the "_" fallback or the same path for the home field in go-source?

For example, cloud.google.com/go/storage is a submodule of cloud.google.com/go. When I run curl https://cloud.google.com/go/storage\?go-get\=1, I see

<meta name="go-import" content="cloud.google.com/go git https://github.com/googleapis/google-cloud-go">
<meta name="go-source" content="cloud.google.com/go https://github.com/googleapis/google-cloud-go https://github.com/GoogleCloudPlatform/gcloud-golang/tree/master{/dir} https://github.com/GoogleCloudPlatform/gcloud-golang/tree/master{/dir}/{file}#L{line}">

@jamalc jamalc added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 18, 2022
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@grouville
Copy link

grouville commented Jan 4, 2023 •
edited

@jamalc, sorry we missed that issue along the way. We'll make sure it doesn't happen again 🙏

Thanks for your insights. To answer your question:

What is the reason to use a different repo root in the go-import tag from the home field in the go-source tag? Can you use the "_" fallback or the same path for the home field in go-source?

Dagger is a monorepo, in which several projects are being versioned. More specifically, 2 tools needed to be versioned inside this repo with potentially different versions (with several go.mod) : our engine (at the root of the project) + a Go SDK (under its own module).

This implementation caused us lots of issues around the versioning of both tools, and we ended up creating a mirror repo which is basically the git filter-tree of our SDK subdir.

When checking the go-get:

curl https://dagger.io\?go-get\=1
<!DOCTYPE html>
<html>
  <head>
    <meta
      name="go-import"
      content="dagger.io/dagger git https://github.com/dagger/dagger-go-sdk"
    />
    <meta
      name="go-source"
      content="dagger.io/dagger     https://github.com/dagger/dagger/tree/main/sdk/go https://github.com/dagger/dagger-go-sdk/tree/main{/dir} https://github.com/dagger/dagger-go-sdk/blob/main{/dir}/{file}#L{line}"
    />
    <meta http-equiv="refresh" content="0; url=https://github.com/dagger/dagger">
  </head>
</html>

We see that we ask the users to download from our mirror, in which we are able to freely version the tool:

name="go-import"
      content="dagger.io/dagger git https://github.com/dagger/dagger-go-sdk"

But, as it is a read-only repo, and using the fact that we would love to centralize any issue on our main monorepo, it would be nice to be able to redirect to the main real implementation of the SDK, when users click on files :

name="go-source"
      content="dagger.io/dagger     https://github.com/dagger/dagger/tree/main/sdk/go https://github.com/dagger/dagger-go-sdk/tree/main{/dir} https://github.com/dagger/dagger-go-sdk/blob/main{/dir}/{file}#L{line}"

Regarding your curl https://cloud.google.com/go/storage\?go-get\=1 example (thanks btw). There is something I am not sure of:

<meta name="go-import" content="cloud.google.com/go git https://github.com/googleapis/google-cloud-go">
<meta name="go-source" content="cloud.google.com/go https://github.com/googleapis/google-cloud-go https://github.com/GoogleCloudPlatform/gcloud-golang/tree/master{/dir} https://github.com/GoogleCloudPlatform/gcloud-golang/tree/master{/dir}/{file}#L{line}">

When going to check the test package on pkg.go.dev, and with above configuration, we see that your first go-source is the same base url as the go-import.

image

When clicking on any of the links, it redirects to the first url specified in the go-source, it redirects indeed to the first url mentioned:
image

Do you think it is possible to have different first url for the go-source and the go-import ? It seems to be the easiest implementation to actually do what we intend to do.

It is surely an issue on our side, but any guidance is welcomed 😇

@grouville
Copy link

Can we please re-open this issue ? 🙏

@jamalc
Copy link

jamalc commented Jan 18, 2023

We can re-open but there is currently no support. The second field in the go-source meta tag should be the repository homepage. You have listed a subdirectory. See more info on this wiki page about the go-source meta tag.

@jamalc jamalc reopened this Jan 18, 2023
@jamalc jamalc removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jan 18, 2023
@dolanor
Copy link
Author

dolanor commented Feb 2, 2023

We're testing our new approach (using redirect directly on the module url). We can close for now.
We will test once we have a new release of our SDK that gets indexed by pkg.go.dev.
I must say I was looking for documentation about the go-source meta tag and somehow I failed for a while, so thank you @jamalc for linking it. (of course, now looking for go-source meta tag on google gives me the documentation nearly immediately).
I still believe that it should have a better place than a github wiki on a now unsupported system (gddo). We should have some vanity url guide either on the pkgsite source documentation, pkg.go.dev or golang source (as, it is used at least by the go modules).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
pkgsite
Projects
None yet
Milestone
pkgsite/backlog
Development

No branches or pull requests
7 participants
@dolanor @hyangah @suzmue @gopherbot @jamalc @jba @grouville