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

cmd/go: tool dependency specified using tools.go interferes with main module dependency #33926

Open
hyangah opened this issue Aug 29, 2019 · 21 comments

Comments

@hyangah
Copy link
Contributor

@hyangah hyangah commented Aug 29, 2019

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

$ gotip version
go version devel +d0eaec7 Wed Aug 28 21:48:01 2019 +0000 darwin/amd64

What did you do?

Following the tool-dependency best-practice in Module wiki, created tools.go to add the tool dependency. Note: the tool and the main module will share some dependencies, which is a problem.

$ gotip mod init work1
go: creating new go.mod: module work1
$ cat << EOF > tools.go
// +build tools

package whatever
import _ "golang.org/x/tools/cmd/godoc"
EOF

$ gotip mod tidy
go: finding golang.org/x/tools latest
$ cat go.mod
module work1

go 1.13

require golang.org/x/tools v0.0.0-20190828213141-aed303cbaa74

$ gotip build -o godoc0 golang.org/x/tools/cmd/godoc
$ gotip version -m godoc0
godoc0: devel +d0eaec7 Wed Aug 28 21:48:01 2019 +0000
	path	golang.org/x/tools/cmd/godoc
	mod	golang.org/x/tools	v0.0.0-20190828213141-aed303cbaa74	h1:4cFkmztxtMslUX2SctSl+blCyXfpzhGOy9LhKAqSMA4=
	dep	golang.org/x/net	v0.0.0-20190620200207-3b0461eec859	h1:R/3boaszxrf1GEUWTVDzSKVwLmSJpwZ1yqXm8j0v2QI=

Then I added some code in the main module that depends on a module the tool (godoc) also depends on.

$ cat << EOF > foo.go
package foo
import _ "golang.org/x/net/http2"
EOF
$ gotip build
$ cat go.mod
module work1

go 1.13

require (
	golang.org/x/net v0.0.0-20190620200207-3b0461eec859
	golang.org/x/tools v0.0.0-20190828213141-aed303cbaa74
)

The go build of the main module picked up golang.org/x/net@v0.0.0-20190620200207-3b0461eec859
due to the existing requirement on golang.org/x/tools.

I wanted a newer version of golang.org/x/net, so upgraded it using go get golang.org/x/net@latest.

$ gotip get golang.org/x/net@latest
go: finding golang.org/x/net latest
$ cat go.mod
module work1

go 1.13

require (
	golang.org/x/net v0.0.0-20190827160401-ba9fcec4b297
	golang.org/x/tools v0.0.0-20190828213141-aed303cbaa74
)

This resulted in a side-effect: upgrade the dependency of the tool as well. That produces a different binary. (different version of golang.org/x/net).

$ GOBIN=`pwd` gotip install golang.org/x/tools/cmd/godoc
$ gotip version -m godoc
godoc: devel +d0eaec7 Wed Aug 28 21:48:01 2019 +0000
	path	golang.org/x/tools/cmd/godoc
	mod	golang.org/x/tools	v0.0.0-20190828213141-aed303cbaa74	h1:4cFkmztxtMslUX2SctSl+blCyXfpzhGOy9LhKAqSMA4=
	dep	golang.org/x/net	v0.0.0-20190827160401-ba9fcec4b297	h1:k7pJ2yAPLPgbskkFdhRCsA77k2fySZ1zf2zCjvQCiIM=

What did you expect to see?

Stable, consistent build of the tools with the dependencies as specified in the tools go.mod.

What did you see instead?

The dependencies interfere in both ways - the tool dependency affects the version selection choice of the main module, and vice versa.

This can cause confusion - the user would believe the godoc was built from the version of golang.org/x/tools module but some of the dependency was actually picked up based on the
main module's go.mod, instead of the tool's.

@julieqiu
Copy link
Contributor

@julieqiu julieqiu commented Aug 29, 2019

@zikaeroh
Copy link
Contributor

@zikaeroh zikaeroh commented Aug 30, 2019

Perhaps I'm misunderstanding, but isn't this the expected/designed behavior? The tools.go pattern/hack uses an uncompiled import of the tool, which is treated no differently than any other import within a module, so it's definitely going to participate in version selection like any other imported library. To not do this, you'd need to define some other method to write down tool versions and have them work independently, which is not how tools.go is going to operate.

In some sense, I see this as a benefit especially for things that are partially code generators and partially libraries, like something that generates models from a database, because I can always know that the version of the generator I ran matches the library I'm about to use. But, there are certainly cases where mixing things produces a bad result; I had a script which used a single go.mod/tools.go to handle my $GOBIN, but that broke with gopls due to its nesting in x/tools (#32586 (comment)).

@hyangah
Copy link
Contributor Author

@hyangah hyangah commented Aug 30, 2019

@zikaeroh sure, this is working as intended from go.mod's perspective. it's not what users who look for the mentioned best practice for setting tools dependencies.

  • want to ensure that everyone is using the same version of that tool while tracking the tool's version in your module's go.mod file

Yes, this is already well-known problem of the tools.go-based hack. I would be happy if this is no longer recommended as the best practice. I hope it to be resolved through #32504 which I hope to help cleanly build each required tool without corrupting or getting influenced by the main module's library dependency version selection.

@bcmills bcmills added the modules label Sep 3, 2019
@bcmills
Copy link
Member

@bcmills bcmills commented Sep 3, 2019

In general we expect dependencies to remain compatible over time, and we expect incompatibilities to be resolved by upgrading past them (rather than pinning to a point before they began).

Moreover, tools will tend to build faster if they can reuse dependencies shared with the rest of the main module, which would not be possible if those dependencies are at some other version.

In your example with golang.org/x/tools and golang.org/x/net above, did the upgrade actually break anything? (What is the concrete problem that led you to report this?)

@zikaeroh
Copy link
Contributor

@zikaeroh zikaeroh commented Sep 3, 2019

For what it's worth, my reading of "ensure that everyone is using the same version of that tool" is that the tool dependencies should be pinned for all people who are actually "working" on the project that uses them, not necessarily that they should match the "upstream versions" to a tee. It's possible to have both (with some new method of managing tools, which could be useful to isolate things and reduce breakage), but if every developer on a project is using the same go.mod to define the tooling for the project, then they all should have a consistent experience.

(Now, that's not entirely true if someone updates a dependency and the behavior changes, but it's going to be much better to have a repo which says "these are my code generators, they produce this output" and committing it, rather than them depending on some external state like what version of stringer they happen to have in their PATH.)

@hyangah
Copy link
Contributor Author

@hyangah hyangah commented Sep 3, 2019

@bcmills @zikaeroh the problem is that when we report a bug for a tool, e.g. included in golang.org/x/tools, we want the tool owner to know the exact version, not the binary compiled with arbitrary dependencies because I am developing a program that happens to import one of the packages in golang.org/x/tools repo. The 'golang.org/x/tools' repo owners are not working on my private project. Often our intention is to communicate with the actual tool author, not the collaborators of my own private project.

@hyangah
Copy link
Contributor Author

@hyangah hyangah commented Sep 4, 2019

What do you think about removing or demoting this tool dependency sections from the wiki page?
Observed another failure case related to gopls picking up incompatible x/tools version (which is unstable), I don't think this is a good idea to mix up tool dependencies with the main module dependencies.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 4, 2019

Observed another failure case related to gopls picking up incompatible x/tools version (which is unstable)

A stable module should not import packages from an unstable dependency unless the two modules have a requirement cycle. Otherwise, that's not sound semantic versioning: the API of the dependency could change and break the “stable” dependent.

Given sound semantic versioning, it should be fine to mix tool dependencies in with other dependencies, since upgrades to those dependencies should not break anything else.

@hyangah
Copy link
Contributor Author

@hyangah hyangah commented Sep 4, 2019

Given that the module echo system is still not mature and many legacy modules and tools end up depending on pseudoversions and v0.*, I don't think that's easy to enforce.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 27, 2019

Stable, consistent build of the tools with the dependencies as specified in the tools go.mod.

Thinking about this some more: this problem statement is exactly #30515.

So my question is, why did you need to add the tool dependency in the first place? (Is the tool run by a test, or in a //go:generate directive, or something else?)

Would it satisfy your use-case if you could go get the tool at a specific version, independent of your go.mod file?

@tv42
Copy link

@tv42 tv42 commented Oct 2, 2019

If the tool dependency version causes undesirable effects in the main app, you could make a subdirectory with its own go.mod and move tools.go there. You'd need to cd there first before go build/go run, but especially with build you could do that just once and then call the resulting binary from other go:generate lines. Worst case make a little proxy shell script / Go app that calls go build in the right dir and then executes it from the original working directory.

@hyangah
Copy link
Contributor Author

@hyangah hyangah commented Oct 2, 2019

Thinking about this some more: this problem statement is exactly #30515.

Yes.

So my question is, why did you need to add the tool dependency in the first place? (Is the tool run by a test, or in a //go:generate directive, or something else?)

For example, I want to specify the version used for // go:generate (e.g. gobind, etc).
My use cases are as described in the module wiki page:

If you:

  • want to use a go-based tool (e.g. stringer) while working on a module, and
  • want to ensure that everyone is using the same version of that tool while tracking the tool's version in your module's go.mod file
    ...

As we observed, tools.go based approach interferes with the build process unnecessarily.

Would it satisfy your use-case if you could go get the tool at a specific version, independent of your go.mod file?

Yes.

I am happy to repurpose this issue to stop suggesting tools.go as the best practice for specifying tool dependency and update the wiki page.

If go tool decides to support tool dependencies, the dependencies should be specified to avoid such interference with main code build process.

@hyangah
Copy link
Contributor Author

@hyangah hyangah commented Oct 2, 2019

Also, I learned recently - the tool behavior may change depending on the version of go used to build the tool (e.g. gobind behavior changes as the go/format changes), so specifying the version of the module containing the tool is not sufficient to ensure everyone to use the exact same version.

@hyangah
Copy link
Contributor Author

@hyangah hyangah commented Nov 20, 2019

@bcmills I encountered another tools.go file and remembered this :-) Do you need more info? Relabel it if more info is necessary.

@hyangah hyangah removed the WaitingForInfo label Nov 20, 2019
@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Nov 21, 2019

@hyangah I think this problem is solved by the -modfile flag in #34506.

The idea is this: a module author could maintain a separate go.tools.mod file. When they need to run go generate or another command that uses those tools, they would use the flag -modfile=go.tools.mod. This lets tool requirements be maintained outside the main go.mod file. Downstream modules won't be affected by these requirements.

I'm hoping -modfile will be released in Go 1.14. We still need verify this works and solves problems like this during the beta. After 1.14 is released, we can update the wiki and other documentation to make use of this.

@hyangah
Copy link
Contributor Author

@hyangah hyangah commented Nov 21, 2019

@jayconrod Yes, #34506 is a more promising approach than tools.go.

I am happy to classify this as a documentation issue (wiki/Modules) - asking for the best practice for tool dependency management -, add a warning in the best practice mentioning #34506, and close this.

@mvdan
Copy link
Member

@mvdan mvdan commented Nov 21, 2019

Completely agree that this will need proper docs. -modfile is necessary, but it's not obvious how to use it properly for tooling deps.

@tv42
Copy link

@tv42 tv42 commented Nov 21, 2019

I don't see how -modfile would remove a need for something like tools.go. Something needs to list all the modules that are needed, so a module file doesn't get add/remove churn. If anything, using -modfile would make using a tools.go for that impossible.

What I've done to isolate tool deps from main deps is to put the tools in a subdir with its own go.mod. This can make using some tools overly fiddly (cd to special dir, go build, cd to where you want to use the tool, ../../blah/tool), but it provides the same isolation while retaining the tools.go idea.

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Nov 22, 2019

Something needs to list all the modules that are needed, so a module file doesn't get add/remove churn. If anything, using -modfile would make using a tools.go for that impossible.

I expect that a file used with -modfile would have much less churn. It wouldn't be necessary to run go mod tidy all the time.

@mvdan
Copy link
Member

@mvdan mvdan commented Nov 22, 2019

You can also imagine trivial use cases like //go:generate go run -modfile=tools/mytool.mod github.com/whatever/mytool@v1.2.3 args.... And if you change the version and want to tidy, go mod tidy -modfile=tools/mytool.mod would probably work.

@tv42
Copy link

@tv42 tv42 commented Nov 22, 2019

I expect that a file used with -modfile would have much less churn. It wouldn't be necessary to run go mod tidy all the time.

Perhaps you'd need it less often, but when you do want it, there'd be no reasonable way to run it.

And if you change the version and want to tidy, go mod tidy -modfile=tools/mytool.mod would probably work.

It seems to me that would use the product source to discover necessary dependencies -> all tools would get removed, all kinds of non-tool deps would be added.

The tools-in-subdir-with-go.mod trick seems to avoid all these downsides...

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
7 participants
You can’t perform that action at this time.