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/gomvpkg: should also rewrite ImportComment for moved packages #10508

Closed
dmitris opened this issue Apr 20, 2015 · 2 comments
Closed

Comments

@dmitris
Copy link
Contributor

dmitris commented Apr 20, 2015

Summary:
gomvpkg should rewrite import comments (https://golang.org/s/go14customimport) next to the package statements (package foo // import "foo")

What version of Go are you using (go version)?
go version go1.4.2 darwin/amd64

What operating system and processor architecture are you using?
OS X 10.10.3

What did you do?
Moved a package using gomvpkg:

mkdir /tmp/foo 
export GOPATH=/tmp/foo
go get -u golang.org/x/net/...
mkdir -p $GOPATH/src/go.corp.company.com/x/golang.org/x
gomvpkg -from golang.org/x/net -to go.corp.company.com/x/golang.org/x/net
cd $GOPATH/src/go.corp.company.com/x/golang.org/x/net
go test -v ./...

What did you expect to see?
All tests passed.

What did you see instead?
Lots of errors like this due to the import comments not being rewritten:

can't load package: package go.corp.company.com/x/golang.org/x/net/context: code in directory /tmp/foo/src/go.corp.company.com/x/golang.org/x/net/context expects import "golang.org/x/net/context"
can't load package: package go.corp.company.com/x/golang.org/x/net/dict: code in directory /tmp/foo/src/go.corp.company.com/x/golang.org/x/net/dict expects import "golang.org/x/net/dict"
can't load package: package go.corp.company.com/x/golang.org/x/net/html: code in directory /tmp/foo/src/go.corp.company.com/x/golang.org/x/net/html expects import "golang.org/x/net/html"

I also added a test in my fork which now fails but I believe should succeed:
https://github.com/dmitris/tools/compare/custom_import_comments

Fixing the issue would make it easier to use gomvpkg for vendoring instead of third-party tools or custom hacks (Perl / sed scripts etc.). CC'ing @bradfitz for comments (would be great to try to "dogfood" gomvpkg for camlistore vendoring! 😄 )

See https://go-review.googlesource.com/#/c/8969/ for a related discussion of gomvpkg (in the context of updating the Go FAQ and the current pointer to github.com/kr/goven)

@dmitris
Copy link
Contributor Author

dmitris commented Apr 20, 2015

I would think that in the rename.move() function, around https://github.com/golang/tools/blob/master/refactor/rename/mvpkg.go#L240
we would need to add the following logic:
is there an import comment next to the package statement with import value matching the "from" parameter? (ex. : package foo // import "github.com/my/foo") If yes, create a new comment node based on the "to" parameter and add it to the AST.

@gopherbot
Copy link

CL https://golang.org/cl/15267 mentions this issue.

@golang golang locked and limited conversation to collaborators Oct 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants