golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.31k stars 17.7k forks source link

cmd/go: `go mod tidy` introduces ambiguous imports in pruned modules #60313

Closed bcmills closed 1 year ago

bcmills commented 1 year ago

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

/tmp/census3$ go version
go version devel go1.21-b60db8f7d9 Fri May 19 18:01:57 2023 +0000 linux/amd64

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='/usr/local/google/home/bcmills/.cache/go-build'
GOENV='/usr/local/google/home/bcmills/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/usr/local/google/home/bcmills/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/usr/local/google/home/bcmills'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/google/home/bcmills/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLDIR='/usr/local/google/home/bcmills/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.21-b60db8f7d9 Fri May 19 18:01:57 2023 +0000'
GCCGO='/usr/bin/gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/tmp/census3/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1942413867=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Clone vocdoni/census3@559b89d34e19824f0027a0724c136f9cac694673 and go mod tidy it twice:

$ git clone https://github.com/vocdoni/census3.git
$ git checkout 559b89d34e19824f0027a0724c136f9cac694673
$ go mod tidy
$ go mod tidy

What did you expect to see?

Both go mod tidy operations should succeed.

What did you see instead?

The first go mod tidy succeeds, but it removes a requirement needed to prevent an ambiguous import in a test dependency of a transitively-imported package, causing the second go mod tidy to fail:

/tmp/census3$ go mod tidy

/tmp/census3$ go mod tidy
github.com/vocdoni/census3/contracts/aragon/want imports
        github.com/ethereum/go-ethereum/accounts/abi imports
        github.com/ethereum/go-ethereum/crypto imports
        github.com/btcsuite/btcd/btcec/v2/ecdsa tested by
        github.com/btcsuite/btcd/btcec/v2/ecdsa.test imports
        github.com/btcsuite/btcd/chaincfg/chainhash: ambiguous import: found package github.com/btcsuite/btcd/chaincfg/chainhash in multiple modules:
        github.com/btcsuite/btcd v0.20.1-beta (/usr/local/google/home/bcmills/pkg/mod/github.com/btcsuite/btcd@v0.20.1-beta/chaincfg/chainhash)
        github.com/btcsuite/btcd/chaincfg/chainhash v1.0.2 (/usr/local/google/home/bcmills/pkg/mod/github.com/btcsuite/btcd/chaincfg/chainhash@v1.0.2)

(CC @matloob)

Example courtesy of @mvdan.

gopherbot commented 1 year ago

Change https://go.dev/cl/496518 mentions this issue: cmd/go: add a test that reproduces an unstable 'go mod tidy'

bcmills commented 1 year ago

This is related to #27899, but simpler in that it only requires preserving existing versions of modules (not upgrading to newer versions).

gopherbot commented 1 year ago

Change https://go.dev/cl/496635 mentions this issue: cmd/go: retain extra roots to disambiguate imports in 'go mod tidy'

bcmills commented 1 year ago

@gopherbot, please backport to Go 1.19 and 1.20. This bug causes go mod tidy to produce go.mod files that then fail on subsequent go mod tidy invocations. That happens only rarely, but the failure mode is confusing and the workarounds are not obvious.

gopherbot commented 1 year ago

Backport issue(s) opened: #60351 (for 1.19), #60352 (for 1.20).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

gopherbot commented 1 year ago

Change https://go.dev/cl/499635 mentions this issue: [release-branch.go1.20] cmd/go: retain extra roots to disambiguate imports in 'go mod tidy'

gopherbot commented 1 year ago

Change https://go.dev/cl/499636 mentions this issue: [release-branch.go1.19] cmd/go: retain extra roots to disambiguate imports in 'go mod tidy'

gopherbot commented 1 year ago

Change https://go.dev/cl/502695 mentions this issue: cmd/go/internal/modload: address comment and test issues from CL 496635

RaghavSood commented 1 year ago

I'm still seeing a similar issue when running go mod tidy, using the current tip of 1.19 (9a2e6c9cc2ffa970b8ac5ecf0ea272415e1d9e03).

github.com/coinhako/superwallet/bch imports
    github.com/btcsuite/btcd/chaincfg imports
    github.com/btcsuite/btcd/chaincfg/chainhash: ambiguous import: found package github.com/btcsuite/btcd/chaincfg/chainhash in multiple modules:
    github.com/btcsuite/btcd v0.22.0-beta (/home/ubuntu/go/pkg/mod/github.com/btcsuite/btcd@v0.22.0-beta/chaincfg/chainhash)
    github.com/btcsuite/btcd/chaincfg/chainhash v1.0.1 (/home/ubuntu/go/pkg/mod/github.com/btcsuite/btcd/chaincfg/chainhash@v1.0.1)
bcmills commented 1 year ago

@RaghavSood, the go mod tidy change only helps if the package is unambiguous to begin with. You need to go get one of those modules or the other to tell it which to use, and then go mod tidy should preserve that choice going forward.

RaghavSood commented 1 year ago

I did try that by running the go get github.com/btcsuite/btcd/chaincfg/chainhash@v1.0.1, but it still results in the same error upon go mod tidy

bcmills commented 1 year ago

Do both the btcd and chainhash modules appear explicitly in your go.mod file? If they do, you'll need to upgrade or downgrade one or the other of them to a version that does not contain the ambiguous package.

RaghavSood commented 1 year ago

Only btcd is directly referenced in go.mod - chainhash is pulled in as an indirect dependency by other packages I depend on

ubuntu@sw-raghav-dev:~/dev/superwallet$ grep btcsuite go.mod
    github.com/btcsuite/btcd v0.23.4
    github.com/btcsuite/btcutil v1.0.3-0.20201208143702-a53e38424cce
bcmills commented 1 year ago

Ah, then perhaps you don't have lazy module loading enabled? (Does your go.mod file indicate go 1.17 or higher?)

costela commented 1 year ago

@bcmills could this maybe still be an issue in the transitive case?

test_gomod_bug imports
    my.private.package imports
    go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc tested by
    go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.test imports
    google.golang.org/grpc/interop imports
    golang.org/x/oauth2/google imports
    cloud.google.com/go/compute/metadata: ambiguous import: found package cloud.google.com/go/compute/metadata in multiple modules:
    cloud.google.com/go v0.65.0 (/go/pkg/mod/cloud.google.com/go@v0.65.0/compute/metadata)
    cloud.google.com/go/compute/metadata v0.2.3 (/go/pkg/mod/cloud.google.com/go/compute/metadata@v0.2.3)

But my.private.package has an // indirect entry for cloud.google.com/go/compute/metadata (which was being pruned before #60352, but is now correctly left). So before the fix, repeated go mod tidy would fail in my.private.package, but now it fails on packages that import it.

I expected the // indirect dep to satisfy the requirement you mentioned, no?

You need to go get one of those modules or the other to tell it which to use

bcmills commented 1 year ago

@costela, consumers of my.private.package may still need to go get a specific module to disambiguate when they first add the dependency. #27899 is the issue for that more general problem.

tranvictor commented 1 year ago

go get github.com/btcsuite/btcd/chaincfg/chainhash works now as chainhash just upgraded to v1.0.2.

go: upgraded github.com/btcsuite/btcd/chaincfg/chainhash v1.0.1 => v1.0.2