dependency with same name as directory causes "missing strict dependencies" error

[nlacasse]

A dependency with the same name as its directory (deps = ["//foo:foo"]) causes an error. The error also occurs if the short-form name is used (deps = ["//foo"]).

Here is a small repro:
https://github.com/nlacasse/rules_go/tree/deps-test/tests/trans_dep

The //tests/trans_dep:lib target builds correctly on 0.5.3, but fails on 0.5.4 and 0.5.5 with the following error:

2017/09/13 17:48:08 missing strict dependencies:
        tests/trans_dep/lib.go: import of github.com/bazelbuild/rules_go/tests/trans_dep/dep/dep, which is not a direct dependency
Target //tests/trans_dep:broken failed to build

The error is coming from the checkDirectDeps function:
https://github.com/bazelbuild/rules_go/blob/master/go/tools/builders/compile.go#L127

The cause of the bug seems to be that we stopped adding the last part of the path in go_importpath if it is a "stutter":
https://github.com/bazelbuild/rules_go/pull/716/files#diff-b2f5a5f41a12c9d41a59f56369d54834L202

This seems wrong, since it is perfectly reasonable to have a package named //foo:foo.

[jayconrod]

As described above, this is the result of a change in behavior of go_importpath, which determines the import path of a library, based on its label. This has had a special case for go_default_library. A new case was added to remove the stutter if the library name matches the package base name.

//foo/bar/baz:go_default_library -> "<go_prefix>/foo/bar/baz"
//foo/bar/baz:baz -> "<go_prefix>/foo/bar/baz"
//foo/bar/baz:quux -> "<go_prefix>/foo/bar/baz/quux"

We're planning to move away from go_prefix (#721), since it places a lot of restrictions on repository layout and requires a lot of special cases. In the future, the import path will be set explicitly with an importpath attribute. This is already supported for go_library, go_binary, and go_test. It is optional now, but in a future version, it will be mandatory. So even if we rolled back this change, you would have the same problem again soon.

go_library(
    name = "foo",
    srcs = [...],
    deps = [...],
    importpath = "github.com/example/project/foo",
)

For projects that follow go build conventions, Gazelle will add these attributes automatically. Gazelle will attempt to infer the import path based on a prefix directive and the directory name

We don't yet have a good story for projects based in google3. Using Buildozer to list rules, calculate the import paths based on label, would be straightforward with a shell script, but it doesn't seem like Copybara supports custom transformations. It does support buildozer commands, but you would need to hard-code all of the go_library rules into your copy.bara.sky.

@ianthehat had an idea: add importpath attributes in your BUILD files, but rather than using the native rules, use a macro that drops the importpath attribute for Blaze only.

# blaze_compat.bzl

def go_library_adapter(name, importpath=None, **kwargs):
  native.go_library(name, **kwargs)

# BUILD
load("//:blaze_compat.bzl", go_library = "go_library_adapter")

...

Is that something that would work for you?

[nlacasse]

For the short-term, I can fix my repo by changing the imports in my go files to remove the duplicated final element. That is something that copybara can do easily. (Not sure why I didn't think of this yesterday.) Still though, this change was undocumented and surprising to me, and may be to other users as well.

For the long-term solution, what you propose will certainly work. However, it's a bit annoying to have to add all of the importpaths in our BUILD files manually, and keep them up-to-date when the code moves. I think it should be possible to generate these attributes automatically (go_importpath basically generates them implicitly now). I'll talk with the Buildozer team to see if there is some functionality we can add that would make this work.

[jayconrod]

Ok, sounds like that will be the easiest path.

Sorry this change wasn't documented more loudly. We probably should have mentioned it in the release notes at some point.

Agree it's annoying to add importpaths automatically. Gazelle and Buildozer could automate a lot of this, but it's difficult to use that with Copybara.