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: package not changed in files with different OS build tags #27145

Open
banks opened this issue Aug 22, 2018 · 1 comment
Open
Labels
Milestone

Comments

@banks
Copy link

@banks banks commented Aug 22, 2018

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10.2 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

GOHOSTARCH="amd64"
GOHOSTOS="darwin"

What did you do?

Ran gomvpkg -from github.com/hashicorp/consul/agent/proxy -to github.com/hashicorp/consul/agent/proxyprocess -vcs_mv_cmd "git mv {{.Src}} {{.Dst}}"

If you checkout the current head of Consul 4d658f34cfcb5c2e0b29ae5103e923872bddcaa7 and run the same command you should see the same issue.

What did you expect to see?

All files moved to the new package name and their package declarations updates

What did you see instead?

All files moved but two files with build tags that didn't match my host OS were not modified and so kept the old package proxy declaration.

The two files are https://github.com/hashicorp/consul/blob/4d658f34cfcb5c2e0b29ae5103e923872bddcaa7/agent/proxy/process_windows.go#L1-L3

// +build windows

package proxy

and

https://github.com/hashicorp/consul/blob/4d658f34cfcb5c2e0b29ae5103e923872bddcaa7/agent/proxy/exitstatus_other.go#L1-L3

// +build !darwin,!linux,!windows

package proxy

This is unpleasant as build and test pass locally still naturally!

This may well be a "known" issue caused by something in the tool chain that is hard to work around, however I don't see an open issue for it and it's certainly surprising to me that this wouldn't work for files with os-dependent build tags. Even if the tooling makes it hard to fix those for some reason, it would be better to at least detect that happening and warn the user to update manually.

Thanks!

@gopherbot gopherbot added this to the Unreleased milestone Aug 22, 2018
@banks banks changed the title x/tools/cmd/gomvpkg: package not changed in files for different build target x/tools/cmd/gomvpkg: package not changed in files with different OS build tags Aug 22, 2018
@banks

This comment has been minimized.

Copy link
Author

@banks banks commented Aug 22, 2018

I notice on reading the help docs more closely there is a caveat which appears to apply here:

gomvpkg will not always be able to rename imports when a package's name is changed.
Import statements may want further cleanup.

It feels like this case is common enough that we could at least be more specific about it there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.