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: add tests describing 'go run' behavior with GO111MODULE=on #26709

Closed
myitcv opened this issue Jul 31, 2018 · 10 comments
Closed

cmd/go: add tests describing 'go run' behavior with GO111MODULE=on #26709

myitcv opened this issue Jul 31, 2018 · 10 comments
Labels
FrozenDueToAge modules NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@myitcv
Copy link
Member

myitcv commented Jul 31, 2018

Please answer these questions before submitting your issue. Thanks!

This might be a duplicate of/covered by the solution to #26639, but I wanted to create a separate issue for go run because that could be considered to be a particularly special case amongst the various commands.

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

go version devel +b294fe9b0a Tue Jul 31 02:04:53 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

n/a: testing with tip.

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

Not available directly because of #26639, but:

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPROXY=""
GORACE=""
GOROOT="/home/myitcv/gos"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/tmp/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-build675792232=/tmp/go-build -gno-record-gcc-switches"

What did you do?

export GO111MODULE=on
cd $(mktemp -d)
cat <<EOD > run.go
package main

import "fmt"

func main() {
        fmt.Println("Hello, world!")
}
EOD
go run run.go

What did you expect to see?

If my assumption that a go.mod shouldn't be required is correct, zero exit code and:

Hello, world!

What did you see instead?

Non-zero exit code and:

go: cannot find main module root; see 'go help modules'

/cc @rsc @bcmills

@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. modules labels Jul 31, 2018
@myitcv myitcv added this to the Go1.11 milestone Jul 31, 2018
@myitcv
Copy link
Member Author

myitcv commented Aug 1, 2018

Problem remains in:

go version devel +6b9c782f9f Wed Aug 1 00:57:00 2018 +0000 linux/amd64

@bcmills
Copy link
Contributor

bcmills commented Aug 1, 2018

I think this is intended behavior.

If run.go only imports standard-library packages, then I guess you could argue that it shouldn't require a go.mod file, but certainly if it imports anything from other modules we need to know what versions of those modules to use.

It seems awkward to have to look at the set of imports to determine whether we need a module definition: I'd rather we require it unconditionally.

@bcmills bcmills added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 1, 2018
@myitcv
Copy link
Member Author

myitcv commented Aug 1, 2018

Actually, I don't know why I thought this should work. You're quite right; if I can create an arbitrary .go file, I can trivially create an arbitrary go.mod file in the same directory, for example.

I'd be happy for this to be closed unless you think there's actually something to decide?

@bcmills bcmills closed this as completed Aug 1, 2018
@myitcv
Copy link
Member Author

myitcv commented Aug 1, 2018

Actually @bcmills, I think there is some sort of issue here, even when I do create a go.mod file.

export GO111MODULE=on
sd=$(mktemp -d)
cd $sd
go mod init main
cat <<EOD > run.go
package main

import "fmt"

func main() {
        fmt.Println("Hello, world!")
}
EOD
go run run.go

succeeds and gives:

Hello, world!

Equally:

cd $(mktemp -d)
go run $sd/run.go

also gives:

Hello, world!

But then if I:

cd $(mktemp -d)
export GOPATH=$PWD
go run $sd/run.go

I get:

go: cannot find main module; see 'go help modules'

So it seems, with GO111MODULE=on, go run with a working directory within GOPATH doesn't work, whereas a working directory outside of GOPATH is fine (so long as I have a go.mod file).

Or do you think this is a duplicate of #26722?

@bcmills
Copy link
Contributor

bcmills commented Aug 1, 2018

I'm not sure, but we can certainly add some tests to cmd/go/testdata/script to describe what happens for that situation.

@bcmills bcmills reopened this Aug 1, 2018
@bcmills bcmills changed the title cmd/go: go run requires go.mod in GO111MODULE=on context cmd/go: add tests describing 'go run' behavior with GO111MODULE=on Aug 1, 2018
@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Aug 1, 2018
@myitcv
Copy link
Member Author

myitcv commented Aug 6, 2018

Confirmed that the repro in #26709 (comment) is solved by https://go-review.googlesource.com/c/go/+/127796 (the fix for #26722)

Will leave this open for, as @bcmills suggests, adding such a test case.

@natefinch
Copy link
Contributor

Please don't make go run require a go.mod. if anything, it should generate a temporary go.mod automatically and then throw it away.

@bcmills
Copy link
Contributor

bcmills commented Sep 12, 2018

Please don't make go run require a go.mod.

I would expect go run pkg@version to work with or without one (using the same mechanism as for #24250).

It's less clear to me whether go run in general should work outside a module. If the solution to #24250 is some sort of per-user fallback go.mod, then go run should clearly use that, but if we resolve #24250 in some other way then I'm not so confident. (So maybe that's an argument for the per-user fallback approach?)

@bcmills bcmills self-assigned this Oct 24, 2018
@rsc
Copy link
Contributor

rsc commented Nov 28, 2018

@bcmills, is this fixed by the "in-memory empty go.mod" behavior we added?

@bcmills
Copy link
Contributor

bcmills commented Dec 4, 2018

Yes, go run is now tested (and works outside of a module).

@bcmills bcmills closed this as completed Dec 4, 2018
@golang golang locked and limited conversation to collaborators Dec 4, 2019
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

5 participants