cmd/go: "leading dot in path element" for previously-undiagnosed paths in go 1.13

[xyt0056]

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

$ go version
go version go1.13

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
GO111MODULE=""
GOARCH="amd64"
GOBIN="/Users/me/go/bin"
GOCACHE="/Users/me/Library/Caches/go-build"
GOENV="/Users/me/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY="none"
GONOSUMDB="code.corp.internal"
GOOS="darwin"
GOPATH="/Users/tanx/go-code"
GOPRIVATE="code.corp.internal"
GOPROXY="https://proxy.mycorp.com"
GOROOT="/private/var/tmp/_bazel_tanx/cde87e3334239cff91d2a561f734e9a6/external/go_sdk"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/private/var/tmp/_bazel_tanx/cde87e3334239cff91d2a561f734e9a6/external/go_sdk/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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/nn/f16l5j1j2f14ctsnkk305lf00000gp/T/go-build331063970=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

upgrade from go1.12.9 to go1.13 in bazel WORKSPACE
run go mod tidy

What did you expect to see?

finish successfully

What did you see instead?

code.corp.internal/repo1/a tested by
	code.corp.internal/repo1/a.test imports
	code.corp.internal/somedependency/a tested by <- external dependency
	code.corp.internal/somedependency/a.test imports
	code.corp.internal/someotherdependency: malformed module path "code.corp.internal/someotherdependency/.gen/somepath...": leading dot in path element

our monorepo setup

gopath/
    src/
        code.corp.internal/
            go.mod
            go.sum
            repo1/
            repo2/

Does go 1.13 scan code of dependency's import path and check for dots as well?

[ianlancetaylor]

CC @bcmills @jayconrod

[bcmills]

Does go 1.13 scan code of dependency's import path and check for dots as well?

Leading dots have been disallowed in module and package paths from the very start of module mode, in Go 1.11. (See https://golang.org/cl/124378.)

The major change in go mod tidy in Go 1.13 is that it now reports errors. (See #27063.)

Is there reason to believe that go mod tidy ever worked successfully for this dependency?

[xyt0056]

in go.1.12 go mod tidy finishes successfully without any explicit errors.
I think importing .gen is such a common case in practice. Do we have any workaround to make go treat it as folder instead of modules? like
import "src/code.corp.internal/..../.gen/..."?

[jayconrod]

Is code.corp.internal/someotherdependency/.gen part of a module name, or is .gen just a directory within a module? As @bcmills said, leading dots are not allowed in module paths, but I think they're allowed in package paths.

We made several changes to the code that resolves package paths to modules, and the error checking is more aggressive than it used to be. I wonder if we're reporting an error there instead of simply disqualifying code.corp.internal/someotherdependency/.gen as a possible module path?

[bcmills]

leading dots are not allowed in module paths, but I think they're allowed in package paths.

That's not the behavior of module.CheckImportPath as of https://golang.org/cl/124378, but it's certainly possible that we're not calling CheckImportPath consistently for all import paths.

I believe that part of the purpose there is to prevent the creation of “hidden packages” (since dotfiles are hidden in many platforms) in the user's module cache, GOPATH/src, and (especially) vendor directory. That way, a module's dependencies can be audited, for example, by (visually) walking the packages in the vendor tree.

[xyt0056]

Is code.corp.internal/someotherdependency/.gen part of a module name, or is .gen just a directory within a module?

It's a directory in the dependency and the .gen folder is checked in VCS. Is package path completely deprecated in GO111MODULE mode? How does the code "resolve package paths to modules"?

[bcmills]

If it's a directory within the dependency, then go mod tidy should not be trying to resolve a module for it in the first place.

Do go list all, go build ./..., and/or go test ./... report the same errors?

[xyt0056]

go list all returns fine

we use bazel instead of go build

[bcmills]

go mod tidy is intended for use with the go command for builds. bazel intentionally supports code layouts that the go command does not, so it is not surprising that go mod tidy produces an error for those layouts.

If go build can build the program, then go mod tidy should certainly be able to tidy it, but if go build cannot build it then we do not expect go mod tidy to work either.

[xyt0056]

If it's a directory within the dependency, then go mod tidy should not be trying to resolve a module for it in the first place.

Ok I just tried. go build repo1 shoots out error same as #35048. Then I fixed with instructions there by

1. moving generated code from vendor/thriftrw to src/code.corp.internal/
2. replacing import path of thriftrw/... to code.corp.internal/thriftrw

now go build succeeds. But go mod tidy still errors

code.corp.internal/repo1/project1 imports
	code.corp.internal/some/dependency tested by
	code.corp.internal/some/dependency.test imports
	code.corp.internal/some/dependency/.gen/config: malformed module path "code.corp.internal/some/dependency/.gen/config": leading dot in path element

[bcmills]

Does go test -c code.corp.internal/some/dependency succeed?

Bear in mind that go mod tidy scans the full transitive dependencies of the packages within your module, whereas go build only scans the non-test dependencies.

[xytan0056]

Does go test -c code.corp.internal/some/dependency succeed?

I got

can't load package: package code.corp.internal/some/dependency: module code.corp.internal/some/dependency@latest (vx.x.x) found, but does not contain package code.corp.internal/some/dependency

[xytan0056]

Let's say we have an internal module that has import xxx/.gen/xxx. Is this not allowed at all in go1.13, if we depend on this module? So we have to fix the internal module's import?

I wonder if this is true for all the other github projects, that they're all enforced to not have .gen import

[bcmills]

@xytan0056, again, such a path has never been allowed in module mode. Is there reason to believe that this was ever a common pattern for packages buildable with the go command?

[xyt0056]

such a path has never been allowed in module mode. Is there reason to believe that this was ever a common pattern for packages buildable with the go command?

@bcmills I understand. Surprised that it's disallowed all this time. It works when we were in package mode. I'm trying to make it work in module mode.
I'm wondering if this requirement is enforced for all dependent modules as well. Or just the imports in main module? Does it mean if I depend on any external module that is not module-aware and has .gen in its import statements (I'm pretty sure there're some in github), I'll see this error in my project?

If it's a directory within the dependency, then go mod tidy should not be trying to resolve a module for it in the first place.

I'm confused. .gen folder is part of a module downloaded. and it has code importing this .gen. Do you mean this is a go toolchain issue?

Is it possibly because I have the same module prefix as my dependency?
repo layout

$GOPATH
--pkg  <--- dependency also imports "code.corp.internal/somepath/.gen/somepath"
--src
----code.corp.internal
------repo1
------repo2
------go.mod  <--- module name "code.corp.internal"

[xytan0056]

Update: It looks like go 1.13 mod tidy starts to check imports format in dependency's test files.

example

• https://github.com/xytan0056/depwithgen
  has generated code .gen checked in. .gen is imported in a test (api_test.go)

• https://github.com/xytan0056/depwithoutgen
  does not have .gen checked in.

1. https://github.com/xytan0056/gomodexp
   is the happy case where it imports both depwithgen and depwithgen/.gen.
   go mod tidy succeeds.

2. https://github.com/xytan0056/gomodnogen
   simply imports depwithoutgen/api, which doesn't have .gen checked in. Although go build/run/testsucceeds, go mod tidy fails with

$ go1.13 mod tidy
go: finding github.com/xytan0056/depwithoutgen latest
github.com/xytan0056/gomodnogen imports
	github.com/xytan0056/depwithoutgen/api imports
	github.com/xytan0056/depwithoutgen/.gen/client: malformed module path "github.com/xytan0056/depwithoutgen/.gen/client": leading dot in path element

3. github.com/xytan0056/gomodnorequire
   imports depwithgen's .gen in its test , same as test in gomodexp(happy case). But doesn't require it in go.mod.
   Although, go build/run succeeds, go test and go mod tidy fail with

$ go1.13 mod tidy
go: finding github.com/xytan0056/depwithgen latest
github.com/xytan0056/gomodnorequire tested by
	github.com/xytan0056/gomodnorequire.test imports
	github.com/xytan0056/depwithgen/.gen/client: malformed module path "github.com/xytan0056/depwithgen/.gen/client": leading dot in path element

All of the above succeeds on go1.12 with GO111MODULE=on

What is the recommendation for generated code with leading dots in general?
What is the recommendation for generated code used in dependency's test files?

[jayconrod]

@xytan0056 Thanks for investigating and publishing those repositories.

It looks like go1.13 does not check import paths all the time. It reports this error when looking up a module for an imported package that is not provided by any module. In this case, there is no module that provides github.com/xytan0056/depwithgen/.gen/client, and go1.13 verifies the module path before requesting a potentially invalid module from a proxy.

I think this error is correct, and actually we ought to be reporting it in more changes. However, I don't think we should make a change like that without understanding how many people that will break, especially at this point in the freeze. We could improve the error message though. It should say malformed import path or malformed package path instead of malformed module path.

What is the recommendation for generated code with leading dots in general?
What is the recommendation for generated code used in dependency's test files?

Avoid leading or trailing dots in path names. module.CheckImportPath is the code that should check this.

Generated code is generally fine. It should just be checked into a directory that follows these rules.

A valid import path consists of one or more valid path elements separated by slashes (U+002F). (It must not begin with nor end in a slash.)

A valid path element is a non-empty string made up of ASCII letters, ASCII digits, and limited ASCII punctuation: + - . _ and ~. It must not begin or end with a dot (U+002E), nor contain two dots in a row.

[ianlancetaylor]

@jayconrod Is there anything to do for this issue for the 1.14 release? Any doc updates?

[jayconrod]

@ianlancetaylor I think we just need to clarify the wording in an error message. Doesn't need to go into the release notes.

[ianlancetaylor]

Moving milestone to backlog.

[liasece]

Our project does not have this problem with go1.15.7 darwin/amd64, but it occurs with go1.16beta1 darwin/arm64.
@ianlancetaylor @dmitshur

[bcmills]

@liasece, please file a separate issue with steps to reproduce (but first see the changes to defaults described at https://tip.golang.org/doc/go1.16#go-command).

[perj]

Unfortunate decision, that I also noticed now in go 1.16 rc1. Oh well, I'll learn to live with it I guess.

[bcmills]

Barring any surprising new information, this behavior of the go command is unlikely to change. See #43985 (comment) for more detail.

[gopherbot]

Change https://golang.org/cl/297530 mentions this issue: cmd: upgrade golang.org/x/mod to relax import path check

[gopherbot]

Change https://golang.org/cl/297634 mentions this issue: cmd/go: test remote lookup of packages with leading dots in path elements

[gopherbot]

Change https://golang.org/cl/297912 mentions this issue: [release-branch.go1.16] cmd: upgrade golang.org/x/mod to relax import path check