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: 'go test' does not reject a relative path spanning a module boundary #30992

Closed
FiloSottile opened this issue Mar 21, 2019 · 6 comments
Closed
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@FiloSottile
Copy link
Contributor

In golang.org/x/exp there's a golang.org/x/exp/notary submodule.

I was making changes in golang.org/x/exp/notary/internal/note, and I ran the tests as go test ./notary/internal/note. If it weren't for the fact that my cache was missing some modules, leading to a go: extracting golang.org/x/exp/notary message, I would not have known I was testing the upstream code downloaded from the Internet instead of the new code on my machine.

$ pwd
~/src/golang.org/x/exp

$ go test ./notary/internal/note
go: finding golang.org/x/mobile v0.0.0-20190312151609-d3739f865fa6
go: finding golang.org/x/sys v0.0.0-20190312061237-fead79001313
go: finding golang.org/x/image v0.0.0-20190227222117-0694c2d4d067
go: finding golang.org/x/tools v0.0.0-20190312151545-0bb0c0a6e846
go: finding golang.org/x/exp/notary v0.0.0-20190316020145-860388717186
go: finding github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802
go: finding golang.org/x/text v0.3.0
go: finding golang.org/x/crypto v0.0.0-20190313024323-a1f597ede03a
go: finding golang.org/x/net v0.0.0-20190311183353-d8887717615a
go: finding golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2
go: finding golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a
go: downloading golang.org/x/exp/notary v0.0.0-20190316020145-860388717186
go: extracting golang.org/x/exp/notary v0.0.0-20190316020145-860388717186
go: downloading golang.org/x/crypto v0.0.0-20190313024323-a1f597ede03a
go: extracting golang.org/x/crypto v0.0.0-20190313024323-a1f597ede03a
ok  	golang.org/x/exp/notary/internal/note	0.058s

$ cd notary
$ go test ./internal/note
--- FAIL: TestSurprise (0.00s)
    note_test.go:191: surprise!
FAIL
FAIL	golang.org/x/exp/notary/internal/note	0.049s

Even worse, you could imagine running go build or go install with the same relative path, and the changes on disk wouldn't be reflected, which sounds a lot like the most frustrating experiences I've had with some web development setups.

There is a solution to this which is to add a replace directive in the golang.org/x/exp go.mod pointing to ./notary, but I don't even know if an unused replace is legal or effective. And anyway, it feels like something most projects will only discover painfully.

(There was one other sign that something went wrong: golang.org/x/exp/notary showed up in the golang.org/x/exp go.mod file, but that would not have happened if it were already there. In that case it should really have had a replace, because otherwise a similar issue would happen with golang.org/x/exp using the unmodified dependency, but again, this is a painful default and it's not clear how one is supposed to learn about the solution.)

/cc @bcmills

@FiloSottile FiloSottile added this to the Go1.13 milestone Mar 21, 2019
@dmitshur dmitshur added the GoCommand cmd/go label Mar 21, 2019
@thepudds
Copy link
Contributor

thepudds commented Mar 22, 2019

Seems this should be an error? At least, that would be my personal expectation...

CC @jayconrod

@dmitshur
Copy link
Contributor

dmitshur commented Mar 22, 2019

https://golang.org/cmd/go/#hdr-The_main_module_and_the_build_list says:

The "main module" is the module containing the directory where the go command is run. The go command finds the module root by looking for a go.mod in the current directory, or else the current directory's parent directory, or else the parent's parent directory, and so on.

So, when you're in ~/src/golang.org/x/exp directory, go test <any arguments> using the golang.org/x/exp as the main module is consistent with the documented behavior.

It seems like the behavior is very unintuitive when the go command is run in one module, but but the package specified on the command line is outside of that module.

@jayconrod
Copy link
Contributor

I agree this is a bug and we should show an error for it.

File system paths (including relative paths) are allowed to reference directories in the main module and in other modules in the build list. Without a file system replace directive, you should only be allowed to reference directories in $GOPATH/pkg/mod (editors need to do this).

@jayconrod jayconrod self-assigned this Mar 22, 2019
@jayconrod jayconrod added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 22, 2019
@bcmills
Copy link
Contributor

bcmills commented Mar 27, 2019

Without a file system replace directive, you should only be allowed to reference directories in $GOPATH/pkg/mod (editors need to do this).

It's not the presence in GOPATH/pkg/mod that matters: rather, it is the difference between relative and absolute paths. Absolute paths can be anything in the build list. Relative paths are supposed to be confined to within the same module (the bug here).

@bcmills
Copy link
Contributor

bcmills commented Mar 27, 2019

I don't even know if an unused replace is legal or effective.

It is legal and has no effect. We are considering removing ineffective directives in go mod tidy; see #30516.

@bcmills bcmills changed the title cmd/go: using relative paths with nested modules is confusing cmd/go: 'go test' does not reject a relative path spanning a module boundary Mar 27, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/185345 mentions this issue: cmd/go: rationalize errors in internal/load and internal/modload

@golang golang locked and limited conversation to collaborators Feb 27, 2021
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
None yet
Development

No branches or pull requests

8 participants