golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.9k stars 17.65k forks source link

cmd/go: unclear diagnostic when fetching requirements for other versions of workspace modules #54680

Open mbrancato opened 2 years ago

mbrancato commented 2 years ago

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

$ go version
go version go1.18.5 darwin/amd64

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="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/mike/Library/Caches/go-build"
GOENV="/Users/mike/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/mike/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/mike/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/opt/go/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.18.5"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK="/Users/mike/Code/go/go.work"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/8l/0qhz2jl149bdfk71_vsx2t6r0000gn/T/go-build3928123174=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

./go.work

go 1.18

use (
    ./libs/foo
    ./projects/bar
)

./libs/foo/go.mod

module internal/foo

go 1.18

./projects/bar/go.mod

module internal/bar

go 1.18

require internal/foo v0.0.0

What did you expect to see?

Based on a number of expressive documents, the workspace proposal, and the workspace tutorial, I thought workspaces would be an alternative to the current usage of the replace directive in multiple modules in the same source tree.

What did you see instead?

While the module location usage of replace is changed for the workspace, the module name is enforced. This is not how replace works today in when specified in go.mod. There, it replaces the location and the module name. This results in the following error in the modules:

go: internal/foo@v0.0.0: malformed module path "internal/foo": missing dot in first path element

Again, this does not happen with the following in the ./projects/bar/go.mod.

module internal/bar

go 1.18

require internal/foo v0.0.0
replace internal/foo v0.0.0 => ../libs/foo

The workaround to this appears to be that a replace directive must also be in the go.work.

go 1.18

use (
    ./libs/foo
    ./projects/bar
)

replace internal/foo v0.0.0 => ./libs/foo

However, it does seem that we're having to specify the override of the module name multiple times as it is in both the go.work file and the module's own go.mod. It took a bit of work to find this (before I saw an example of replace used here) solution, and not really what I expected. But it does bring up the issue, if the module specifies its own name in go.mod, and the workspace knows how to access it, why do we need an explicit replace statement in the go.work to simply reassert the name of the module?

dr2chase commented 2 years ago

@bcmills @matloob ptal. From a quick read, I can't tell if this is an RFE or a documentation problem.

bcmills commented 2 years ago

module internal/foo should be accepted as valid in both replace directives and the go.mod files referenced by go.work files, so this seems like a bug.

bcmills commented 2 years ago

(That said, in general module paths without dots are reserved for the Go toolchain and standard library. I would suggest using module paths that begin with a domain name you control, even if you don't actually publish the modules to that domain.)

bcmills commented 2 years ago

I can reproduce the bug with the following script test (work_issue54680.txt):

go list -m all
stdout internal/foo
stdout internal/bar

-- go.work --
go 1.18

use (
    ./libs/foo
    ./projects/bar
)
-- libs/foo/go.mod --
module internal/foo

go 1.18
-- projects/bar/go.mod --
module internal/bar

go 1.18

require internal/foo v0.0.0

The test currently fails with the message reported by @mbrancato:

--- FAIL: TestScript (0.01s)
    --- FAIL: TestScript/work_issue54680 (0.02s)
        script_test.go:282:
            # (2022-08-29T16:17:48Z)
            > go list -m all
            [stderr]
            go: internal/foo@v0.0.0: malformed module path "internal/foo": missing dot in first path element
            [exit status 1]
            FAIL: testdata/script/work_issue54680.txt:1: unexpected command failure

FAIL
FAIL    cmd/go  0.151s
bcmills commented 2 years ago

Ah, no! The require internal/foo v0.0.0 refers to a different version of internal/foo from what's in the workspace, and because of the way module graph pruning works the dependencies of internal/foo v0.0.0 should be present in the module graph, regardless of what other version of internal/foo is actually selected in the workspace overall, because those dependencies would be needed by consumers of internal/bar outside of the workspace in order to run go test all.

The error message here is (awkwardly!) telling you that the go command can't fetch internal/foo@v0.0.0 because it can't fetch anything beginning with internal/ at all.

One solution in this case might be to leave the internal/foo dependency out of the go.mod files entirely, but then go mod tidy probably wouldn't work (#50750), but at least go mod tidy -e would. 🤔

mbrancato commented 2 years ago

(That said, in general module paths without dots are reserved for the Go toolchain and standard library. I would suggest using module paths that begin with a domain name you control, even if you don't actually publish the modules to that domain.)

I understand that Go wants this named differently. Giving a hostname brings up other issues about internal libraries that are not published to an external repository. Basically, it now means you need to set GOPRIVATE or GOPROXY env vars, and ideally I'd like to avoid the need for a script or something to set env vars for a build to happen. For example, if instead I change internal/foo to github.com/my-corp/internal/foo (which may actually be where it lives, and that repo is private, then now go is prompting for authentication to GitHub. And if I provide a fake domain, under certain narrow circumstances, that could lead to grabbing the wrong dependency. In general, I like how public packages reference github.com etc and just work, but it also seems a bit too opinionated around processes such as innersourcing that may or may not have formal releases published to private repositories (e.g. monorepo).

mbrancato commented 2 years ago

I just noticed that my mentioned workaround only works for building a project. Commands like go work sync and go mod tidy still fail with the same error - missing dot in first path element. Basically, I need to keep the replace directives in all the go.mod files in addition to the go.work file, so it actually isn't reducing any complexity here for this use of the replace directive.