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: `go get -u` updates dependency to incompatible v2 module version #34165

Open
andig opened this issue Sep 7, 2019 · 7 comments

Comments

@andig
Copy link
Contributor

commented Sep 7, 2019

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

$ go version
go version go1.13 darwin/amd64

I've also seen this on go1.12, so its not recently introduced.

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/andig/Library/Caches/go-build"
GOENV="/Users/andig/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/andig/htdocs/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org"
GOROOT="/usr/local/opt/go/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/andig/htdocs/mbmd/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/w5/_c0nb6n90fn96tzw04dtc6240000gn/T/go-build373981879=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I'm using github.com/spf13/cobra/doc, with cobra locked in my go.mod:

github.com/spf13/cobra v0.0.5

cobra/doc indirectly depends on blackfriday:

❯ go mod why github.com/russross/blackfriday
# github.com/russross/blackfriday
github.com/volkszaehler/mbmd/cmd
github.com/spf13/cobra/doc
github.com/cpuguy83/go-md2man/md2man
github.com/russross/blackfriday

❯ go mod graph | grep blackfriday
github.com/cpuguy83/go-md2man@v1.0.10 github.com/russross/blackfriday@v1.5.2

What did you expect to see?

Running go get -u I expect module updates. As for blackfriday, that update should remain in the compatible v1 import path.

What did you see instead?

❯ go get -u
go: finding golang.org/x/tools latest
go: finding github.com/influxdata/line-protocol latest
go: finding github.com/crabmusket/gosunspec latest
go: finding github.com/tcnksm/go-latest latest
go: finding golang.org/x/net latest
go: finding github.com/grid-x/modbus latest
go: finding golang.org/x/sys latest
go: finding github.com/influxdata/influxdb1-client latest
go: finding github.com/grid-x/serial latest
# github.com/cpuguy83/go-md2man/md2man
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/md2man.go:11:16: undefined: blackfriday.EXTENSION_NO_INTRA_EMPHASIS
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/md2man.go:12:16: undefined: blackfriday.EXTENSION_TABLES
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/md2man.go:13:16: undefined: blackfriday.EXTENSION_FENCED_CODE
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/md2man.go:14:16: undefined: blackfriday.EXTENSION_AUTOLINK
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/md2man.go:15:16: undefined: blackfriday.EXTENSION_SPACE_HEADERS
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/md2man.go:16:16: undefined: blackfriday.EXTENSION_FOOTNOTES
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/md2man.go:17:16: undefined: blackfriday.EXTENSION_TITLEBLOCK
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/md2man.go:19:29: too many arguments to conversion to blackfriday.Markdown: blackfriday.Markdown(doc, renderer, extensions)
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/roff.go:19:9: cannot use &roffRenderer literal (type *roffRenderer) as type blackfriday.Renderer in return argument:
        *roffRenderer does not implement blackfriday.Renderer (missing RenderFooter method)
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/roff.go:102:11: undefined: blackfriday.LIST_TYPE_ORDERED
../go/pkg/mod/github.com/cpuguy83/go-md2man@v1.0.10/md2man/roff.go:102:11: too many errors

The error is due to pulling blackfriday v2 into go.mod which does not have the required constants from 1.5.2 anymore (https://github.com/russross/blackfriday/tree/v2):

github.com/russross/blackfriday v2.0.0+incompatible // indirect

I feel that go get -u should not pull in the blackfriday v2 dependency as it is not used. md2man in its latest version still relies on 1.5.2 so import path should not change (https://github.com/cpuguy83/go-md2man/blob/7762f7e404f8416dfa1d9bb6a8c192aa9acb4d19/go.mod):

module github.com/cpuguy83/go-md2man

go 1.12

require github.com/russross/blackfriday v1.5.2

Refs cpuguy83/go-md2man#48

@andig andig changed the title cmd/go: `go get -u` updates to incompatible module version cmd/go: `go get -u` updates to incompatible v2 module version Sep 7, 2019

@andig andig changed the title cmd/go: `go get -u` updates to incompatible v2 module version cmd/go: `go get -u` updates indirect dependency to incompatible v2 module version Sep 7, 2019

@andig

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

I've added https://github.com/andig/blackfriday/ for a small demo of the go get -u behaviour with a direct dependency. Any insight what is wrong here would be highly appreciated.

@andig andig changed the title cmd/go: `go get -u` updates indirect dependency to incompatible v2 module version cmd/go: `go get -u` updates dependency to incompatible v2 module version Sep 10, 2019

@FiloSottile FiloSottile added this to the Go1.14 milestone Sep 10, 2019

@FiloSottile

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

@jayconrod

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2019

This is currently working as designed, but the behavior is unfortunate. #34217 discusses some alternative behaviors.

The reason this happens is that a v2.0.0 tag was added to blackfriday before they migrated to modules, along with the breaking changes that a major version bump implies. Normally, the go command requires v2.0.0 to be associated with modules whose paths end with a /v2 suffix (github.com/russross/blackfriday/v2), but that's only mandatory for repositories that have added a go.mod file. Without a go.mod file, v2.0.0 is considered part of the original module, and go get -u upgrades over the breaking change.

#34217 is a pretty large proposal, but perhaps a narrower fix for this issue would be that go get -u should not upgrade across incompatible major versions.

@andig

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2019

Normally, the go command requires v2.0.0 to be associated with modules whose paths end with a /v2 suffix (github.com/russross/blackfriday/v2), but that's only mandatory for repositories that have added a go.mod file.

Could you elaborate on that? There ist a go.mod file in place: https://github.com/russross/blackfriday/blob/master/go.mod

Or is what we‘re seeing here (keeping https://github.com/golang/go/wiki/Modules#releasing-modules-v2-or-higher in mind) the added case of a go.mod in place but not following the spec for case 1) where it needs to add a /v2 to the module name?

@bcmills

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

There [is] a go.mod file in place:

Not in v2.0.0:
https://github.com/russross/blackfriday/tree/v2.0.0

The go.mod file was added in v2.0.1. IMO that is not the appropriate tag according to the rules at https://semver.org — adding a go.mod file is “adding functionality”, not a “backwards compatible bug fix”, so it should be at least a minor release — but it is what it is.

At any rate, because v2.0.0 lacks a go.mod file' the go command still allows it to be used with the +incompatible suffix, and given support for +incompatible at all it is correct to do so: that version could be used for github.com/russross/blackfriday before v2.0.1 was introduced, so it should continue to be valid afterwards.

@bcmills

This comment has been minimized.

Copy link
Member

commented Sep 12, 2019

If we don't do #34217, then we should consider this as an alternative.

If there is no existing requirement on a module at all, we should probably also prefer the latest compatible version with a go.mod file (if such a version exists) over the semantically-highest +incompatible version.

Unfortunately, that doesn't address the problem of what to do if one of your dependencies already requires an incompatible major version higher than what your own package needs. (At that point, it seems that the only scalable option is to update to the new API and upgrade your way out, and try to stick to dependencies that follow the import compatibility rule from there on out.)

@andig

This comment has been minimized.

Copy link
Contributor Author

commented Sep 14, 2019

Without a go.mod file, v2.0.0 is considered part of the original module, and go get -u upgrades over the breaking change.

That is indeed very unfortunate as it seems to contradict semver or just expected behavior.

If there is no existing requirement on a module at all, we should probably also prefer the latest compatible version with a go.mod file (if such a version exists) over the semantically-highest +incompatible version.

+1. I do not understand the incompatible use case (need to read up) but sticking to compatible versions would be what I‘d expect without reading the docs.

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.