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: 'mod tidy' downgrades an unused indirect dependency upgraded by 'get -u' #29702

Open
SamWhited opened this Issue Jan 12, 2019 · 10 comments

Comments

Projects
None yet
4 participants
@SamWhited
Copy link
Member

SamWhited commented Jan 12, 2019

I thought I remembered an issue about this being fixed already, but I couldn't find it and it's still happening and causing problems. Sorry of this is noise and the issue is still floating around somewhere.

The specific dependency in this example that's being stripped out and re-added does not use modules, but does use semantic versioned tags which testify is pinning to.

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

$ go version
go version devel +a2bb68de4d1c 20190111160859 linux/amd64

Does this issue reproduce with the latest release?

It happens with go1.12beta2, go1.12beta1, and go1.11.4.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN="/home/sam/bin"
GOCACHE="/home/sam/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/sam/go"
GOPROXY=""
GORACE=""
GOROOT="/home/sam/Projects/go"
GOTMPDIR=""
GOTOOLDIR="/home/sam/Projects/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/sam/test/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-build393255074=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Create a program that imports a package that exhibits this behavior. I noticed it with the Stripe SDK, and testify seems to be the dependency causing the problem so the following also has the same issue:

package main

import (
	"fmt"

	"github.com/stretchr/testify/assert"
)

func main() {
	_ = assert.CallerInfo
	fmt.Println("vim-go")
}

Now run go mod init testmod, and go get -u. You will end up with the following go.mod file:

module testmod

go 1.12

require (
	github.com/davecgh/go-spew v1.1.1 // indirect
	github.com/stretchr/objx v0.1.1 // indirect
	github.com/stretchr/testify v1.3.0
)

now run go mod tidy and you wind up with the following:

module testmod

go 1.12

require (
	github.com/davecgh/go-spew v1.1.1 // indirect
	github.com/stretchr/testify v1.3.0
)

What did you expect to see?

The file does not change.

What did you see instead?

Every time you run go mod tidy it strips out indirect dependencies inserted by go get -u and you end up with unmerged changes in your repo.

@mvdan mvdan added the modules label Jan 12, 2019

@mvdan mvdan changed the title go mod tidy and go get -u fight eachother cmd/go: mod tidy and get -u fight each other Jan 12, 2019

@mvdan

This comment has been minimized.

Copy link
Member

mvdan commented Jan 12, 2019

/cc @bcmills

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Jan 14, 2019

Looks like they agree on the build list but disagree about whether dependencies are indirect:

$ go get -u
go: finding github.com/davecgh/go-spew v1.1.1
go: finding github.com/stretchr/objx v0.1.1
go: downloading github.com/davecgh/go-spew v1.1.1

$ cat go.mod
module golang.org/issue/scratch

require (
        github.com/davecgh/go-spew v1.1.1 // indirect
        github.com/stretchr/objx v0.1.1 // indirect
        github.com/stretchr/testify v1.3.0
)

$ go list -m all
golang.org/issue/scratch
github.com/davecgh/go-spew v1.1.1
github.com/pmezard/go-difflib v1.0.0
github.com/stretchr/objx v0.1.1
github.com/stretchr/testify v1.3.0

$ go mod tidy

$ cat go.mod
module golang.org/issue/scratch

require (
        github.com/davecgh/go-spew v1.1.1 // indirect
        github.com/stretchr/testify v1.3.0
)

$ go list -m all
golang.org/issue/scratch
github.com/davecgh/go-spew v1.1.1
github.com/pmezard/go-difflib v1.0.0
github.com/stretchr/objx v0.1.0
github.com/stretchr/testify v1.3.0

@bcmills bcmills added this to the Go1.13 milestone Jan 14, 2019

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Jan 14, 2019

Wait, no. go mod tidy ends up downgrading; that might be a bug.

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Jan 14, 2019

This case is interesting, though: nothing in the build actually imports from objx, and nothing in objx affects the versions of other libraries, so perhaps go mod tidy is correct to remove it from the go.mod file even though it remains in the transitive module graph.

@SamWhited

This comment has been minimized.

Copy link
Member Author

SamWhited commented Jan 14, 2019

This case is interesting, though: nothing in the build actually imports from objx.

The tests do though (via testify/mock), and it appears that mod tidy is adding things behind build tags and test files generally; it's just this one that it's stripping out for some reason.
Even though it doesn't affect any other libraries, if we require pinning to a specific version of objx stripping it out might mean that we pull in a newer version that's broken, no?

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Jan 14, 2019

The tests do though (via testify/mock),

Nothing in the import graph imports testify/mock. go mod tidy only considers the packages transitively imported by the main module and the transitive dependencies of the tests of those packages.

if we require pinning to a specific version of objx stripping it out might mean that we pull in a newer version that's broken, no?

In this case, go test all literally doesn't build anything in objx, so the only way it could be broken is through some invalid module requirement.

@SamWhited

This comment has been minimized.

Copy link
Member Author

SamWhited commented Jan 14, 2019

Nothing in the import graph imports testify/mock. go mod tidy only considers the packages transitively imported by the main module and the transitive dependencies of the tests of those packages.

doc.go imports testify/http which imports testify/mock in a test_round_tripper.go, unless I'm still misunderstanding. It's a weird _ import though. I don't really understand why, not having ever used testify except transitively. It doesn't appear to have any import side effects (thank goodness).

In this case, go test all literally doesn't build anything in objx, so the only way it could be broken is through some invalid module requirement.

I see; I suppose if it's an _ import and nothing is actually used that's fair.

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Jan 14, 2019

doc.go imports testify/http which imports testify/mock

Nothing in the example program imports github.com/stretchr/testify (that is, the package containing doc.go) itself.

@SamWhited

This comment has been minimized.

Copy link
Member Author

SamWhited commented Jan 14, 2019

oops, sorry, you're right of course.

@bcmills bcmills changed the title cmd/go: mod tidy and get -u fight each other cmd/go: 'mod tidy' downgrades an unused indirect dependency upgraded by 'get -u' Jan 14, 2019

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Jan 24, 2019

Why did go get -u decide to look at objx?

@bcmills bcmills self-assigned this Feb 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment