Skip to content

Commit

Permalink
cmd/go: emit an error for extraneous files in GOROOT/src in module mode
Browse files Browse the repository at this point in the history
If there's a go file immediately in GOROOT/src, it was probably
accidentally added by the user. Since that package shouldn't
exist, return an error if a user tries to list it. We're only making
this change for GOPATH mode because we don't want to break cases
where users have been doing this historically, but want to fix
this case for the future.

This also leaves open the weird cases where files are placed directly
in vendor directories.

Fixes #36587

Change-Id: I9738e47b1e89fd5048cbb8dd28e44648834b8ea7
Reviewed-on: https://go-review.googlesource.com/c/go/+/216381
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
  • Loading branch information
matloob committed Feb 25, 2020
1 parent f0ee49b commit 0652c80
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 0 deletions.
2 changes: 2 additions & 0 deletions src/cmd/go/internal/modload/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ func Import(path string) (m module.Version, dir string, err error) {
}
dir := filepath.Join(cfg.GOROOT, "src", path)
return module.Version{}, dir, nil
} else if pathIsStd && path == cfg.GOROOTsrc {
return module.Version{}, dir, errors.New("directory should not directly contain source files")
}

// -mod=vendor is special.
Expand Down
6 changes: 6 additions & 0 deletions src/cmd/go/internal/modload/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ func ImportPathsQuiet(patterns []string, tags map[string]bool) []*search.Match {
// It's not strictly necessary but helpful to keep the checks.
if modRoot != "" && dir == modRoot {
pkg = targetPrefix
if modRoot == cfg.GOROOTsrc {
// A package in GOROOT/src would have an empty path.
// Keep the path as cfg.GOROOTsrc. We'll report an error in Import.
// See golang.org/issue/36587.
pkg = modRoot
}
} else if modRoot != "" && strings.HasPrefix(dir, modRoot+string(filepath.Separator)) && !strings.Contains(dir[len(modRoot):], "@") {
suffix := filepath.ToSlash(dir[len(modRoot):])
if strings.HasPrefix(suffix, "/vendor/") {
Expand Down
46 changes: 46 additions & 0 deletions src/cmd/go/testdata/script/list_gofile_in_goroot.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Return an error if the user tries to list a go source file directly in $GOROOT/src.
# Tests golang.org/issue/36587

mkdir $WORK/fakegoroot/src
mkdir $WORK/fakegopath/src

env GOROOT=$WORK/fakegoroot
env GOPATH=$WORK/fakegopath

cp go.mod $GOROOT/src/go.mod
cp foo.go $GOROOT/src/foo.go

go env GOROOT
stdout $WORK(/|\\)fakegoroot

# switch to GOROOT/src
cd $GOROOT/src

# GO111MODULE=on,GOROOT
env GO111MODULE=on
! go list ./...
stderr 'directory should not directly contain source files'
go list -e .
go list -f '{{if .Error}}{{.Error.Err}}{{end}}' -e ./...
stdout 'directory should not directly contain source files'

# GO111MODULE=off,GOROOT
env GO111MODULE=off
go list ./...
[!windows] stdout _$WORK/fakegoroot/src
[windows] stdout fakegoroot/src # On windows the ":" in the volume name is mangled

# switch to GOPATH/src
cp $WORK/gopath/src/foo.go $GOPATH/src/foo.go
cd $GOPATH/src

# GO111MODULE=off,GOPATH
env GO111MODULE=off
go list ./...

-- go.mod --
module g

go 1.14
-- foo.go --
package foo

0 comments on commit 0652c80

Please sign in to comment.