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: get does not respect GOFLAGS="-mod=readonly" #32147

Closed
kortschak opened this issue May 19, 2019 · 11 comments
Closed

cmd/go: get does not respect GOFLAGS="-mod=readonly" #32147

kortschak opened this issue May 19, 2019 · 11 comments
Labels
FrozenDueToAge 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
Go1.13

Comments

@kortschak
Copy link
Contributor

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

$ go version
go version devel +1ab063c Fri May 17 22:32:30 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

No - it cannot.

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

go env Output
$ go env
GOARCH="amd64"
GOBIN="~/bin"
GOCACHE="~/.cache/go-build"
GOENV="~/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="~"
GOPROXY="direct"
GOROOT="~/go"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="~/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="~/src/gonum.org/v1/gonum/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-build627513913=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Run go test ./... with GOFLAGS="-mod=readonly" in gonum.org/v1/gonum. Originally noticed in a travis log.

What did you expect to see?

No modification of go.mod

What did you see instead?

~/src/gonum.org/v1/gonum [master]$ export GOFLAGS="-mod=readonly"
~/src/gonum.org/v1/gonum [master]$ go test ./...
?   	gonum.org/v1/gonum	[no test files]
?   	gonum.org/v1/gonum/blas	[no test files]
ok  	gonum.org/v1/gonum/blas/blas32	0.012s
ok  	gonum.org/v1/gonum/blas/blas64	0.002s
ok  	gonum.org/v1/gonum/blas/cblas128	0.002s
ok  	gonum.org/v1/gonum/blas/cblas64	0.008s
ok  	gonum.org/v1/gonum/blas/gonum	2.428s
ok  	gonum.org/v1/gonum/blas/testblas	0.026s
?   	gonum.org/v1/gonum/blas/testblas/benchautogen	[no test files]
ok  	gonum.org/v1/gonum/bound	0.002s
ok  	gonum.org/v1/gonum/diff/fd	0.015s
ok  	gonum.org/v1/gonum/floats	0.002s
ok  	gonum.org/v1/gonum/fourier	21.741s
ok  	gonum.org/v1/gonum/fourier/internal/fftpack	0.012s
ok  	gonum.org/v1/gonum/graph	0.025s
ok  	gonum.org/v1/gonum/graph/community	69.349s
?   	gonum.org/v1/gonum/graph/encoding	[no test files]
ok  	gonum.org/v1/gonum/graph/encoding/digraph6	0.013s
ok  	gonum.org/v1/gonum/graph/encoding/dot	1.776s
ok  	gonum.org/v1/gonum/graph/encoding/graph6	0.029s
ok  	gonum.org/v1/gonum/graph/encoding/graphql	0.152s
?   	gonum.org/v1/gonum/graph/ex/fdpclust	[no test files]
ok  	gonum.org/v1/gonum/graph/flow	0.017s
ok  	gonum.org/v1/gonum/graph/formats/cytoscapejs	0.504s
?   	gonum.org/v1/gonum/graph/formats/dot	[no test files]
ok  	gonum.org/v1/gonum/graph/formats/dot/ast	0.016s
ok  	gonum.org/v1/gonum/graph/formats/dot/internal/astx	0.002s
?   	gonum.org/v1/gonum/graph/formats/dot/internal/errors	[no test files]
ok  	gonum.org/v1/gonum/graph/formats/dot/internal/lexer	0.149s
ok  	gonum.org/v1/gonum/graph/formats/dot/internal/parser	0.132s
?   	gonum.org/v1/gonum/graph/formats/dot/internal/token	[no test files]
?   	gonum.org/v1/gonum/graph/formats/dot/internal/util	[no test files]
ok  	gonum.org/v1/gonum/graph/formats/gexf12	0.035s
?   	gonum.org/v1/gonum/graph/formats/graphml	[no test files]
ok  	gonum.org/v1/gonum/graph/formats/sigmajs	0.318s
ok  	gonum.org/v1/gonum/graph/graphs/gen	16.733s
?   	gonum.org/v1/gonum/graph/internal/linear	[no test files]
?   	gonum.org/v1/gonum/graph/internal/ordered	[no test files]
ok  	gonum.org/v1/gonum/graph/internal/set	0.008s
?   	gonum.org/v1/gonum/graph/internal/uid	[no test files]
ok  	gonum.org/v1/gonum/graph/iterator	0.025s
ok  	gonum.org/v1/gonum/graph/multi	0.480s
ok  	gonum.org/v1/gonum/graph/network	0.025s
ok  	gonum.org/v1/gonum/graph/path	41.818s
ok  	gonum.org/v1/gonum/graph/path/dynamic	0.148s
ok  	gonum.org/v1/gonum/graph/path/internal/testgraphs	0.006s
ok  	gonum.org/v1/gonum/graph/simple	0.208s
ok  	gonum.org/v1/gonum/graph/testgraph	0.002s
ok  	gonum.org/v1/gonum/graph/topo	0.827s
ok  	gonum.org/v1/gonum/graph/traverse	0.593s
ok  	gonum.org/v1/gonum/integrate	1.481s
ok  	gonum.org/v1/gonum/integrate/quad	0.121s
ok  	gonum.org/v1/gonum/internal/asm/c128	0.026s
ok  	gonum.org/v1/gonum/internal/asm/c64	0.041s
ok  	gonum.org/v1/gonum/internal/asm/f32	0.021s
ok  	gonum.org/v1/gonum/internal/asm/f64	0.079s
ok  	gonum.org/v1/gonum/internal/cmplx64	0.002s
ok  	gonum.org/v1/gonum/internal/math32	0.028s
?   	gonum.org/v1/gonum/lapack	[no test files]
ok  	gonum.org/v1/gonum/lapack/gonum	102.826s
?   	gonum.org/v1/gonum/lapack/lapack64	[no test files]
ok  	gonum.org/v1/gonum/lapack/testlapack	0.007s
ok  	gonum.org/v1/gonum/mat	73.861s
ok  	gonum.org/v1/gonum/mathext	6.934s
ok  	gonum.org/v1/gonum/mathext/internal/amos	4.191s
?   	gonum.org/v1/gonum/mathext/internal/cephes	[no test files]
?   	gonum.org/v1/gonum/mathext/internal/gonum	[no test files]
ok  	gonum.org/v1/gonum/num/dual	0.013s
ok  	gonum.org/v1/gonum/num/dualcmplx	0.051s
ok  	gonum.org/v1/gonum/num/dualquat	0.007s
ok  	gonum.org/v1/gonum/num/hyperdual	0.002s
ok  	gonum.org/v1/gonum/num/quat	0.014s
ok  	gonum.org/v1/gonum/optimize	34.605s
ok  	gonum.org/v1/gonum/optimize/convex/lp	29.074s
ok  	gonum.org/v1/gonum/optimize/functions	0.684s
ok  	gonum.org/v1/gonum/spatial/barneshut	103.186s
ok  	gonum.org/v1/gonum/spatial/kdtree	4.023s
?   	gonum.org/v1/gonum/spatial/r2	[no test files]
?   	gonum.org/v1/gonum/spatial/r3	[no test files]
ok  	gonum.org/v1/gonum/spatial/vptree	4.832s
ok  	gonum.org/v1/gonum/stat	0.008s
ok  	gonum.org/v1/gonum/stat/combin	0.007s
ok  	gonum.org/v1/gonum/stat/distmat	0.726s
ok  	gonum.org/v1/gonum/stat/distmv	28.425s
ok  	gonum.org/v1/gonum/stat/distuv	94.125s
ok  	gonum.org/v1/gonum/stat/mds	0.006s
ok  	gonum.org/v1/gonum/stat/samplemv	5.604s
ok  	gonum.org/v1/gonum/stat/sampleuv	3.695s
ok  	gonum.org/v1/gonum/stat/spatial	0.454s
ok  	gonum.org/v1/gonum/unit	0.020s
ok  	gonum.org/v1/gonum/unit/constant	0.002s
~/src/gonum.org/v1/gonum [master*]$ git diff
diff --git a/go.mod b/go.mod
index fec89a6..1a0f476 100644
--- a/go.mod
+++ b/go.mod
@@ -5,3 +5,5 @@ require (
        golang.org/x/tools v0.0.0-20190206041539-40960b6deb8e
        gonum.org/v1/netlib v0.0.0-20190313105609-8cb42192e0e0
 )
+
+go 1.13
~/src/gonum.org/v1/gonum [master*]$ go version
go version devel +1ab063c Fri May 17 22:32:30 2019 +0000 linux/amd64
@kortschak
Copy link
Contributor Author

It would be nice if the semantics and intended use of that line in the go.mod file were documented somewhere; I don't see it mentioned in go help modules, in the wiki page or in the Go & Versioning article. It is shown without mention in the blog post, and is not explained.

@kortschak kortschak mentioned this issue May 20, 2019
Merged
@bcmills
Copy link
Contributor

bcmills commented May 20, 2019

It would be nice if the semantics and intended use of that line in the go.mod file were documented somewhere

That's #30791.

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels May 20, 2019
@bcmills bcmills added this to the Go1.13 milestone May 20, 2019
@bcmills bcmills self-assigned this May 20, 2019
@bcmills
Copy link
Contributor

bcmills commented May 20, 2019

I'm not able to reproduce the observed behavior using the repro steps provided.
Could you please double-check the steps to reproduce? If there was a typo in your GOFLAGS — or if some step in your Travis build cleared the environment — then it's possible the go test command was executed without the expected GOFLAGS.

~/src/gonum.org/v1/gonum$ git checkout d76380b3
HEAD is now at d76380b3 mat: check RawVectorers for shadowing and special case unit increments

~/src/gonum.org/v1/gonum$ cat go.mod
module gonum.org/v1/gonum

require (
        golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2
        golang.org/x/tools v0.0.0-20190206041539-40960b6deb8e
        gonum.org/v1/netlib v0.0.0-20190313105609-8cb42192e0e0
)

~/src/gonum.org/v1/gonum$ git status
HEAD detached at d76380b3
nothing to commit, working tree clean

~/src/gonum.org/v1/gonum$ gotip version
go version devel +c6f9321b Mon May 20 04:22:40 2019 +0000 linux/amd64

~/src/gonum.org/v1/gonum$ gotip env GOMOD GOFLAGS
/usr/local/google/home/bcmills/src/gonum.org/v1/gonum/go.mod
-mod=readonly

~/src/gonum.org/v1/gonum$ gotip test ./...
go: updates to go.mod needed, disabled by -mod=readonly

~/src/gonum.org/v1/gonum$ git status
HEAD detached at d76380b3
nothing to commit, working tree clean

@bcmills bcmills added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed release-blocker labels May 20, 2019
@kortschak
Copy link
Contributor Author

I am now unable to repeat this locally for reasons I have no clue about.

So I have investigated further on travis and see that the mutation occurs prior to invoking go test. This explains why the tests do not fail with "go: updates to go.mod needed, disabled by -mod=readonly". The command that does result in the mutation is a tool we have that performs copyright checks (a single file executable here that depends on "go/parser"). This is demonstrated by this more instrumented travis log (ignore the success status, there are diffs that would cause a failure if I were not clobbering the exit status with the instrumentation).

@bcmills
Copy link
Contributor

bcmills commented May 21, 2019

@kortschak, that Travis log unfortunately doesn't tell us much. It seems likely that the culprit is an exec.Command that drops the process's environment (by setting the Env field explicitly).

If you believe that the error is in cmd/go itself, we really need a standalone reproducer.

@kortschak
Copy link
Contributor Author

OK, I can see where this is coming from now. The travis travis_install_go_dependencies bash function does not respect GOFLAGS. This can be seen here where the change has only happened after that function runs (it invokes go get). I'll post an issue at their community discussion forum.

This issue does raise another problem though. I have worked around the failure by adding a go directive line to the go.mod (go 1.10 which is our oldest supported version - from discussion on issues, this looks like it is not how that is supposed to be used, though it works currently). I would like to know how to either specify a range of go versions or to not specify a go version and have the go tool just leave that alone.

@kortschak kortschak changed the title cmd/go: test does not respect GOFLAGS="-mod=readonly" cmd/go: get does not respect GOFLAGS="-mod=readonly" May 25, 2019
@kortschak
Copy link
Contributor Author

I have debugged this on travis and now have a local reproducer. The problem was not with go test, but rather go get.

~/src/gonum.org/v1/gonum [mod-debug]$ git diff
~/src/gonum.org/v1/gonum [mod-debug]$ go get -mod=readonly ./...
~/src/gonum.org/v1/gonum [mod-debug*]$ git diff
diff --git a/go.mod b/go.mod
index fec89a6e..1a0f4760 100644
--- a/go.mod
+++ b/go.mod
@@ -5,3 +5,5 @@ require (
        golang.org/x/tools v0.0.0-20190206041539-40960b6deb8e
        gonum.org/v1/netlib v0.0.0-20190313105609-8cb42192e0e0
 )
+
+go 1.13
~/src/gonum.org/v1/gonum [mod-debug*]$ git reset --hard 
HEAD is now at d20d80b9 final diff
~/src/gonum.org/v1/gonum [mod-debug]$ go test -mod=readonly ./...
go: updates to go.mod needed, disabled by -mod=readonly

@kortschak
Copy link
Contributor Author

This looks like it's related to #30345.

@bcmills
Copy link
Contributor

bcmills commented May 28, 2019

See #26850 (comment).

@bcmills
Copy link
Contributor

bcmills commented May 28, 2019

Duplicate of #26850

@bcmills bcmills marked this as a duplicate of #26850 May 28, 2019
@bcmills bcmills closed this as completed May 28, 2019
@kortschak
Copy link
Contributor Author

Given that this breaks builds in a way that has not been accounted for and has can only be worked around using a technique that has not been documented to behave in a way that will necessarily result in a sensible desired behaviour (and that you've suggested should break reasonable module configurations) would you please explain how to approach this if it is not going to be fixed.

I've dealt with occasional frustrations with the approaches taken by the core team in feature roll out and planning in the past, but nothing that comes close to how modules is being handled.

@golang golang locked and limited conversation to collaborators May 27, 2020
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge 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
Go1.13
Development

No branches or pull requests
3 participants
@kortschak @bcmills @gopherbot