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: build not respecting empty go mod files #35070

Open
gertcuykens opened this issue Oct 22, 2019 · 12 comments
Open

cmd/go: build not respecting empty go mod files #35070

gertcuykens opened this issue Oct 22, 2019 · 12 comments

Comments

@gertcuykens
Copy link
Contributor

@gertcuykens gertcuykens commented Oct 22, 2019

go version devel +8c6876e9a4 Thu Oct 17 22:27:31 2019 +0000 darwin/amd64

go env
go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/Users/gert/bin"
GOCACHE="/Users/gert/Library/Caches/go-build"
GOENV="/Users/gert/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/gert/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/gert/go/src/bitbucket.org/gertcuykens/lib/module/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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/dv/8tlwvjr91zjdyq4rk14lkkfm0000gn/T/go-build556715727=/tmp/go-build -gno-record-gcc-switches -fno-common"

Don't think it's desired behavior if you do go build by accident in a directory with no go files to automatically fill the empty go.mod file. Especially when a empty go.mod file was specifically designed to exclude that directory from the actual module. I know about -mod=readonly but when there are no go files it should make a exception and leave that file alone even without the -mod=readonly

> mkdir test
> cd test
> touch go.mod
> cat go.mod 
> go build
can't load package: package .: no Go files in ...
> cat go.mod 
module .../test

go 1.14

EDIT:
go doc also modifies a go.mod file in gopath

go doc github.com/gertcuykens/module/v5
@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Oct 22, 2019

So it looks like go build and other commands will initialize the module path in go.mod with the directory in GOPATH. So if you're in a subdirectory of GOPATH literally named .../test, you'll get a go.mod file like the one printed here.

Several parts of this seem wrong. go build should probably just fail with an error in this situation (as it does outside GOPATH), and we should check that subdirectory names are actually valid paths when initializing a module like this.

@jayconrod jayconrod added the NeedsFix label Oct 22, 2019
@jayconrod jayconrod added this to the Go1.14 milestone Oct 22, 2019
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Oct 22, 2019

This is somewhat related to #29433, in that we're filling in a go.mod file that we probably shouldn't touch.

See also #25272.

@ghostlandr

This comment has been minimized.

Copy link

@ghostlandr ghostlandr commented Nov 13, 2019

@bcmills @jayconrod if I'd like to work on this, how do I convey that? Do I just submit a PR and hope no one else has worked on this already? Haha. I didn't see this covered in the contribution guide!

@gertcuykens

This comment has been minimized.

Copy link
Contributor Author

@gertcuykens gertcuykens commented Nov 13, 2019

If somebody is working on it a CL gerrit link will be added automatically in this issue. But is it just me that most of the time get lost in the go codebase? Multiple times when I create a ticket somebody comes in and say I will fix this while I look at the codebase following the first few steps but then it get exponentially more difficult to follow the trail of interfaces and run out of breadcrumbs before I even reach a point where I can print a debug statement to get started?

@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Nov 13, 2019

@gdholtslander Just submit a PR or Gerrit CL. Make sure to include a regression test in src/cmd/go/testdata/script. GitHub doesn't let us assign issues outside the org, but commenting here is clear enough.

@gertcuykens That's definitely how I felt when I started working on cmd/go. It's admittedly not the most approachable code base.

@ghostlandr

This comment has been minimized.

Copy link

@ghostlandr ghostlandr commented Nov 13, 2019

@jayconrod so in the future if I'm interested in an issue, just make a comment stating my intention to start working on it? Thanks 👍

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Dec 5, 2019

@bcmills @jayconrod This is currently marked for Go 1.14; anything that needs to be done here for the release, or should this move to the backlog milestone?

@bcmills bcmills modified the milestones: Go1.14, Backlog Dec 5, 2019
@gopherbot gopherbot removed the NeedsFix label Dec 5, 2019
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Dec 5, 2019

I think we need to figure out the exact conditions in which go build should (or should not) fill in the go.mod file. This certainly won't happen for 1.14.

@ghostlandr

This comment has been minimized.

Copy link

@ghostlandr ghostlandr commented Dec 11, 2019

Sorry @bcmills, I haven't made time to investigate this yet. I have enjoyed reading through the go command testing utilities though!

@ghostlandr

This comment has been minimized.

Copy link

@ghostlandr ghostlandr commented Dec 16, 2019

I won't be able to take this on right now. If someone else wants to have a go at it please feel free.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 16, 2019

Change https://golang.org/cl/211597 mentions this issue: src/cmd/go/internal/modload: reject some bad module paths

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 19, 2019

Change https://golang.org/cl/212198 mentions this issue: module: allow unicode Letters in CheckImportPath

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.