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

Closed
bradfitz opened this issue Apr 16, 2019 · 12 comments
Closed
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bradfitz
Copy link
Contributor

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

Related: #29452

@bcmills
Copy link
Contributor

bcmills commented Apr 17, 2019

See also #28992.

@bcmills
Copy link
Contributor

bcmills commented Apr 17, 2019

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

@bcmills
Copy link
Contributor

bcmills 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 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
Copy link
Contributor Author

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
Copy link
Contributor Author

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 The path to resolution is known, but the work has not been done. label Apr 17, 2019
@bcmills
Copy link
Contributor

bcmills commented Apr 19, 2019

CC @robpike and @mvdan for cmd/doc.

@dmitshur is looking into #28992.

@robpike
Copy link
Contributor

robpike commented Apr 20, 2019

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

@bradfitz
Copy link
Contributor Author

@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
Copy link
Contributor

robpike 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
Copy link
Member

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

@robpike
Copy link
Contributor

robpike commented Jun 16, 2019

I believe this has been fixed by changes to module support elsewhere. Please reopen if not.

@robpike robpike closed this as completed Jun 16, 2019
@golang golang locked and limited conversation to collaborators Jun 15, 2020
@rsc rsc unassigned robpike Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

6 participants