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

Jump to bottom

x/tools/imports: don't prepend std/ to packages from GOROOT #31814

Closed
bradfitz opened this issue May 2, 2019 · 3 comments
Closed

x/tools/imports: don't prepend std/ to packages from GOROOT #31814

bradfitz opened this issue May 2, 2019 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@bradfitz
Copy link
Contributor

bradfitz commented May 2, 2019

In #31769 goimports added "std/go/token" for me. (Because I used a new API only in master and not yet recognized by the goimports fast path)

And in https://go-review.googlesource.com/c/go/+/175099/ I got a failure:

https://storage.googleapis.com/go-build-log/0c3a1672/freebsd-amd64-12_0_de6f6ad5.log

ok  	context	1.249s
ok  	crypto	0.006s
ok  	crypto/aes	0.020s
# crypto/cipher
package crypto/cipher_test
	imports std/internal/testenv: unknown import path "std/internal/testenv": cannot find module providing package std/internal/testenv
FAIL	crypto/cipher [setup failed]
ok  	crypto/des	0.007s
ok  	crypto/dsa	0.004s
2019/05/02 22:10:18 Failed: exit status 1

because goimports added "std/internal/testenv".

We need to remove the std/ prefixes.
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label May 2, 2019
@gopherbot gopherbot added this to the Unreleased milestone May 2, 2019
@gopherbot
Copy link

Change https://golang.org/cl/175101 mentions this issue: imports: remove std/ prefix from added GOROOT imports

@mvdan
Copy link
Member

mvdan commented May 3, 2019

I had the same happen to me while I was working on the go/token CL itself, but I figured I was using goimports wrong :)

@dmitshur dmitshur mentioned this issue May 5, 2019
Closed
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
@gopherbot
Copy link

Change https://golang.org/cl/195063 mentions this issue: internal/imports: don't prefix stdlib package with std/

clintjedwards pushed a commit to clintjedwards/tools that referenced this issue Sep 19, 2019
@heschi @clintjedwards
internal/imports: don't prefix stdlib package with std/
5c6ecec 
When running in GOROOT/src, `go list -m all` shows the std package (or
cmd package) as the main module. This confuses goimports into adding
std/ or cmd/ at the beginning of import paths. Skip canonicalization for
paths under GOROOT.

Fixes golang/go#31814

Change-Id: Iff5cc7e2a2053e4cc87c1a579a4c47d856cd0a2e
Reviewed-on: https://go-review.googlesource.com/c/tools/+/195063
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
@golang golang locked and limited conversation to collaborators Sep 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@bradfitz @mvdan @gopherbot