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

Jump to bottom

cmd/go: 'go mod download' should not modify the go.mod file #34450

Closed
jpreese opened this issue Sep 21, 2019 · 9 comments
Closed

cmd/go: 'go mod download' should not modify the go.mod file #34450

jpreese opened this issue Sep 21, 2019 · 9 comments
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Backlog

Comments

@jpreese
Copy link

jpreese commented Sep 21, 2019

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

$ go version 

go1.13 windows/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

set GO111MODULE=nt>

set GOARCH=amd64

set GOBIN=C:\Go\bin

set GOCACHE=C:\Users\jpree\AppData\Local\go-build

set GOENV=C:\Users\jpree\AppData\Roaming\go\env

set GOEXE=.exe

set GOFLAGS=

set GOHOSTARCH=amd64

set GOHOSTOS=windows

set GONOPROXY=

set GONOSUMDB=

set GOOS=windows

set GOPATH=C:\Users\jpree\go

set GOPRIVATE=

set GOPROXY=https://proxy.golang.org,direct

set GOROOT=c:\go

set GOSUMDB=sum.golang.org

set GOTMPDIR=

set GOTOOLDIR=c:\go\pkg\tool\windows_amd64

set GCCGO=gccgo

set AR=ar

set CC=gcc

set CXX=g++

set CGO_ENABLED=1

set GOMOD=C:\k2\protolint\go.mod

set CGO_CFLAGS=-g -O2

set CGO_CPPFLAGS=

set CGO_CXXFLAGS=-g -O2

set CGO_FFLAGS=-g -O2

set CGO_LDFLAGS=-g -O2

set PKG_CONFIG=pkg-config

set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\jpree\AppData\Local\Temp\go-build378974426=/tmp/go-build -gno-record-gcc-switches

PS C:\k2\protolint> go env

set GO111MODULE=

set GOARCH=amd64

set GOBIN=C:\Go\bin

set GOCACHE=C:\Users\jpree\AppData\Local\go-build

set GOENV=C:\Users\jpree\AppData\Roaming\go\env

set GOEXE=.exe

set GOFLAGS=

set GOHOSTARCH=amd64

set GOHOSTOS=windows

set GONOPROXY=

set GONOSUMDB=

set GOOS=windows

set GOPATH=C:\Users\jpree\go

set GOPRIVATE=

set GOPROXY=https://proxy.golang.org,direct

set GOROOT=c:\go

set GOSUMDB=sum.golang.org

set GOTMPDIR=

set GOTOOLDIR=c:\go\pkg\tool\windows_amd64

set GCCGO=gccgo

set AR=ar

set CC=gcc

set CXX=g++

set CGO_ENABLED=1

set GOMOD=C:\k2\protolint\go.mod

set CGO_CFLAGS=-g -O2

set CGO_CPPFLAGS=

set CGO_CXXFLAGS=-g -O2

set CGO_FFLAGS=-g -O2

set CGO_LDFLAGS=-g -O2

set PKG_CONFIG=pkg-config

set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\jpree\AppData\Local\Temp\go-build387406178=/tmp/go-build -gno-record-gcc-switches

What did you do?

Attempted to run go mod download -mod=readonly and was given an error

What did you expect to see?

The go mod download command to run to completion, downloading the required modules in go.mod

What did you see instead?

An error stating that this flag is not supported

Reasoning

Succinctly, it's currently possible that when building a Go application in a Dockerfile, CI/CD pipeline, etc that the go.mod file is updated on the fly. This causes a potential difference in the go.mod file that's located on the local machine/source repository, then what is built post-commit.

It is currently possible to use this flag with go build, but copying go.mod and go.sum, followed by a go mod download is a common pattern when using Dockerfiles.

Additional discussion from the twitterverse: https://twitter.com/FiloSottile/status/1175135833424351234
@thepudds
Copy link
Contributor

See also #26850 (comment).

Separately, if you want to disable downloads entirely, you can do GOPROXY=off, which then should cause a command to fail if it needs to download something.

@jpreese
Copy link
Author

jpreese commented Sep 23, 2019

I don't necessarily want to stop all downloads, it's going to need to happen in a CI/CD pipeline. I'm looking to make sure the go.mod I submit to my pipeline is the exact go.mod that goes to production. go mod download has the potential to change it.

go build can also modify go.mod, but it supports readonly

@toothrot toothrot added modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 23, 2019
@toothrot toothrot added this to the Go1.14 milestone Sep 23, 2019
@toothrot
Copy link
Contributor

@jpreese In this case, I still feel like go mod download could be working as intended similar to the comment that @thepudds linked.

As I understand it, if go.mod needs an update, either it will return an error from go build -mod=readonly or go mod download will update it. Let me know if that makes sense, or if I misunderstood something.

Do you have a scenario where go.mod needs an update, but go build -mod=readonly does not return an error?

/cc @bcmills @jayconrod

@bcmills
Copy link
Contributor

bcmills commented Sep 23, 2019

This is closely related to #31372, in that it describes a situation in which a mod subcommand resolves packages or otherwise implicitly tidies up the go.mod file.

Perhaps it should not. (See #31372 (comment).)

@jpreese
Copy link
Author

jpreese commented Sep 23, 2019

Yes this is pretty much the same request, but with the download command. Similar to the linked comment, I would rather see go mod download not modify go.mod at all, and fail if the given go.mod file is in a state that cannot be resolved.

@jayconrod jayconrod mentioned this issue Sep 24, 2019
Closed
@dolmen
Copy link
Contributor

dolmen commented Sep 26, 2019 •
edited

Workaround: use go list -test -mod=readonly
(Works since go1.11)

In my Travis-CI build I use this:

install:
# Download runtime dependencies and test dependencies
- "if [[ `go version` = 'go version go1.1'[1-9][.\\ ]* ]]; then go list -test && git diff --exit-code ; else go get -t -v ./... ; fi"

@jpreese
Copy link
Author

jpreese commented Sep 26, 2019 •
edited

@dolmen yep, this was a similar thought I had. Just check to see in the pipeline if the go.mod file was updated. We do something similar with formatting related commands.

Just wanted to create this issue so the Go team knew and it was documented, since I believe this flag should be supported.

Even better, just make it so the default behavior is to fail if the go.mod file needs to be updated. I personally think that's how a lot of these commands should work. I like to be explicit when I bump versions.

@bcmills bcmills changed the title cmd/go: go mod download support -readonly cmd/go: 'go mod download' should not modify the go.mod file Sep 26, 2019
@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 26, 2019
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 26, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@bcmills
Copy link
Contributor

bcmills commented Nov 26, 2019

@jpreese, is this still an issue with a go command built from head? #34822 may have fixed it.

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Nov 26, 2019
@gopherbot gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label Nov 26, 2019
@gopherbot
Copy link

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@codyoss codyoss mentioned this issue Jul 21, 2020
Closed
@golang golang locked and limited conversation to collaborators Dec 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
7 participants
@toothrot @rsc @dolmen @bcmills @jpreese @gopherbot @thepudds