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: tools.go/+build tools dependencies in go.mod not updated with any go get -u variant #32345

Closed
zikaeroh opened this issue May 31, 2019 · 10 comments
Assignees
Labels
Milestone

Comments

@zikaeroh
Copy link

@zikaeroh zikaeroh commented May 31, 2019

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

$ go version
go version devel +d53f380 Fri May 31 00:51:48 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

No. (1.12.5)

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/jake/nobackup/gotip_home/cache/go-build"
GOENV="/home/jake/nobackup/gotip_home/config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/jake/nobackup/gotip_home/gopath"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/jake/sdk/gotip"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/jake/sdk/gotip/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/jake/tooldeps/go.mod"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build472536705=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I use the tools.go/+build tools method with gobin to manage tools within my project. This is a boiled down version of my actual setup:

go.mod:

module tooldeps

go 1.12

require (
	golang.org/x/tools v0.0.0-20190529010454-aa71c3f32488
	rsc.io/quote v1.5.1
)

main.go:

package main

import "rsc.io/quote"

func main() {
	println(quote.Hello())
}

tools.go:

// +build tools

package tools

import _ "golang.org/x/tools/cmd/stringer"

Both of the dependencies listed in go.mod are outdated (by design). go list knows about both:

go list -m golang.org/x/tools rsc.io/quote
golang.org/x/tools v0.0.0-20190529010454-aa71c3f32488
rsc.io/quote v1.5.1

And shows their updates (go list -m -u all):

tooldeps
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 [v0.0.0-20190530122614-20be4c3c3ed5]
golang.org/x/net v0.0.0-20190311183353-d8887717615a [v0.0.0-20190522155817-f3200d17e092]
golang.org/x/sync v0.0.0-20190423024810-112230192c58
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a [v0.0.0-20190530182044-ad28b68e88f1]
golang.org/x/text v0.3.0 [v0.3.2]
golang.org/x/tools v0.0.0-20190529010454-aa71c3f32488 [v0.0.0-20190530215528-75312fb06703]
rsc.io/quote v1.5.1 [v1.5.2]
rsc.io/sampler v1.3.0 [v1.99.99]

I want to update both of them, so I try go get -t -u, go get -t -u ./..., or even go get -t -u all.

What did you expect to see?

Both golang.org/x/tools and rsc.io/quote are updated.

What did you see instead?

Only rsc.io/quote is updated:

diff --git a/go.mod b/go.mod
index e97a3c2..f6ab66d 100644
--- a/go.mod
+++ b/go.mod
@@ -3,6 +3,8 @@ module tooldeps
 go 1.12
 
 require (
+	golang.org/x/text v0.3.2 // indirect
 	golang.org/x/tools v0.0.0-20190529010454-aa71c3f32488
-	rsc.io/quote v1.5.1
+	rsc.io/quote v1.5.2
+	rsc.io/sampler v1.99.99 // indirect
 )

In Go 1.12, I can do any of the go get -u, go get -u ./..., or go get -u all and have everything update.

The only ways I've gotten the dependency to update is to remove the +build tools, but that causes the file to error out, as even _ imports of main packages are disallowed. The only way then would be to both have a tools.go file and another dummy package depend on an unrelated non-main package in the dependency (say, golang.org/x/tools/go/analysis), but that isn't ideal.

@thepudds

This comment has been minimized.

Copy link

@thepudds thepudds commented May 31, 2019

Interesting. I had been wondering about this case.

My expectation (or really I guess hope) had been the new go get behavior would look across build tags, but it apparently does not. I don't know if that is accidental or not.

You could try to specify the build tag, e.g., gotip get -tags=tools -u ./....

That might or might not work, but the more interesting question probably is whether or not the go tool should change behavior here.

Healthy chance this is related to changes in #26902 (but I suspect you knew that). A related question would be for GOOS and GOARCH.

CC @jayconrod @bcmills

@thepudds thepudds added the modules label May 31, 2019
@thepudds

This comment has been minimized.

Copy link

@thepudds thepudds commented May 31, 2019

Passing a build tag does indeed update the version of golang.org/x/tools in the go.mod, but the go get fails with an error because stringer is not importable:

$ gotip get -tags=tools -u ./...
go: finding golang.org/x/tools latest
tools.go:5:8: import "golang.org/x/tools/cmd/stringer" is a program, not an importable package
$ grep x/tools go.mod
        golang.org/x/tools v0.0.0-20190530215528-75312fb06703
@zikaeroh

This comment has been minimized.

Copy link
Author

@zikaeroh zikaeroh commented May 31, 2019

I get the same, which sort of makes sense given the tag is what's keeping the directory "sane".

Healthy chance this is related to changes in #26902 (but I suspect you knew that). A related question would be for GOOS and GOARCH.

Yes, I read all of the commits and was waiting for that CL specifically to land to retest updating dependencies on all my projects. 😃

@zikaeroh

This comment has been minimized.

Copy link
Author

@zikaeroh zikaeroh commented May 31, 2019

I know that go mod tidy is one command which goes and inspects all variants of tags to make sure the dependencies work; should go get have that same behavior, but locally? (But as you said, it could just be an oversight.)

@jayconrod jayconrod added this to the Go1.13 milestone May 31, 2019
@jayconrod jayconrod self-assigned this May 31, 2019
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented May 31, 2019

Change https://golang.org/cl/179898 mentions this issue: cmd/go: ignore build tags when 'go get' modifies build list

@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented May 31, 2019

Thanks for reporting this. When loading packages for the purpose of modifying the build list, go get should consider all build tags to be true (except ignore), the same way go mod tidy does. CL 179898 should fix this.

@zikaeroh

This comment has been minimized.

Copy link
Author

@zikaeroh zikaeroh commented May 31, 2019

Pulled that CL locally to test, and it does fix my test case. Thanks.

I'm not sure how much it matters, but the test case in that CL doesn't quite match the "tools" pattern since tools.go is also package m, versus package tools. (It also doesn't check go get -u, but I think that's all the same logic, and does work for me, but I thought I'd mention it.)

@gopherbot gopherbot closed this in 6f7542e May 31, 2019
@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented May 31, 2019

tools.go is also package m, versus package tools

I updated the test to match this. It shouldn't matter, since we don't check package names when constructing the build list. If we ever consolidate that logic, this would be a good regression test.

It also doesn't check go get -u

This should be okay. go get will scan the same packages whether or not -u is set. -u upgrades modules that provide packages imported by the ones named on the command line, but those packages have to be loaded either way.

@zikaeroh

This comment has been minimized.

Copy link
Author

@zikaeroh zikaeroh commented May 31, 2019

This should be okay. go get will scan the same packages whether or not -u is set. -u upgrades modules that provide packages imported by the ones named on the command line, but those packages have to be loaded either way.

Do you mean in terms of the test you've written, or my original test case? I know that I need to use -u somewhere, otherwise no modules get updated (which is what I'd expect).

--

This is only partially related, but I hope when 1.13 gets released the explanation of the "right" way to update dependencies for a project is noted somewhere. I follow all of the changes here, and it's getting a bit difficult to figure out what variant of go get I'll eventually need to use to achieve "update all dependencies to their latest available versions" with effects equivalent to what I currently do in 1.12 (mainly how to update everything while avoiding -u all at all costs due to its bad interaction with docker packages). The explanation for -u is "update modules providing dependencies of packages named on the command line" but doesn't at all explain if it's "correct" to not provide the list at all, or if specifying ./... will be different.

@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Jun 3, 2019

I hope when 1.13 gets released the explanation of the "right" way to update dependencies for a project is noted somewhere.

I'm planning to write a guide for this before 1.13 is out. We clearly need to improve module documentation in a lot of areas, but given the changes to go get, this especially needs to be covered.

To summarize though, in 1.13, go get does the following (skipping the details):

  • Construct the build list by reading go.mod files for the main module and dependencies.
  • Load packages mentioned on the command line and their transitive imports. Add missing modules to the build list.
  • Match modules providing packages with command line arguments. Upgrade each module to the requested version (if no version was requested, upgrade to @latest).
  • If -u was specified, also upgrade modules providing transitive imports of the packages mentioned on the command line.

So -u doesn't control which packages are loaded. It only controls which modules are upgraded.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.