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: javascript links to wrong lines (cs.opensource.google) #51624

Closed
mindfarm opened this issue Mar 11, 2022 · 6 comments
Closed

x/pkgsite: javascript links to wrong lines (cs.opensource.google) #51624

mindfarm opened this issue Mar 11, 2022 · 6 comments

Comments

@mindfarm
Copy link

What version of Go are you using (go version)?

$ go version

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

Chrome/Ubuntu

go env Output
$ go env

What did you do?

When I go to
https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/database/sql/sql.go;l=1792 I click on "QueryRowContext"

What did you expect to see?

I expect to be taken to the defintion of that method

What did you see instead?

I am taken instead to
https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/database/sql/sql.go;l=1814
Which is an unrelated call inside an unrelated method

It was pointed out that the definition for db.QueryRowContext on master is at https://cs.opensource.google/go/go/+/master:src/database/sql/sql.go;l=1814

It appears that the problem is that the javascript is confusing the branches that it is on, it is linking to

@gopherbot gopherbot added this to the pkgsite/unplanned milestone Mar 11, 2022
@jamalc
Copy link

jamalc commented Mar 14, 2022

I've submitted this feedback to cs.opensource.google. We can leave this open for visibility, but this is out of scope for pkgsite.

@mindfarm
Copy link
Author

Thanks, I wasn't sure where to raise it

@mindfarm
Copy link
Author

mindfarm commented Jun 6, 2022

I tested this this morning and things are still a little confused

That is I go to https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/database/sql/sql.go;l=1792

Click on "QueryRowContext"

and am now taken to
https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/database/sql/sql.go;drc=47f806ce81aac555946144f112b9f8733e2ed871;l=1812

Which has the correct definition for the method, but the version of the code is different ( a look at the page shows that it should instead be linked to https://cs.opensource.google/go/go/+/refs/tags/go1.17.8:src/database/sql/sql.go;l=1777 )

@prattmic
Copy link
Member

prattmic commented Jun 6, 2022

This is a limitation in https://cs.opensource.google/. The cross-reference metadata is generated at one commit. In this case, it is currently 47f806c. This updates about once per day.

When browsing at a different commit [1], the UI still does its best to match references despite source skew. However, to try to link to the right location, it always links to code at the cross-reference commit (this the drc parameter you see in the URL; it is unfortunately rather misleading to include the tag name and drc in the URL).

When browsing the Go 1.17 code, this limitation is certainly rather annoying since the code skew can be quite high. I think https://cs.opensource.google/ would like to be able to address this and support multiple cross-reference metadata commits (one per release branch, perhaps?), but I don't know if / when that will ever come.

P.S. in your original issue comment, I'm not sure why you ended up with a cross-reference link missing the drc component. That may have been a bug that was fixed. The behavior you see today of being taken to a different version is the intended behavior.

[1] This is quite likely even when browsing master, at least by a few commits, since the cross-references are only updated once per day.

@mindfarm
Copy link
Author

mindfarm commented Jun 6, 2022

@prattmic Thanks!

Is there a better way to discuss/resolve this issue - there's no apparent way to directly send feedback to the team looking after the site, nor to get feedback from them on when a fix is rolled out (etc) (More for the future, I think that this particular issue is as resolved as it's going to be)

WRT the feedback on the actual issue, I had suspected that the problem might lie in the building of the map for the javascript to know where to link to because of things like tags/commits changing things often (an update every time a PR was merged to the branch would be expensive, I wonder how Git{lab,hub} manage it).

@prattmic
Copy link
Member

prattmic commented Jun 7, 2022

No, unfortunately I don't think they have much public presence. There should be a "Send feedback" button in the ... menu in the upper right corner of the pages, but that is obviously a one-way mechanism.

I wonder how Git{lab,hub} manage it

FWIW, my experience with GitHub cross-references (not sure about Gitlab) is they seem to be mostly text-search-based which looks like strings that might be the identifier, but is not fully precise. https://cs.opensource.google/ is using Kythe indexing, which actually builds the code to provide completely precise references (for the configuration built; we build linux/amd64 e.g.). The text approach does have the advantage that I imagine it is a bit simpler/cheaper to provide across multiple revisions.

@prattmic prattmic closed this as completed Jun 7, 2022
@golang golang locked and limited conversation to collaborators Jun 7, 2023
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

4 participants