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: unclear diagnostic when fetching requirements for other versions of workspace modules #54680

Open
mbrancato opened this issue Aug 25, 2022 · 7 comments
Labels
GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@mbrancato
Copy link

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
Copy link
Contributor

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

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 26, 2022
@seankhliao seankhliao added this to the Unplanned milestone Aug 27, 2022
@bcmills
Copy link
Contributor

bcmills commented Aug 29, 2022

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
Copy link
Contributor

bcmills commented Aug 29, 2022

(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
Copy link
Contributor

bcmills commented Aug 29, 2022

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
Copy link
Contributor

bcmills commented Aug 29, 2022

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
Copy link
Author

(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).

@bcmills bcmills changed the title cmd/go: workspace use does not ignore local module name as replace does cmd/go: unclear diagnostic when fetching requirements for other versions of workspace modules Sep 6, 2022
@bcmills bcmills modified the milestones: Unplanned, Backlog Sep 6, 2022
@mbrancato
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants