uber-go / dig

A reflection based dependency injection toolkit for Go.
https://go.uber.org/dig
MIT License
3.91k stars 206 forks source link

Does not work with go.mod / vgo #203

Closed albenik closed 5 years ago

albenik commented 6 years ago

Just import go.uber.org/dig to any go.mod enabled project and then run vgo mod -sync

import "go.uber.org/dig" ->
import "go.uber.org/dig/internal/digreflect" ->
test ->
import "go.uber.org/dig/internal/digreflect/tests/myrepository.git" ->
import "mydependency": cannot find module providing package mydependency
yuhui-lin commented 6 years ago

I run into the same issue. By running go1.11beta3 mod tidy, it got

    test ->
    import "go.uber.org/yarpc/transport/tchannel" ->
    test ->
    import "go.uber.org/yarpc/x/yarpctest" ->
    import "go.uber.org/yarpc/transport/grpc" ->
    test ->
    import "go.uber.org/yarpc/internal/examples/protobuf/examplepb" ->
    import "go.uber.org/fx" ->
    import "go.uber.org/dig" ->
    import "go.uber.org/dig/internal/digreflect" ->
    test ->
    import "go.uber.org/dig/internal/digreflect/tests/myrepository.git" ->
        import "mydependency": cannot find module providing package mydependency
glibsm commented 6 years ago

Strange. mydependency is included in the source code: https://github.com/uber-go/dig/tree/master/internal/digreflect/tests/myrepository.git/vendor/mydependency

The one thing that may be throwing go mod off is the fact that it's located in a nested vendor directory (internal/digreflect/tests/myrepository.git/vendor) and not in a root of the repository.

I haven't looked into mod much yet, I'm currently defaulting to dep, so it is not clear if this is intended behavior or something not yet implemented for mod. Does anyone know?

bufdev commented 6 years ago

Just came here to file same issue. Interesting note: this only happens with vgo, and not with go 1.11rc1.

Simple reproduction:

package main

import "go.uber.org/dig"

var _ = dig.Name

func main() {}
$ cd test
$ vgo mod init github.com/uber/test
go: creating new go.mod: module github.com/uber/test
$ vgo mod tidy
go: finding github.com/stretchr/testify/assert latest
go: finding github.com/stretchr/testify/require latest
go: import "github.com/uber/test" ->
    import "go.uber.org/dig" ->
    import "go.uber.org/dig/internal/digreflect" ->
    test ->
    import "go.uber.org/dig/internal/digreflect/tests/myrepository.git" ->
    import "mydependency": cannot find module providing package mydependency
$ cd test
$ go mod init github.com/uber/test
go: creating new go.mod: module github.com/uber/test
$ go mod tidy
go: finding github.com/stretchr/testify/assert latest
go: finding github.com/stretchr/testify/require latest
go: finding github.com/pmezard/go-difflib/difflib latest
go: finding github.com/davecgh/go-spew/spew latest

This might be OK if it's "fixed" in go 1.11rc1, thoughts? @akshayjshah @abhinav might have more insight.

bufdev commented 6 years ago

...and I lied, it's not fixed. I'm working on doing this for Prototool, I use go instead of vgo with export GO111MODULE=on, and then run into the same problem:

$ go test all
../../../../pkg/mod/go.uber.org/dig@v1.4.0/internal/digreflect/tests/myrepository.git/vendor.go:23:8: unknown import path "mydependency": cannot find module providing package mydependency
can't load package: package github.com/uber/prototool: unknown import path "github.com/uber/prototool": cannot find package
can't load package: ../../../../pkg/mod/go.uber.org/dig@v1.4.0/internal/digreflect/tests/myrepository.git/vendor.go:23:8: unknown import path "mydependency": cannot find module providing package mydependency

Work so far at https://github.com/uber/prototool/pull/197

https://github.com/golang/go/wiki/Modules

bufdev commented 6 years ago

Last comment: I must be doing something wrong, I'm having issues with go modules in a lot of places, someone with more vgo experience might have some good advice. I'm sure there's some advice for this.

# for prototool vgo branch
$ go build ./...
# github.com/cpuguy83/go-md2man/md2man
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/md2man.go:11:16: undefined: blackfriday.EXTENSION_NO_INTRA_EMPHASIS
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/md2man.go:12:16: undefined: blackfriday.EXTENSION_TABLES
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/md2man.go:13:16: undefined: blackfriday.EXTENSION_FENCED_CODE
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/md2man.go:14:16: undefined: blackfriday.EXTENSION_AUTOLINK
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/md2man.go:15:16: undefined: blackfriday.EXTENSION_SPACE_HEADERS
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/md2man.go:16:16: undefined: blackfriday.EXTENSION_FOOTNOTES
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/md2man.go:17:16: undefined: blackfriday.EXTENSION_TITLEBLOCK
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/md2man.go:19:29: too many arguments to conversion to blackfriday.Markdown: blackfriday.Markdown(doc, renderer, extensions)
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/roff.go:19:9: cannot use roffRenderer literal (type *roffRenderer) as type blackfriday.Renderer in return argument:
    *roffRenderer does not implement blackfriday.Renderer (missing RenderFooter method)
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/roff.go:102:11: undefined: blackfriday.LIST_TYPE_ORDERED
../../../../pkg/mod/github.com/cpuguy83/go-md2man@v1.0.8/md2man/roff.go:102:11: too many errors
glibsm commented 6 years ago

Cloned the vgo repo locally to try to find exactly what problem is happening. Found the problem: https://github.com/golang/vgo/blob/master/vendor/cmd/go/internal/get/vcs.go#L770

vgo assumes all packages must contain at least one dot to denote hostname :|

Will try to put up a diff to modify the testing setup because right now the test package is just mydependency and should be something like foo.bar/mydependency

glibsm commented 6 years ago

Hm. Maybe the fix is not as straight forward as I thought. After tucking mydependency in a tld, now it tries to fetch it (since it ignores that vendor directory, probably?)

$ go mod tidy -v
... <snip> ...
unrecognized import path "foo.bar/mydependency" (https fetch: Get https://foo.bar/mydependency?go-get=1: dial tcp: lookup foo.bar: no such host)
Fetching https://foo.bar?go-get=1
https fetch failed: Get https://foo.bar?go-get=1: dial tcp: lookup foo.bar: no such host
unrecognized import path "foo.bar" (https fetch: Get https://foo.bar?go-get=1: dial tcp: lookup foo.bar: no such host)
bufdev commented 6 years ago

The vcs.go file I think is old/just part of github.com/golang/go right? Not sure if that's relevant.

glibsm commented 6 years ago

Not sure what you mean by "not relevant". I grabbed the latest vgo and found the particular error in it's source that corresponds with dig not working.

unrecognized import path "mydependency" (import path does not begin with hostname)

bufdev commented 6 years ago

Was just saying it might not be related to the cause, there might be something special going on with vgo.

Best thing I would recommend is to actually use go1.11rc1 with export GO111MODULE=on to test this as I think that's where new stuff is going into.

bufdev commented 6 years ago

brew reinstall go --HEAD will also get you where you want with that (and export GO111MODULE=on)

glibsm commented 6 years ago

That vgo repo vendors the latest go command. Brew installing is not really desired here since it's not easy to dig into the source and insert some "debugging" (go mod swallows original errors as it's not yet able to distinguish between the many failure cases)

abhinav commented 6 years ago

I was able to repro this in the dig repo with,

$ export GO111MODULE=on
$ cd dig
$ go mod init
$ go test all
go: finding github.com/stretchr/testify/assert latest
go: finding github.com/stretchr/testify/require latest
go: finding github.com/pmezard/go-difflib/difflib latest
go: finding github.com/davecgh/go-spew/spew latest
internal/digreflect/tests/myrepository.git/vendor.go:23:8: unknown import path "mydependency": cannot find module providing package mydependency
can't load package: internal/digreflect/tests/myrepository.git/vendor.go:23:8: unknown import path "mydependency": cannot find module providing package mydependency

The root of the issue is that with modules enabled, the toolchain ignores vendored directories by default. https://github.com/golang/go/wiki/Modules#how-do-i-use-vendoring-with-modules-is-vendoring-going-away So tests/myrepository.git can't import tests/myrepository.git/vendor/mydependency.

Unfortunately, the replace directive doesn't solve this for us because it operates on the current/main module only. https://github.com/golang/go/wiki/Modules#when-should-i-use-the-replace-directive With the right number of replace directives, this works for dig itself but not for consumers of dig. (So @peter-edge's example still fails.)

To fix this, we either need to create a couple test repositories, delete these tests, or find a different way of testing this functionality.

CC @akshayjshah @kriskowal

glibsm commented 6 years ago

One way I've seen other libraries approach it is an external "testing" repo and rely on it. For example, uber-go/dig-dependency.

There may be a way to simply rewrite the tests to validate that we would strip the vendor path, without the need to actually have a dependency package.

mattfarina commented 6 years ago

Has anyone filed an issue in the Go repo about this problem with go modules?

glibsm commented 6 years ago

Has anyone filed an issue in the Go repo about this problem with go modules?

I don't think this has been sufficiently investigated to understand if this is a problem with Go modules, or with the testing setup of dig. Any feedback in this area is appreciated (it works with glide and dep).

Either way, we're going to make some changes to unblock people using go modules while we figure it out.

prashantv commented 6 years ago

Go modules intentionally don't support vendor, see https://github.com/golang/go/wiki/Modules#how-do-i-use-vendoring-with-modules-is-vendoring-going-away

Specifically:

The -mod=vendor flag (e.g., go build -mod=vendor) instructs the go commands to use the main module's top-level vendor directory to satisfy dependencies. The go commands in this mode therefore ignore the dependency descriptions in go.mod and assume that the vendor directory holds the correct copies of dependencies. Note that only the main module's top-level vendor directory is used; vendor directories in other locations are still ignored.

Even if we use -mod=vendor, the vendor inside the test package (E.g., here) won't be supported. The tests should probably be updated to stop using vendor.

glibsm commented 6 years ago

I'm no longer able to reproduce the problem simply through using the tidy, but go test all still fails as described in https://github.com/uber-go/dig/issues/203#issuecomment-421455224

Tidy used to fail in the past:

$ cat main.go
package main

import (
        "fmt"

        "go.uber.org/dig"
)

func main() {
        c := dig.New()
        fmt.Println(c)
}

$ ll
total 8
-rw-r--r--  1 glib  staff   101B Oct 30 13:55 main.go

$ pwd
/Users/glib/code/dig-mod

$ go mod init github.com/glib/dig-mod
go: creating new go.mod: module github.com/glib/dig-mod
$ go mod tidy
go: finding github.com/stretchr/testify/assert latest
go: finding github.com/stretchr/testify/require latest
go: finding github.com/pmezard/go-difflib/difflib latest
go: finding github.com/davecgh/go-spew/spew latest

$ go build
$ ./dig-mod
nodes: {
}
values: {
}

$ go version
go version go1.11.1 darwin/amd64
glibsm commented 6 years ago

@albenik Are you still having issues with go mod and dig?

albenik commented 6 years ago

@glibsm Yes

➜ go mod tidy
go: finding github.com/stretchr/testify/require latest
go: finding github.com/stretchr/testify/assert latest
go: finding github.com/pmezard/go-difflib/difflib latest
go: finding github.com/davecgh/go-spew/spew latest

➜ go test all
internal/digreflect/tests/myrepository.git/vendor.go:23:8: unknown import path "mydependency": cannot find module providing package mydependency
can't load package: internal/digreflect/tests/myrepository.git/vendor.go:23:8: unknown import path "mydependency": cannot find module providing package mydependency

➜ go version
go version go1.11.1 darwin/amd64
glibsm commented 6 years ago

@albenik Thanks.

Even though there are still issues, it seems like it shifted from blocking the tidy command to affecting the test.

Will take a look to make sure go test all passes

JoseFMP commented 5 years ago

For me this is still happening

glibsm commented 5 years ago

@JoseFMP could you provide an example (including your go version)? I'm having a hard time reproducing it.