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: fails on build problems elsewhere in the GOPATH #10907

Open
jmhodges opened this issue May 19, 2015 · 4 comments
Open

x/tools/cmd/gomvpkg: fails on build problems elsewhere in the GOPATH #10907

jmhodges opened this issue May 19, 2015 · 4 comments
Assignees
Labels
Milestone

Comments

@jmhodges
Copy link
Contributor

@jmhodges jmhodges commented May 19, 2015

Was calling

gomvpkg -from github.com/jmhodges/foobar/baz -to github.com/jmhodges/foobar/quux

and gomvpkg errored out with

While scanning Go workspace:
Package "github.com/jmhodges/serve": /Users/jmhodges/src/github.com/jmhodges/serve/serve.go:1:1: expected 'package', found 'IDENT' ypackage.
Package "github.com/dominikh/go-mode.el/indentation_tests": found packages dangling_operator.go (main) and gh-10.go (gh10) in /Users/jmhodges/src/github.com/dominikh/go-mode.el/indentation_tests.
gomvpkg: failed to construct import graph.

These packages are unrelated and it would be nice for gomvpkg to be able to build errors in unrelated areas. This might have to be an additional feature of buildutil.

@minux

This comment has been minimized.

Copy link
Member

@minux minux commented May 19, 2015

@jmhodges

This comment has been minimized.

Copy link
Contributor Author

@jmhodges jmhodges commented May 19, 2015

Well, the short answer is to emit a log message, and move on because if they care about it, they'll run into the build issue and run gofmt -r or sed to fix it.

I understand your point. However, it seems likely that most folks will have dirty GOPATHs. Even popular projects like bmizerany/pat had multiple packages in an example directory, breaking these tools when they hit it, for a very long time. We all accumulate checkouts of projects either aren't up to our standards or depend on other things.

Expecting each engineer to track every dependency of things that they, say, only use a simple binary of, and then expect them to know how to fix them to run a refactoring tool on their own code is is a lot to ask of them. Not all of us are senior engineers who can just dive in and know how to fix the problem and do so quickly. And those senior engineers that do are often the ones most strapped for time.

On top of that, it's often recommended that $GOPATH be set to $HOME (I even recommend it!) and I think the users could be forgiven for trying to reuse a directory named ~/src. As Go becomes more popular, tools like go-mode.el and google-cloud-sdk (which refers to a appengine package that can't be found by normal tools) are going to end up in $GOPATH. This is going to become a worse problem, not a better one, over time.

Finally, Lots of the same tools that use buildutil are also going to have to learn about partially compiling projects in order to perform useful refactors just like IDEs can do. These errors from gomvpkg/buildutil are just a part of that same use case. Engineers are often performing changes in multiple files at once, but know that they can get it back to a resolvable form. Nice refactoring tools accommodate that.

We can't get every open source project and its dependencies to be perfect in their repositories, and we can't stop folks from putting things in $GOPATH that don't build, and we can't fix up the code for them if they're too strapped for time or experience, but we can make the tools do nicer things when they run.

@minux

This comment has been minimized.

Copy link
Member

@minux minux commented May 20, 2015

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jun 4, 2015

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

alandonovan added a commit to golang/tools that referenced this issue Jun 26, 2015
And in gomvpkg, don't stop just because some packages had errors.
This is inevitable in a large GOPATH tree.

Fixes issue golang/go#10907

Change-Id: I9a60b070228d06d44880202eeef54394e914f5d5
Reviewed-on: https://go-review.googlesource.com/10715
Reviewed-by: Sameer Ajmani <sameer@golang.org>
@gopherbot gopherbot added the Tools label Sep 12, 2019
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
5 participants
You can’t perform that action at this time.