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

Comments

Projects
None yet
9 participants
@empijei
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 from Golang Doc (https://golang.org/pkg/builtin/) shows error to 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

This comment has been minimized.

Show comment
Hide comment
@odeke-em

odeke-em Aug 25, 2017

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

Member

odeke-em commented Aug 25, 2017

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

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Aug 25, 2017

Member

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?).

Member

bradfitz commented Aug 25, 2017

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

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Aug 25, 2017

Member

@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

Member

bradfitz commented Aug 25, 2017

@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

This comment has been minimized.

Show comment
Hide comment
@odeke-em

odeke-em Aug 25, 2017

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.

Member

odeke-em commented Aug 25, 2017

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 sheepbao Aug 25, 2017

@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor
Contributor

ianlancetaylor commented Aug 25, 2017

@broady

This comment has been minimized.

Show comment
Hide comment
@broady

broady Aug 25, 2017

Member

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/

Member

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

This comment has been minimized.

Show comment
Hide comment
@broady

broady Aug 25, 2017

Member

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
Member

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

This comment has been minimized.

Show comment
Hide comment
@griesemer

griesemer Aug 25, 2017

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
Contributor

griesemer commented Aug 25, 2017

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

This comment has been minimized.

Show comment
Hide comment
@myitcv

myitcv Aug 25, 2017

Member

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
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

This comment has been minimized.

Show comment
Hide comment
@myitcv

myitcv Aug 25, 2017

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@kevinburke

kevinburke Aug 25, 2017

Contributor

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

Contributor

kevinburke commented Aug 25, 2017

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

@broady

This comment has been minimized.

Show comment
Hide comment
@broady
Member

broady commented Aug 25, 2017

@broady broady closed this Aug 25, 2017

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Aug 25, 2017

Member

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.

Member

bradfitz commented Aug 25, 2017

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.