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: run constantly fails and suggests 'go mod tidy' #41416

Closed
myitcv opened this issue Sep 16, 2020 · 13 comments
Closed

cmd/go: run constantly fails and suggests 'go mod tidy' #41416

myitcv opened this issue Sep 16, 2020 · 13 comments
Labels
FrozenDueToAge GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@myitcv
Copy link
Member

myitcv commented Sep 16, 2020

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

$ go version
go version devel +eaa97fbf20 Wed Sep 16 03:02:13 2020 +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="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/myitcv/gostuff/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/playground/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-build826187771=/tmp/go-build -gno-record-gcc-switches"

What did you do?

exec go mod tidy
exec go run blah.com

-- blah/go.mod --
module blah.com

go 1.16
-- blah/main.go --
package main

func main() {
}
-- go.mod --
module mod.com

go 1.16

replace blah.com => ./blah
-- go.sum --
-- main.go --
package main

func main() {
}

What did you expect to see?

Success

What did you see instead?

go: found blah.com in blah.com v0.0.0-00010101000000-000000000000
go: updates to go.mod needed; try 'go mod tidy' first

cc @bcmills @jayconrod

@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Sep 16, 2020
@myitcv
Copy link
Member Author

myitcv commented Sep 16, 2020

Hmm this doesn't even appear to be specific to directory replace directives.

@bcmills
Copy link
Contributor

bcmills commented Sep 16, 2020

This is almost certainly a consequence of CL 251881 / #40728, and (except for the details of the error message) it's mostly working as designed.

That said, we should at least give a better error message when the arguments include one or more packages outside the main module (for which go mod tidy won't necessarily do any good). For packages outside the main module, go get would suffice as an alternative, but for individual source files there isn't really a good alternative to -mod=mod at the moment.

CC @matloob

@bcmills bcmills added this to the Go1.16 milestone Sep 16, 2020
@bcmills bcmills changed the title cmd/go: run constantly require a go mod tidy cmd/go: run constantly fails and suggests 'go mod tidy' Sep 16, 2020
@myitcv
Copy link
Member Author

myitcv commented Sep 16, 2020

(except for the details of the error message)

As discussed offline, the main problem here is with my bad repro 😄

The repro above does not include a // +build tools-esque file that _ requires blah.com, despite the fact that I thought it did in my local testing. Adding such a file ensures a require directive exists for blah.com with a v0.0.0-00010101000000-000000000000 version and all is good.

Thanks for taking the time to patiently discover the real cause (me)!

@jayconrod jayconrod self-assigned this Sep 16, 2020
@jayconrod
Copy link
Contributor

I'll take a look at this. There are some changes to go.mod that don't cause errors in -mod=readonly: things like adding a go directive to go.mod, formatting, or changing // indirect comments. Adding a null requirement for a replacement should probably be one of those (actually i thought it already was).

@bcmills
Copy link
Contributor

bcmills commented Sep 16, 2020

The null requirement can actually be significant, depending on what the replacement's go.mod file says about other dependencies. (For example, it can change the output of go list -m all for other modules — and will continue to do so even once we have lazy loading.)

@jayconrod
Copy link
Contributor

@bcmills Just to confirm what you're saying, the two files below are not equivalent:

module mod.com

go 1.16

replace blah.com => ./blah
module mod.com

go 1.16

require blah.com v0.0.0-00010101000000-000000000000

replace blah.com => ./blah

If we don't load a package from blah.com, we won't add the null requirement or read the go.mod file from the replacement. That go.mod could require higher versions of other dependencies.

So it's correct that we report an error here, but the recommendation to run go mod tidy isn't helpful. go mod tidy doesn't fix this because packages in blah.com aren't in all.

@toothrot
Copy link
Contributor

@jayconrod @bcmills What are the next steps on fixing this, given that it is a release blocker?

@jayconrod
Copy link
Contributor

I plan to work on this before the freeze. Should just involve fixing the error message.

@gopherbot
Copy link

Change https://golang.org/cl/258220 mentions this issue: cmd/go/internal/modload: allow 'go get' to use replaced versions

@gopherbot
Copy link

Change https://golang.org/cl/263146 mentions this issue: cmd/go: change error message for missing import with unused replacement

gopherbot pushed a commit that referenced this issue Oct 16, 2020
'go mod tidy' has been able to use replaced versions since CL 152739,
but 'go get' failed for many of the same paths. Now that we are
recommending 'go get' more aggressively due to #40728, we should make
that work too.

In the future, we might consider factoring out the new replacementRepo
type so that 'go list' can report the new versions as well.

For #41577
For #41416
For #37438
Updates #26241

Change-Id: I9140c556424b584fdd9bdd0a747842774664a7d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/258220
Trust: Bryan C. Mills <bcmills@google.com>
Trust: Jay Conrod <jayconrod@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@bcmills
Copy link
Contributor

bcmills commented Oct 16, 2020

As of CL 258220, the error message is now:

$ go run blah.com
no required module provides package blah.com; try 'go get -d blah.com' to add it

If the replace directive specified some explicit version other than the latest one, that suggestion might fetch a version other than the intended one, but it's still much better than suggesting go mod tidy.

@bcmills
Copy link
Contributor

bcmills commented Oct 16, 2020

The go mod tidy suggestion from the original also suggests to me that there is an error in our inAll computation, possibly related to #37376.

@gopherbot
Copy link

Change https://golang.org/cl/263266 mentions this issue: cmd/go/internal/modload: fix sort condition in (*replacementRepo).Versions

gopherbot pushed a commit that referenced this issue Oct 17, 2020
…sions

In CL 258220 I added replacement versions to the repo versions used in
the modload.Query functions. The versions are computed from a map in
the modfile index, which has a nondeterministic iteration order.

I added a short-circuit condition to skip sorting in the (vastly
common) case where no replacement versions are added. However, while
cleaning up the change I accidentally deleted the line of code that
sets that condition. As a result, the test of that functionality
(mod_get_replaced) has been failing nondeterministically.

This change fixes the condition by comparing the slices before and
after adding versions, rather than by setting a separate variable.
The test now passes reliably (tested with -count=200).

Updates #41577
Updates #41416
Updates #37438
Updates #26241

Change-Id: I49a66a3a5510da00ef42b47f20a168de66100db6
Reviewed-on: https://go-review.googlesource.com/c/go/+/263266
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
@golang golang locked and limited conversation to collaborators Oct 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants