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

cmd/go: module work & spam when running 'go doc' on a standard-library package #31505

Open
bradfitz opened this issue Apr 16, 2019 · 11 comments

Comments

Projects
None yet
5 participants
@bradfitz
Copy link
Member

commented Apr 16, 2019

I'm trying to read about strconv and modules seems to be trying to do work?

$ go doc strconv.ParseInt
go: finding google.golang.org/api v0.0.0-20170327174102-48e49d1645e2
go: finding cloud.google.com/go v0.0.0-20161006024015-b70ccc799b9d
go: google.golang.org/api@v0.0.0-20170327174102-48e49d1645e2: git -c protocol.version=0 fetch --unshallow -f https://code.googlesource.com/google-api-go-client refs/heads/*:refs/heads/* refs/tags/*:refs/tags/* in /home/bradfitz/pkg/mod/cache/vcs/9e62a95b0409d58bc0130bae299bdffbc7b7e74f3abe1ecf897474cc474b8bc0: exit status 128:
        fatal: remote error: 


        Invalid authentication credentials.

        Please generate a new identifier:
          https://code.googlesource.com/new-password
go: cloud.google.com/go@v0.0.0-20161006024015-b70ccc799b9d: git -c protocol.version=0 fetch --unshallow -f https://code.googlesource.com/gocloud refs/heads/*:refs/heads/* refs/tags/*:refs/tags/* in /home/bradfitz/pkg/mod/cache/vcs/b0e27935eb83c1d7843713bafab507e95768b550f0552cb42d9f41e5fd9c8375: exit status 128:
        fatal: remote error: 


        Invalid authentication credentials.

        Please generate a new identifier:
          https://code.googlesource.com/new-password
go: error loading module requirements
func ParseInt(s string, base int, bitSize int) (i int64, err error)
    ParseInt interprets a string s in the given base (0, 2 to 36) and bit size
    (0 to 64) and returns the corresponding value i.

    If base == 0, the base is implied by the string's prefix: base 2 for "0b",
    base 8 for "0" or "0o", base 16 for "0x", and base 10 otherwise. If base is
    below 0, is 1, or is above 36, an error is returned.

    The bitSize argument specifies the integer type that the result must fit
    into. Bit sizes 0, 8, 16, 32, and 64 correspond to int, int8, int16, int32,
    and int64. If bitSize is below 0 or above 64, an error is returned.

    The errors that ParseInt returns have concrete type *NumError and include
    err.Num = s. If s is empty or contains invalid digits, err.Err = ErrSyntax
    and the returned value is 0; if the value corresponding to s cannot be
    represented by a signed integer of the given size, err.Err = ErrRange and
    the returned value is the maximum magnitude integer of the appropriate
    bitSize and sign.

/cc @bcmills @jayconrod

@elagergren-spideroak

This comment has been minimized.

Copy link

commented Apr 16, 2019

Related: #29452

@bcmills

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

See also #28992.

@bcmills

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

Where did you execute go doc, and what version of the go command were you using?

@bcmills

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

I'm guessing that this is another side-effect of loading the build list eagerly, but switching it to lazy loading will require quite a bit of refactoring. (I've been hoping to do that for 1.13, but with the freeze looming it's not looking very likely.)

@bcmills bcmills added the modules label Apr 17, 2019

@bcmills bcmills changed the title cmd/doc: module work & spam when looking up std docs cmd/go: module work & spam when running 'go doc' on a standard-library package Apr 17, 2019

@bradfitz bradfitz added this to the Go1.13 milestone Apr 17, 2019

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 17, 2019

dev:pk-web $ go version
go version devel +ff3ae455d9 Wed Apr 17 16:36:56 2019 +0000 linux/amd64
dev:pk-web $ pwd
/home/bradfitz/src/perkeep.org/website/pk-web
dev:pk-web $ go doc strconv Atoi
go: finding google.golang.org/api v0.0.0-20170327174102-48e49d1645e2
go: finding cloud.google.com/go v0.0.0-20161006024015-b70ccc799b9d
go: cloud.google.com/go@v0.0.0-20161006024015-b70ccc799b9d: git -c protocol.version=0 fetch --unshallow -f https://code.googlesource.com/gocloud refs/heads/*:refs/heads/* refs/tags/*:refs/tags/* in /home/bradfitz/pkg/mod/cache/vcs/b0e27935eb83c1d7843713bafab507e95768b550f0552cb42d9f41e5fd9c8375: exit status 128:
        fatal: remote error: 


        Invalid authentication credentials.

        Please generate a new identifier:
          https://code.googlesource.com/new-password
func Atoi(s string) (int, error)
    Atoi is equivalent to ParseInt(s, 10, 0), converted to type int.

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 17, 2019

If you can't fix this before Go 1.13, I think we should special case the std packages in cmd/doc. Goimports does. Std is super common & should be fast..

In fact, we should do that regardless.

@bradfitz bradfitz added the NeedsFix label Apr 17, 2019

@bcmills

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

CC @robpike and @mvdan for cmd/doc.

@dmitshur is looking into #28992.

@robpike

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2019

@bradfitz: Define 'special case'. That is, what do you want the command to do?

@bradfitz

This comment has been minimized.

Copy link
Member Author

commented Apr 20, 2019

@robpike, if I run something like:

$ go doc bufio.NewScanner
$ go doc net/http

It should recognize that net/http and bufio are std packages and not invoke the go/packages code that then downloads half of github looking for them, complete with authentication errors along the way.

In goimports we have this auto-generated file that gives goimports that info for what is known to be in std (and thus in $GOROOT): https://github.com/golang/tools/blob/master/imports/zstdlib.go

@robpike robpike self-assigned this Apr 20, 2019

@robpike

This comment has been minimized.

Copy link
Contributor

commented Apr 20, 2019

I can do that, but I can't help thinking there is something much deeper that needs fixing and papering it over is a mistake.

@mvdan

This comment has been minimized.

Copy link
Member

commented Apr 20, 2019

Just a drive-by thought - does go doc strconv parseint have the same issue? It should be much easier for go doc to understand that there isn't any domain or remote host involved in resolving that query, whereas go doc foo.bar is a harder problem.

For example, what should go doc strconv.com do? Perhaps there's a symbol in strconv that happens to match com, or perhaps someone is actually hosting a Go package at that domain. The only solution I can see in this scenario is a whitelist, like @bradfitz suggested. Or to do go doc strconv com, like I suggested above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.