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: module update should drop unused indirect dependencies? #26474

Open
mwf opened this issue Jul 19, 2018 · 9 comments
Open

cmd/go: module update should drop unused indirect dependencies? #26474

mwf opened this issue Jul 19, 2018 · 9 comments
Labels
modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mwf
Copy link

mwf commented Jul 19, 2018

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

Latest go devel: go version devel +d278f09333 Thu Jul 19 05:40:37 2018 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/ikorolev/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/ikorolev/.gvm/pkgsets/go1.11beta1/global:/Users/ikorolev/dev/go"
GOPROXY=""
GORACE=""
GOROOT="/Users/ikorolev/.gvm/gos/go1.11beta1"
GOTMPDIR=""
GOTOOLDIR="/Users/ikorolev/.gvm/gos/go1.11beta1/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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/_b/d1934m9s587_8t_6ngv3hnc00000gp/T/go-build301560759=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Sorry, can't give you a standalone reproduction - bug is produced only with downloading updated dependencies.

Consider the following flow:

A(v0.0.1) uses B(v0.0.1):

module github.com/mwf/vgo-modules/a

require github.com/mwf/vgo-modules/b v0.0.1

Then A(v0.1.0) drops the dependency.

Here we got a project main which uses A(v0.0.1) and we want to upgrade A to v0.1.0

cd `mktemp -d`
git clone https://github.com/mwf/vgo-modules .
echo "\n$(cat go.mod)"
echo "\n$(cat go.sum)"
go get github.com/mwf/vgo-modules/[email protected]
echo "\n$(cat go.mod)"

We get an unexpected indirect dependency github.com/mwf/vgo-modules/b v0.0.1 // indirect in go.mod, it seems to be taken from go.sum as a side-effect.

What did you expect to see?

go.mod should contain updated a dependency

module github.com/mwf/vgo-modules

require github.com/mwf/vgo-modules/a v0.1.0

What did you see instead?

We get an unexpected indirect dependency from b in go.mod, it seems to be taken from go.sum as a side-effect.

module github.com/mwf/vgo-modules

require (
    github.com/mwf/vgo-modules/a v0.1.0
    github.com/mwf/vgo-modules/b v0.0.1 // indirect
)
mwf added a commit to mwf/vgo-modules that referenced this issue Jul 19, 2018
@mwf mwf changed the title cmd/go: updating a module with go get <module> produces old indirect dependencies cmd/go: upgrading a module with go get <module> produces old indirect dependencies Jul 19, 2018
@mwf
Copy link
Author

mwf commented Jul 19, 2018

Currently there is a workaround to run go mod -sync after go get, so maybe the issue is not so critical. But still looks buggy.

@bcmills
Copy link
Contributor

bcmills commented Jul 19, 2018

This is working as intended. (It's a bit confusing, but arguably less confusing than the alternatives.)

In general, go get tries to make the smallest possible change to the build list in order to get the requested modules at the requested versions.

When you started out, you had a direct requirement on a and an indirect one (via a) on b. Upgrading a did not require any changes to b in the build list, so go get did not make them, but that means that the previously-implicit dependency on b must be made explicit in the build file.

In this specific case, we theoretically have enough information to prune out the dependency. However, in the general case that can get expensive: if the main module depends on some other package c that imports b, and c does not have a go.mod of its own, then we don't actually know whether we still need b unless we process all of the (transitive) imports for c. But you didn't tell us to change anything about c and we don't need to touch it otherwise, so if go get started scanning its imports we'd be doing a lot of extra work that you didn't ask for.

$ cd $(mktemp -d)
$ git clone https://github.com/mwf/vgo-modules .

$ go list -m all
github.com/mwf/vgo-modules
github.com/mwf/vgo-modules/a v0.0.1
github.com/mwf/vgo-modules/b v0.0.1

$ go mod -graph
github.com/mwf/vgo-modules github.com/mwf/vgo-modules/[email protected]
github.com/mwf/vgo-modules github.com/mwf/vgo-modules/[email protected]  # bug in mod -graph: #26489
github.com/mwf/vgo-modules/[email protected] github.com/mwf/vgo-modules/[email protected]

$ go get github.com/mwf/vgo-modules/[email protected]

$ go list -m all
github.com/mwf/vgo-modules
github.com/mwf/vgo-modules/a v0.1.0
github.com/mwf/vgo-modules/b v0.0.1

$ go mod -graph
github.com/mwf/vgo-modules github.com/mwf/vgo-modules/[email protected]
github.com/mwf/vgo-modules github.com/mwf/vgo-modules/[email protected]

@bcmills
Copy link
Contributor

bcmills commented Jul 19, 2018

I'm going to close this for now, but we'll keep an eye out for similar issues. Thanks for the great repro steps!

@bcmills bcmills closed this as completed Jul 19, 2018
@mwf
Copy link
Author

mwf commented Jul 19, 2018

Thanks for a great explanation, Brian!
The logic does make sense now when dealing with dependencies without a go.mod file.

Actually the common usage will be

go get module1
go get module2
...
go mod -sync

Which leads to the minimal case of upgrading a single dependency with go get module1 && go mod-sync.
It's really unobvious and a bit confusing why do you need to run 2 commands instead of a single one, but for a bunch of consequent updates it could be really faster than if go get would always traverse the full tree.

I checked go help module-get - this nuance is not covered even in a minimal way (that one may want to run go mod -sync after get).

Let's reopen the issue to update the docs?

And @bcmills, could you please take a look at #26048 (comment) about go install behaviour? Maybe I'm missing there some similar nuances, but I can't see them yet :)

@bcmills bcmills reopened this Jul 20, 2018
@bcmills bcmills added Documentation NeedsFix The path to resolution is known, but the work has not been done. modules labels Jul 20, 2018
@bcmills bcmills added this to the Go1.11 milestone Jul 20, 2018
@bcmills bcmills changed the title cmd/go: upgrading a module with go get <module> produces old indirect dependencies cmd/go: clarify doc for go get <module> and indirect dependencies Jul 20, 2018
@bcmills
Copy link
Contributor

bcmills commented Jul 20, 2018

Let's reopen the issue to update the docs?

Done.

@rsc
Copy link
Contributor

rsc commented Jul 24, 2018

I don't think this is working as intended, fwiw. Maybe I'm wrong. Will return to this.

@rsc
Copy link
Contributor

rsc commented Aug 17, 2018

I think this is probably incorrect behavior but it's not a showstopper for Go 1.11 because 'go mod tidy' will correct it. Don't want to make changes now but we should investigate for Go 1.12.

@rsc rsc modified the milestones: Go1.11, Go1.12 Aug 17, 2018
@rsc rsc changed the title cmd/go: clarify doc for go get <module> and indirect dependencies cmd/go: module update should not add indirect dependencies? Aug 17, 2018
@rsc rsc added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed Documentation NeedsFix The path to resolution is known, but the work has not been done. labels Aug 17, 2018
@rsc rsc modified the milestones: Go1.12, Go1.13 Nov 20, 2018
@bcmills
Copy link
Contributor

bcmills commented Jan 15, 2019

Contrast #29702.

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@bcmills bcmills changed the title cmd/go: module update should not add indirect dependencies? cmd/go: module update should drop unused indirect dependencies? Feb 13, 2020
@bcmills
Copy link
Contributor

bcmills commented Feb 13, 2020

This still reproduces as of Go 1.14 RC1.
(It is somewhat related to #26902, which was fixed in 1.13, but the behavior here is “not removing transitive dependencies that were never actually relevant” rather than “upgrading dependencies that are not relevant”.)

$ go version
go version devel +363bcd00 Wed Feb 12 18:22:50 2020 +0000 linux/amd64

$ git clone https://github.com/mwf/vgo-modules
…

$ cd vgo-modules/

vgo-modules$ cat go.mod
module github.com/mwf/vgo-modules

require github.com/mwf/vgo-modules/a v0.0.1

vgo-modules$ cat go.sum
github.com/mwf/vgo-modules/a v0.0.1 h1:BaK3DzgdA4LxVXd42qID7F9G+KFxQ4SL69hm+BNcjA0=
github.com/mwf/vgo-modules/a v0.0.1/go.mod h1:nK31MtN2feKpYgPBywpeTy/Om+1x3OxQHQAsyi95AQ0=
github.com/mwf/vgo-modules/b v0.0.1 h1:aEPPx6pEuQ1/Wu9SzeLkiYltlir2h1fDJmKRv1ckarY=
github.com/mwf/vgo-modules/b v0.0.1/go.mod h1:7kI8RJ5+IPwKWU1MrE/kTA9AdWBNzMmucdpAiWXATOs=

vgo-modules$ go get github.com/mwf/vgo-modules/[email protected]
go: downloading github.com/mwf/vgo-modules/a v0.1.0

vgo-modules$ cat go.mod
module github.com/mwf/vgo-modules

go 1.14

require (
        github.com/mwf/vgo-modules/a v0.1.0
        github.com/mwf/vgo-modules/b v0.0.1 // indirect
)

vgo-modules$ cat go.sum
github.com/mwf/vgo-modules/a v0.0.1 h1:BaK3DzgdA4LxVXd42qID7F9G+KFxQ4SL69hm+BNcjA0=
github.com/mwf/vgo-modules/a v0.0.1/go.mod h1:nK31MtN2feKpYgPBywpeTy/Om+1x3OxQHQAsyi95AQ0=
github.com/mwf/vgo-modules/a v0.1.0 h1:QD+ijrXwAYk4CMTOqEAzcogJoF4zLfit2alZVmT80EM=
github.com/mwf/vgo-modules/a v0.1.0/go.mod h1:XGJvSKC62ygHgRNmDf4RqXyp0zngXIJMLc6BaSfyTfI=
github.com/mwf/vgo-modules/b v0.0.1 h1:aEPPx6pEuQ1/Wu9SzeLkiYltlir2h1fDJmKRv1ckarY=
github.com/mwf/vgo-modules/b v0.0.1/go.mod h1:7kI8RJ5+IPwKWU1MrE/kTA9AdWBNzMmucdpAiWXATOs=

vgo-modules$ go mod tidy

vgo-modules$ cat go.mod
module github.com/mwf/vgo-modules

go 1.14

require github.com/mwf/vgo-modules/a v0.1.0

vgo-modules$ cat go.sum
github.com/mwf/vgo-modules/a v0.1.0 h1:QD+ijrXwAYk4CMTOqEAzcogJoF4zLfit2alZVmT80EM=
github.com/mwf/vgo-modules/a v0.1.0/go.mod h1:XGJvSKC62ygHgRNmDf4RqXyp0zngXIJMLc6BaSfyTfI=

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants