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: using a module path without dot fails to build after toolchain selection [1.20 backport] #61873

Closed
dmitshur opened this issue Aug 8, 2023 · 10 comments
Assignees
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge GoCommand cmd/go release-blocker
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Aug 8, 2023

Similar to #61871, but:

$ cd $(mktemp -d)    
$ go mod init example/m
go: creating new go.mod: module example/m
$ go get go@1.20
go: downgraded go 1.21.0 => 1.20
$ echo "package main; func main() {}" > main.go

$ GOTOOLCHAIN=go1.20.7 go run .
ambiguous import: found package example/m in multiple modules:
	example/m (/tmp/LpbXADzD)
	 (/Users/gopher/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.20.7.darwin-arm64/src/example/m)
$ echo $?
1

It works fine without toolchain selection, or if a dot is used in first path element.

I noticed because I usually type something short (i.e., without a dot in first path element) when running a throwaway program locally. I know import paths without a dot are reserved for the standard library and shouldn't be used, but example/... and test/... are reserved for user progams per #37641.

CC @bcmills, @matloob, @rsc.

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Aug 8, 2023
@dmitshur dmitshur added this to the Backlog milestone Aug 8, 2023
@bcmills bcmills self-assigned this Aug 9, 2023
@bcmills bcmills modified the milestones: Backlog, Go1.22 Aug 9, 2023
@bcmills
Copy link
Contributor

bcmills commented Aug 9, 2023

@gopherbot, please backport to Go 1.21. This is a very confusing failure mode.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #61896 (for 1.21).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@bcmills
Copy link
Contributor

bcmills commented Aug 10, 2023

I think this is actually a bug in go1.20.7, with a workaround for Go 1.21 in https://go.dev/cl/504119 and a proper fix pending in https://go.dev/cl/504063.

@bcmills
Copy link
Contributor

bcmills commented Aug 10, 2023

If so, that should imply that if you install go1.20.7 locally (so that it can be found via $PATH instead of in the module cache), then you will not observe this odd failure.

It should also not be present if you set GOTOOLCHAIN to, say, go1.21rc3.

@bcmills bcmills modified the milestones: Go1.22, Go1.20.8 Aug 10, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/518415 mentions this issue: cmd/go: do not index std as a module in modcache

@dmitshur
Copy link
Contributor Author

dmitshur commented Aug 10, 2023

should imply that if you install go1.20.7 locally (so that it can be found via $PATH instead of in the module cache), then you will not observe this odd failure.

It should also not be present if you set GOTOOLCHAIN to, say, go1.21rc3.

Both statements are true based on a quick local run.

I think it would be fine to close this issue as fixed in the Go1.22 or Go1.21 milestone, and make a new one for Go 1.20 backport. But since it's being repurposed to be a backport issue directly, I'll update the title and labels accordingly.

@dmitshur dmitshur added CherryPickCandidate Used during the release process for point releases and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 10, 2023
@dmitshur dmitshur changed the title cmd/go: using a module path without dot fails to build after toolchain selection cmd/go: using a module path without dot fails to build after toolchain selection [1.21 backport] Aug 10, 2023
@dmitshur dmitshur changed the title cmd/go: using a module path without dot fails to build after toolchain selection [1.21 backport] cmd/go: using a module path without dot fails to build after toolchain selection [1.20 backport] Aug 10, 2023
@bcmills bcmills added the CherryPickApproved Used during the release process for point releases label Aug 11, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Aug 11, 2023
gopherbot pushed a commit that referenced this issue Aug 14, 2023
We do not index std as a whole module ever.

When working in the main Go repo, files in package change often,
so we don't want to pay the cost of reindexing all of std when what
we really need is just to reindex strings. Per-package indexing
works better for that case.

When using a released Go toolchain, we don't have to worry about
the whole module changing, but if we switch to whole-module indexing
at that point, we have the potential for bugs that only happen in
released toolchains. Probably not worth the risk.

For similar reasons, we don't index the current work module as
a whole module (individual packages are changing), so we use the heuristic
that we only do whole-module indexing in the module cache.

The new toolchain modules live in the module cache, though, and
our heuristic was causing whole-module indexing for them.
As predicted, enabling whole-module indexing for std when it's
completely untested does in fact lead to bugs (a very minor one).

This CL turns off whole-module indexing for std even when it is
in the module cache, to bring toolchain module behavior back in
line with the other ways to run toolchains.

Updates #57001.
For #61873.

Change-Id: I5012dc713f566846eb4b2848facc7f75bc956eb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/504119
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
(cherry picked from commit a7b1793)
Reviewed-on: https://go-review.googlesource.com/c/go/+/518415
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
@gopherbot
Copy link
Contributor

Closed by merging 1a91bb9 to release-branch.go1.20.

@bcmills
Copy link
Contributor

bcmills commented Aug 14, 2023

This is fixed as far as Go 1.20. @dmitshur, I think there is still one final release planned for Go 1.19 — do you think this is worth fixing on that branch, given that Go 1.19 would normally be past its support window?

@dmitshur
Copy link
Contributor Author

dmitshur commented Aug 14, 2023

@bcmills I tested out the original repro steps with Go 1.19 as the "target toolchain" instead of 1.20, and it's hard for me to tell if it's affected by the same bug as described in this issue or a different one. It doesn't work either, but the error is different:

$ cd $(mktemp -d)
$ go mod init example/m
go: creating new go.mod: module example/m
$ go get go@1.19
go: downgraded go 1.21.0 => 1.19
$ echo "package main; func main() {}" > main.go
$ GOTOOLCHAIN=go1.19.12 go run .
import "": import of unknown directory
$ echo $? 
1

There's some connection to #61871, but that issue is about error text in invalid programs, while this issue was about valid programs not working. (But it's possible their root cause is the same.)

It's hard for me to say without more information, so leaving the decision to you. In general I think it's better not to break cmd/go in Go 1.19.13 at this point, even if it means some UX inconvenience (i.e., needing to include a "." in the import path of short example or test programs when trying to use them via toolchain selection). On the other hand if you're very confident in the safety of the fix and the same CL will work despite the error being seemingly different, you do have that opportunity to include the fix.

@bcmills
Copy link
Contributor

bcmills commented Aug 15, 2023

My inclination is that it's not worth the risk to mess with the final 1.19 release: as a workaround, folks can always install it in $PATH to prevent it from being loaded from the module cache.

vincentbernat added a commit to vincentbernat/dependabot-core that referenced this issue Sep 6, 2023
Due to a bug in Go 1.20 (see golang/go#61873),
when importing a module without a dot in it, we get:

```
 18:13 ❱ GOTOOLCHAIN="go1.20+auto" go get
akvorado: ambiguous import: found package akvorado in multiple modules:
        akvorado (/home/bernat/code/free/akvorado)
         (/home/bernat/src/gocode/pkg/mod/golang.org/toolchain@v0.0.1-go1.20.linux-amd64/src/akvorado)
```

This issue was fixed in Go 1.20.8 which was just released. By using
`go1.20.8` as a toolchain for anything <= 1.20, we still cover the use
case introduced in dependabot#7884, but we also avoid this bug. `go1.20.8` is a
valid toolchain. For Go 1.20, only `go1.20` is a valid version for `go`
directive in `go.mod`.
jakecoffman pushed a commit to dependabot/dependabot-core that referenced this issue Sep 6, 2023
Due to a bug in Go 1.20 (see golang/go#61873),
when importing a module without a dot in it, we get:

```
 18:13 ❱ GOTOOLCHAIN="go1.20+auto" go get
akvorado: ambiguous import: found package akvorado in multiple modules:
        akvorado (/home/bernat/code/free/akvorado)
         (/home/bernat/src/gocode/pkg/mod/golang.org/toolchain@v0.0.1-go1.20.linux-amd64/src/akvorado)
```

This issue was fixed in Go 1.20.8 which was just released. By using
`go1.20.8` as a toolchain for anything <= 1.20, we still cover the use
case introduced in #7884, but we also avoid this bug. `go1.20.8` is a
valid toolchain. For Go 1.20, only `go1.20` is a valid version for `go`
directive in `go.mod`.
@golang golang locked and limited conversation to collaborators Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge GoCommand cmd/go release-blocker
Projects
None yet
Development

No branches or pull requests

4 participants