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 mod tidy introduces ambiguous imports in pruned modules #60313

Closed
bcmills opened this issue May 19, 2023 · 17 comments
Closed

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

bcmills opened this issue May 19, 2023 · 17 comments
Assignees
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented May 19, 2023

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@559b89d 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.

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go modules labels May 19, 2023
@bcmills bcmills self-assigned this May 19, 2023
@bcmills bcmills added this to the Backlog milestone May 19, 2023
@gopherbot
Copy link
Contributor

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

@bcmills
Copy link
Contributor Author

bcmills commented May 19, 2023

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

@gopherbot
Copy link
Contributor

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

@bcmills bcmills modified the milestones: Backlog, Go1.22, Go1.21 May 20, 2023
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label May 20, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 20, 2023
gopherbot pushed a commit that referenced this issue May 22, 2023
For #60313.

Change-Id: I76e48f52341e9962de9b809741a677d61baae6a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/496518
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
@bcmills
Copy link
Contributor Author

bcmills commented May 22, 2023

@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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

gopherbot pushed a commit that referenced this issue Jun 12, 2023
Michael noticed some minor issues in backports of CL 496635.
Those issues have been addressed in the backport CLs; this change
applies them to the main branch as well.

Updates #60313.

Change-Id: If68696711a10a9270193df66ed551395c14cae00
Reviewed-on: https://go-review.googlesource.com/c/go/+/502695
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Jun 19, 2023
…ports in 'go mod tidy'

We don't normally keep explicit requirements for test dependencies of
packages loaded from other modules when the required version is
already the selected version in the module graph. However, in some
cases we may need to keep an explicit requirement in order to make use
of lazy module loading to disambiguate an otherwise-ambiguous import.

Note that there is no Go version guard for this change: in the cases
where the behavior of 'go mod tidy' has changed, previous versions of
Go would produce go.mod files that break successive calls to
'go mod tidy'. Given that, I suspect that any existing user in the
wild affected by this bug either already has a workaround in place
using redundant import statements (in which case the change does not
affect them) or is running 'go mod tidy -e' to force past the error
(in which case a change in behavior to a non-error should not be
surprising).

Updates #60313.
Fixes #60352.

Change-Id: Idf294f72cbe3904b871290d79e4493595a0c7bfc
Reviewed-on: https://go-review.googlesource.com/c/go/+/496635
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 2ed6a54)
Reviewed-on: https://go-review.googlesource.com/c/go/+/499635
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
gopherbot pushed a commit that referenced this issue Jun 19, 2023
…ports in 'go mod tidy'

We don't normally keep explicit requirements for test dependencies of
packages loaded from other modules when the required version is
already the selected version in the module graph. However, in some
cases we may need to keep an explicit requirement in order to make use
of lazy module loading to disambiguate an otherwise-ambiguous import.

Note that there is no Go version guard for this change: in the cases
where the behavior of 'go mod tidy' has changed, previous versions of
Go would produce go.mod files that break successive calls to
'go mod tidy'. Given that, I suspect that any existing user in the
wild affected by this bug either already has a workaround in place
using redundant import statements (in which case the change does not
affect them) or is running 'go mod tidy -e' to force past the error
(in which case a change in behavior to a non-error should not be
surprising).

Updates #60313.
Fixes #60351.

Change-Id: Idf294f72cbe3904b871290d79e4493595a0c7bfc
Reviewed-on: https://go-review.googlesource.com/c/go/+/496635
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 2ed6a54)
Reviewed-on: https://go-review.googlesource.com/c/go/+/499636
TryBot-Bypass: Bryan Mills <bcmills@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
@RaghavSood
Copy link

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

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
Copy link
Contributor Author

bcmills commented Jun 29, 2023

@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
Copy link

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
Copy link
Contributor Author

bcmills commented Jun 29, 2023

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
Copy link

RaghavSood commented Jun 29, 2023

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
Copy link
Contributor Author

bcmills commented Jun 29, 2023

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

@costela
Copy link
Contributor

costela commented Jul 19, 2023

@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
Copy link
Contributor Author

bcmills commented Jul 19, 2023

@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
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants