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: ambiguous import path not detected when the colliding path is in the standard library #35270

Closed
gertcuykens opened this issue Oct 31, 2019 · 8 comments
Assignees
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@gertcuykens
Copy link
Contributor

Suggest in Go 1.14 we go one step further when GO111MODULE="on", outside and inside! GOPATH, module path must be specified to prevent shenanigans.

When trying to create a module crypto outside gopath you get

% go mod init
go: cannot determine module path for source directory /Users/gert/crypto (outside GOPATH, module path must be specified)

Perfect, but if you do that inside gopath

(force GO111MODULE="on")
% mkdir $GOPATH/src/crypto
% cd $GOPATH/src/crypto
% echo "package crypto" > c_test.go
% go mod init
% cat go.mod
module crypto
go 1.14
% go test -v
=== RUN   TestRC4OutOfBoundsWrite
--- PASS: TestRC4OutOfBoundsWrite (0.00s)
=== RUN   TestCTROutOfBoundsWrite
--- PASS: TestCTROutOfBoundsWrite (0.00s)
=== RUN   TestOFBOutOfBoundsWrite
--- PASS: TestOFBOutOfBoundsWrite (0.00s)
=== RUN   TestCFBEncryptOutOfBoundsWrite
--- PASS: TestCFBEncryptOutOfBoundsWrite (0.00s)
=== RUN   TestCFBDecryptOutOfBoundsWrite
--- PASS: TestCFBDecryptOutOfBoundsWrite (0.00s)
PASS
ok      crypto  0.049s

Not only is it allowed, you even launch the wrong test files from GOROOT

I understand this is done for backward compatible sake, but GOPATH must die and end up somewhere as a dot folder hidden only used for cach.

Furthermore I suggest to change error message

go: cannot determine module path for source directory /Users/gert/crypto (outside GOPATH, module path must be specified)

to

go: cannot determine module path for source directory /Users/gert/crypto (module path must be specified)

to stop promoting people to use GOPATH

(related to: #35070)

@bcmills
Copy link
Contributor

bcmills commented Nov 5, 2019

This has ~nothing to do with specifying module paths: it would be conceptually ok (if somewhat fragile) to create a module crypto provided that it does not define any packages that are also defined in GOPATH/src.

The problem here is that the existing ambiguous import error is not detecting the collision between the user's module and the standard library. That should be a much easier (and less invasive) fix.

CC @jayconrod

@bcmills bcmills changed the title cmd/go: when GO111MODULE="on" outside and inside! GOPATH, module path must be specified to prevent shenanigans cmd/go: ambiguous import path not detected when the colliding path is in the standard library Nov 5, 2019
@bcmills bcmills added modules NeedsFix The path to resolution is known, but the work has not been done. labels Nov 5, 2019
@bcmills bcmills added this to the Backlog milestone Nov 5, 2019
@bcmills
Copy link
Contributor

bcmills commented May 3, 2021

See also #26227.

@ianlancetaylor
Copy link
Contributor

@bcmills Is this likely to happen for 1.18? Thanks.

@bcmills bcmills modified the milestones: Go1.18, Go1.19 Nov 10, 2021
@bcmills
Copy link
Contributor

bcmills commented Nov 10, 2021

I'm not sure. It should be a fairly easy fix, but it doesn't need to hold up the release; moving to 1.19 for now, but I may move it back if the fix turns out to be small enough to go in during the freeze.

@gopherbot
Copy link

Change https://go.dev/cl/434095 mentions this issue: cmd/go: show error when mod name conflict with std lib path

@mvdan
Copy link
Member

mvdan commented Oct 26, 2022

@bcmills does the fix here also address #26227 or #34068? They all seem very similar.

romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
…n std

Fixes golang#35270

Change-Id: I5d2a04359702be6dc04affb867540091b926bc23
Reviewed-on: https://go-review.googlesource.com/c/go/+/434095
Run-TryBot: xie cui <523516579@qq.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
@bcmills
Copy link
Contributor

bcmills commented Nov 17, 2022

#34068 is in GOPATH mode, so the fix here doesn't help (this issue was for module mode).

I suspect that #26227 also still affects GOPATH mode as well, although I'm less sure about that one.

@nmiyake
Copy link
Contributor

nmiyake commented Feb 8, 2023

@bcmills wanted to flag that the code that merged to fix this issue (https://go.dev/cl/434095) introduced what I believe are regressions in behavior for vendor mode in Go 1.20:

#58417: cmd/go: "list -json" prints non-JSON output if vendor directory is present (Go 1.20 regression)
#58418: cmd/go: "list -json" does not populate "Dir" field for module in vendor directory (Go 1.20 regression)

I'm going to investigate further to try to understand what's going on, but as this issue doesn't mention vendoring, it seems likely that the change in behavior was an unintentional side effect of the fix for this issue.

It seems to me that, in the workflows I posted in my issue, the behavior before this change was merged was more correct, but if you could provide any clarity around that it would be much appreciated (I have tooling and workflows that depended on the previous behavior and it would be helpful to know if the new behavior is a bug where I can wait for a fix, or if the updated information/behavior is intentional and I need to figure out a different workaround).

@golang golang locked and limited conversation to collaborators Feb 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
Development

No branches or pull requests

7 participants