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

cmd/go: document that <meta> tags should be early in the HTML for go get #8163

Closed
gopherbot opened this issue Jun 7, 2014 · 6 comments
Closed

cmd/go: document that <meta> tags should be early in the HTML for go get #8163

gopherbot opened this issue Jun 7, 2014 · 6 comments
Labels
FrozenDueToAge
Milestone
Go1.5

Comments

@gopherbot
Copy link

by jbowtie:

What does 'go version' print?

go version go1.2.1 linux/amd64

What steps reproduce the problem?

$ go get -v gitlab.com/jbowtie/ratago

What happened?

Fetching https://gitlab.com/jbowtie/ratago?go-get=1
Parsing meta tags from https://gitlab.com/jbowtie/ratago?go-get=1 (status code 200)
import "gitlab.com/jbowtie/ratago": parsing gitlab.com/jbowtie/ratago: XML
syntax error on line 5: expected attribute name in element
package gitlab.com/jbowtie/ratago: unrecognized import path
"gitlab.com/jbowtie/ratago"


What should have happened instead?

Should have located the meta tag embedded in the head:

<meta name="go-import" content="gitlab.com/jbowtie/ratago git
https://gitlab.com/jbowtie/ratago.git";>

Please provide any additional information below.

It looks like parseMetaGoImports in cmd/go/discovery.go is trying to parse the returned
document as XML. However, valid HTML documents are frequently not valid XML. In this
particular case a javascript code block in the header contains an unescaped less-than
sign which makes error recovery unlikely.

The correct way to handle this is probably to use a proper HTML5 parser when served
text/html with the HTML5 doctype.

In gitlabhq/gitlabhq#5958 I will suggest moving the meta tag to
an earlier point in the head (ie, before the problematic javascript).
@griesemer
Copy link
Contributor

Comment 1:

Labels changed: added repo-main.

@bradfitz bradfitz removed the new label Dec 18, 2014
@rsc
Copy link
Contributor

rsc commented Apr 10, 2015

Not going to put a full HTML parser in go get, sorry.
But we should document that the tag should be early in the HTML.

@rsc rsc changed the title cmd/go: go get cannot discover meta tag in HTML5 documents cmd/go: document that <meta> tags should be early in the HTML for go get Apr 10, 2015
@rsc rsc added this to the Go1.5 milestone Apr 10, 2015
@rsc rsc removed the repo-main label Apr 14, 2015
@gopherbot
Copy link
Author

CL https://golang.org/cl/12148 mentions this issue.

@rsc rsc closed this as completed in 687925f Jul 15, 2015
@tv42
Copy link

tv42 commented Dec 18, 2015

Sorry to necro this issue, but "early" is not enough.

HTML5 does not enforce <head>...</head> to be used, and many HTML minifiers strip it out, leading to this error from encoding/xml even if the meta tag is literally right after doctype and title.

go get chitin.io/chitin fails because of this, because I used https://github.com/tdewolff/minify on it. Yet it's perfectly valid HTML5, and the meta tag is near the beginning.

Here's a HTML5 validator being happy with a sample page (different url because /chitin is a 404, but it has the same meta tags):
https://html5.validator.nu/?doc=https%3A%2F%2Fchitin.io%2F&showsource=yes

@tv42 tv42 mentioned this issue Dec 18, 2015
Closed
@bradfitz
Copy link
Contributor

@tv42, please don't "necro" issues. File a new one if it's important. We don't track closed bugs, so your chances of an already-fixed bug getting fixed are... well, very good. This is already fixed!

@rsc
Copy link
Contributor

rsc commented Dec 18, 2015

It's certainly fine - and encouraged - to mention this old issue in the new
one, though.

On Fri, Dec 18, 2015 at 2:24 PM, Brad Fitzpatrick notifications@github.com
wrote:

@tv42 https://github.com/tv42, please don't "necro" issues. File a new
one if it's important. We don't track closed bugs, so your chances of an
already-fixed bug getting fixed are... well, very good. This is already
fixed!

—
Reply to this email directly or view it on GitHub
#8163 (comment).

@tv42 tv42 mentioned this issue Dec 18, 2015
Closed
@golang golang locked and limited conversation to collaborators Dec 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.5
Development

No branches or pull requests
5 participants
@bradfitz @rsc @tv42 @griesemer @gopherbot