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/tools/godoc: godoc failing on aliases when viewing pkg/builtin #21601

Closed
empijei opened this issue Aug 25, 2017 · 13 comments
Closed

x/tools/godoc: godoc failing on aliases when viewing pkg/builtin #21601

empijei opened this issue Aug 25, 2017 · 13 comments

Comments

@empijei
Copy link
Contributor

empijei commented Aug 25, 2017

This is not really an issue with the language but with golang.org

What did you do?

Visited https://golang.org/pkg/builtin/

What did you expect to see?

The doc

What did you see instead?

A build error

2017-08-24_17 35 02_crop_screenshot

@odeke-em odeke-em changed the title Golang Doc (https://golang.org/pkg/builtin/) shows error x/tools/godoc: godoc failing on aliases when viewing pkg/builtin Aug 25, 2017
@gopherbot gopherbot added this to the Unreleased milestone Aug 25, 2017
@odeke-em
Copy link
Member

I can reproduce this even locally and interestingly godoc at https://golang.org says it is running on Go1.9
screen shot 2017-08-24 at 9 50 56 pm

where the failing code in question is
screen shot 2017-08-24 at 9 52 22 pm

/cc @broady @griesemer @adams-sarah

@bradfitz
Copy link
Contributor

Works for me running x/tools/cmd/godoc locally.

Seems like the golang.org godoc is built weird. Maybe it's a mix of versions. (one version godoc, one version GOROOT?).

@bradfitz
Copy link
Contributor

@odeke-em, what do you mean by you can "reproduce this even locally". You can make it fail? I can't. It works for me: http://gdev.bradfitz.com:6060/pkg/builtin/#byte

@odeke-em
Copy link
Member

Good proposition @bradfitz actually that seems plausible, now I just re-ran go get golang.org/x/tools/cmd/godoc and now it works alright locally. Perhaps we need to do the exact same thing on production.

@golang golang deleted a comment from zboya Aug 25, 2017
@ianlancetaylor
Copy link
Contributor

CC @griesemer @broady

@broady
Copy link
Contributor

broady commented Aug 25, 2017

golang.org should be running on the 1.9 release branch for tools. Looking at this bug, though, it probably isn't. Wither I made a mistake when deploying, or the 1.9 release branch for tools is missing something from master. Former is more likely.

looks good on tip.golang.org: https://tip.golang.org/pkg/builtin/

@broady
Copy link
Contributor

broady commented Aug 25, 2017

Oh, it's because GAE is on Go <1.9. godoc uses the standard library.

I'm not really sure what to do here, other than move from GAE standard to GAE flexible (or GKE), where we'd have more control over that.

Are there other instances of broken pages, or just builtin?

Maybe a short term fix is a monkey patch to make the content of the builtin page static.

edit: ok, looks like the only type aliases in the standard library are for rune and and byte.

It's not very correct, but we could apply this patch (basically, going back to 1.8) just for golang.org. That is:

@@ -85,11 +85,11 @@ type uintptr uintptr
 // byte is an alias for uint8 and is equivalent to uint8 in all ways. It is
 // used, by convention, to distinguish byte values from 8-bit unsigned
 // integer values.
-type byte = uint8
+type byte byte

 // rune is an alias for int32 and is equivalent to int32 in all ways. It is
 // used, by convention, to distinguish character values from integer values.
-type rune = int32
+type rune rune

@griesemer
Copy link
Contributor

The built-in package is a fake package anyway and only here for documentation. It seems fine to me to go back to 1.8 with that file for godoc only. Maybe:

type byte uint8 // really: type byte = uint8, but alias declarations are not supported in 1.8

@myitcv
Copy link
Member

myitcv commented Aug 25, 2017

Could be unrelated, but godoc.org doesn't appear to be returning results as before.

The search https://godoc.org/?q=os doesn't return the os package itself.

Affected packages:

os
strings

@myitcv
Copy link
Member

myitcv commented Aug 25, 2017

Ditto for strings - I'll now amend my previous comment with other packages that suffer the same problem

@kevinburke
Copy link
Contributor

kevinburke commented Aug 25, 2017

@myitcv, can you open a separate ticket at github.com/golang/gddo?

@broady
Copy link
Contributor

broady commented Aug 25, 2017

Fixed https://golang.org/pkg/builtin/

via monkey patch of https://golang.org/cl/59136

@broady broady closed this as completed Aug 25, 2017
@bradfitz
Copy link
Contributor

I'm not really sure what to do here, other than move from GAE standard to GAE flexible (or GKE), where we'd have more control over that.

I strongly believe we need to move off GAE standard. We need control of the Go version going forward. We will by definition always be ahead of App Engine, and using GAE standard precludes us from ever using new Go features in godoc. This will just keep biting us. We should move to Flex or GKE.

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

9 participants