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/cmd/goimports: doesn't traverse hidden files (dotfiles) #34100

Closed
preslavmihaylov opened this issue Sep 5, 2019 · 7 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@preslavmihaylov
Copy link

preslavmihaylov commented Sep 5, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.9 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/pmihaylov/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/pmihaylov/programming/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.9/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.9/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/9v/vvj1r6_54pb855bvmtqv3wn80000gn/T/go-build618023056=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

In a go project:

  1. Create the file .gen/foo/foo.go with contents:
package foo

import "fmt"

func Foo() {
	fmt.Println("Foo")
}
  1. Create the file bar/bar.go with contents:
package bar

import "fmt"

func Bar() {
	fmt.Println("Bar")
}
  1. Create the file main.go with contents:
package main

func main() {
	foo.Foo()
	bar.Bar()
}
  1. Run goimports main.go.

What did you expect to see?

Both foo and bar packages included

What did you see instead?

Only bar package was included

It would be great if goimports traverses hidden directories as we use a .gen folder for storing auto-generated code (e.g. protobuf). Or at least to export a flag that enables this feature.

From a little tweaking, it seems that Intellij and GoLand already support this feature and that doesn't come from the integrated goimports.

NOTE: If I add the import manually, the program compiles.

@gopherbot gopherbot added this to the Unreleased milestone Sep 5, 2019
@katiehockman katiehockman added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 5, 2019
@katiehockman
Copy link
Contributor

/cc @bradfitz

@heschi
Copy link
Contributor

heschi commented Sep 5, 2019

Clearly goimports shouldn't enter VCS directories like .git, .hg, etc. So allowing all hidden directories isn't viable. That means having a list of directories to avoid or enter, and that means configuration, which I don't think is justified for a use case this niche.

Is it so annoying to rename the directory to gen instead of .gen and have it appear in directory listings?

@preslavmihaylov
Copy link
Author

preslavmihaylov commented Sep 5, 2019

Well, it’s hard to change company-wide standards (I’m working at Uber).

@bradfitz
Copy link
Contributor

bradfitz commented Sep 5, 2019

Sorry, this is not a configuration knob I want to add.

Corporations with their own opinions can have their own internal goimports binary available to their employes. (Google does, for instance, for misc reasons)

@preslavmihaylov
Copy link
Author

preslavmihaylov commented Sep 9, 2019

Alright, fair enough.

In case anyone else has this or a similar issue, here is what I did to create a custom binary that works for me:

  1. Open $GOPATH/src/golang.org/x/tools/internal/gopathwalk/walk.go. Download the sources first if you don't have them (go get ...)
  2. Change this line https://tinyurl.com/y6evpuqy the way you prefer.
  3. Remove the specific golang.org packages from $GOPATH/pkg or simply run rm -rf $GOPATH/pkg && mkdir $GOPATH/pkg if you don't care about the other compiled packages
  4. Recompile goimports with go build goimports.go goimports_not_gc.go in $GOPATH/src/golang.org/x/tools/cmd/goimports
  5. Execute rm -rf $GOPATH/bin/goimports && mv goimports $GOPATH/bin/

@agnivade
Copy link
Contributor

agnivade commented Sep 9, 2019

Sounds like this is resolved. Is there anything else you wanted to be done on this issue ?

@preslavmihaylov
Copy link
Author

Nope, that’s all folks.

@agnivade agnivade closed this as completed Sep 9, 2019
@golang golang locked and limited conversation to collaborators Sep 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants