Go completer has dependencies on outdated fork and crashes

[LuisAlejandro]

So, ycmd uses https://github.com/Manishearth/godef.git as a submodule for building the go completer but that fork is outdated, causing the following error:

./godef.go:52: undefined: types.GoPath
./godef.go:90: not enough arguments in call to parser.ParseFile
./godef.go:123: not enough arguments in call to types.ExprType
./godef.go:137: not enough arguments in call to types.ExprType
./godef.go:235: too many arguments in call to typ.Iter
./godef.go:246: not enough arguments in call to types.ExprType
./godef.go:267: too many arguments in call to typ.Underlying
./godef.go:274: not enough arguments in call to parser.ParseExpr
./godef.go:326: not enough arguments in call to parser.ParseFile
./godef.go:341: not enough arguments in call to parser.ParseFile
./godef.go:341: too many errors

Upon further inspection, such fork depends on a types.GoPath variable which is no longer set by one of its dependencies (rogpeppe/godef@21b69de).

Hope you can fix this. Let me know if I can help in any way.

[micbou]

Thanks for the report. @Manishearth is maintaining the fork so we need to wait for him to update it and then we'll update the submodule.

[Manishearth]

That's not right. We don't depend on rogpeppe/godef, all dependencies are in-tree with paths rewritten, precisely to avoid this issue.

https://github.com/Manishearth/godef/blob/ycmd_compat/go_local/types/types.go

[LuisAlejandro]

@Manishearth Well, then ymcd's submodule is not pointing to the right branch https://github.com/Valloric/ycmd/blob/master/.gitmodules#L28

[LuisAlejandro]

Wait, no ... there's something wrong on my side. Let me take a look and I'll report back.

[LuisAlejandro]

I'm sorry, i ran git submodule update --init --recursive --remote, which made every submodule to sync into different hashes. I'll close this issue.

[vheon]

@Manishearth while we're on the subject, since the original project does support the json output now (IIRC that was the reason for using a fork), are there any reasons why we're not using the original project?

[Manishearth]

That was only part of the reason for using a fork, the other reason is that that project can't be built in-tree.

Go has two mutually-exclusive modes of referring to package deps. Either you use github.com/foo/bar deps, or you use path deps. If you are a github.com package, even if you have sub-packages in folders locally, you have to refer to them as github.com/user/repo/subpackagefolder.

Now, we could make the build steps go get github.com/rogpeppe/godef && go build github.com/rogpeppe/godef && go run github.com/rogpeppe/godef $ARGS. However, this leaves us without a way to pin to a version to ensure that there are no breaking changes (i.e. to avoid this exact situation).

If we just add upstream godef as a submodule, it won't build. godef uses github.com dependencies to refer to its subpackages (because of the silly requirement that github.com packages can't have path references within themselves), and so go build will expect something in $GOPATH/github.com/rogpeppe/godef, which won't be there -- even if we fetch something there, it would have the same issue with breaking changes. We could directly write to $GOPATH but that's hacky and probably imperfect depending on the larger setup.

Thus, to be able to pin to a version, we need to have something with the github.com paths rewritten as relative paths. The upstream repo can't do this because such projects cannot be go getted. Thus the fork.

I can rebase the fork to pull in any improvements, but we can't get rid of it 😢

[vheon]

@Manishearth thanks for the explanation. Has been a while since I coded with go and I didn't thought of those detailed 😒 A over engineered solution could be to add upstream godef as a submodule and write a little go program that parses the godef and rewrite the imports so when we build ycmd with go support it would run and patch godef as we like, but I don't think that we want to go there 😜 Thanks again 👍