language/go: fix should remove importpath from go_binary and go_test

[AustinSchuh]

I've got the following BUILD file from gazelle:

package(default_visibility = ["//visibility:public"])

load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

licenses(["notice"])

go_library(
    name = "go_default_library",
    srcs = [ 
        "api.go",
        "service.go",
    ],  
    importmap = "peloton-tech.com/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata",
    importpath = "github.com/aws/aws-sdk-go/aws/ec2metadata",
    deps = [ 
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws:go_default_library",
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/awserr:go_default_library",
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/client:go_default_library",
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/client/metadata:go_default_library",
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/corehandlers:go_default_library",
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/request:go_default_library",
    ],  
)

go_test(
    name = "go_default_test",
    srcs = [ 
        "api_test.go",
        "service_test.go",
    ],  
    embed = [":go_default_library"],
    importpath = "github.com/aws/aws-sdk-go/aws/ec2metadata_test",
    deps = [ 
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws:go_default_library",
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/awserr:go_default_library",
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/request:go_default_library",
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/awstesting:go_default_library",
        "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/awstesting/unit:go_default_library",
    ],  
)

This is AWS's sdk in case you want to peek at the project. Gazelle keeps rewriting the :go_default_library dependency to embed.

diff --git a/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata/BUILD b/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata/BUILD
index 3b61f9e..d54d646 100644
--- a/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata/BUILD
+++ b/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata/BUILD
@@ -28,9 +28,9 @@ go_test(
         "api_test.go",
         "service_test.go",
     ],
+    embed = [":go_default_library"],
     importpath = "github.com/aws/aws-sdk-go/aws/ec2metadata_test",
     deps = [
-        ":go_default_library",
         "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws:go_default_library",
         "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/awserr:go_default_library",
         "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/request:go_default_library",

The end result is that when I build, I get the following error:

Use --sandbox_debug to see verbose messages from the sandbox
GoCompile: missing strict dependencies:
        /dev/shm/bazel-sandbox.10adf27e5d0740a5a4759df72d8b9ee9/linux-sandbox/2/execroot/com_peloton_tech/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata/api_test.go: import of "github.com/aws/aws-sdk-go/aws/ec2metadata"
        /dev/shm/bazel-sandbox.10adf27e5d0740a5a4759df72d8b9ee9/linux-sandbox/2/execroot/com_peloton_tech/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata/service_test.go: import of "github.com/aws/aws-sdk-go/aws/ec2metadata"
Known dependencies are:
        github.com/aws/aws-sdk-go/aws
        github.com/aws/aws-sdk-go/aws/awserr
        github.com/aws/aws-sdk-go/aws/request
        github.com/aws/aws-sdk-go/awstesting
        github.com/aws/aws-sdk-go/awstesting/unit
        github.com/aws/aws-sdk-go/aws/client
        github.com/aws/aws-sdk-go/aws/client/metadata
        github.com/aws/aws-sdk-go/aws/corehandlers
        github.com/aws/aws-sdk-go/aws/ec2metadata_test
Check that imports in Go sources match importpath attributes in deps.

From the generated rule, you can see that the package path doesn't match on the two packages. I think Gazelle should not use embed in that case.

[jayconrod]

I think the embed is correct and the importpath of github.com/aws/aws-sdk-go/aws/ec2metadata_test is incorrect. It should match the library being tested, even if all the test sources are external. Did Gazelle generate the importpath, or was that changed manually? Maybe it came from an old version, before go_default_xtest was removed?

Under the hood, go_test will build three archives.

• An internal test archive is built from the sources (both in the go_test srcs attribute and those of embedded libraries) that have non-test package declarations (e.g., package foo). This includes sources from the library being tested. Anything that imports the library will get this archive. This has the importpath from the go_test or from the embedded library.
• An external test archive is built from sources (again, both in go_test and embedded) that have test package declarations (e.g., package foo_test). The import path for this is the internal import path plus _test.
• The main archive is built from a generated source file that imports the two archives above.

So even if a go_test only has external test sources, it should still embed the library. We used to require two separate go_test rules for internal test and external tests, but that prevented people from extending the public API in internal tests and using that from external tests. gazelle fix should squash remaining go_default_xtest rules, since they're no longer needed.

[AustinSchuh]

A previous version of Gazelle generated the importpath attribute. It made the following change to update the rules:

diff --git a/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata/BUILD b/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata/BUILD
index 196f394..3b61f9e 100644
--- a/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata/BUILD
+++ b/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata/BUILD
@@ -10,6 +10,7 @@ go_library(
         "api.go",
         "service.go",
     ],
+    importmap = "peloton-tech.com/third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws/ec2metadata",
     importpath = "github.com/aws/aws-sdk-go/aws/ec2metadata",
     deps = [
         "//third_party/go/src/vendor/github.com/aws/aws-sdk-go/aws:go_default_library",
@@ -22,7 +23,7 @@ go_library(
 )

 go_test(
-    name = "go_default_xtest",
+    name = "go_default_test",
     srcs = [
         "api_test.go",
         "service_test.go",

I tried changing the import path of the go_test target and gazelle leaves it alone on a rerun.

I now know how to make the changes in such a way that gazelle and I don't fight so I'm happy. Does it make sense to change the behavior during upgrading for future users? Or leave it?

[jayconrod]

Hmm, this is a little delicate. Gazelle used to generate importpath attributes for go_test and go_binary rules, but it doesn't anymore. importpath is not considered mergeable for those rules, so Gazelle won't remove it if you've set it. That's important for standalone binaries and tests, since they won't show up in the right place in a go_path otherwise. Ideally, we should remove importpath attributes if you've embedded a library with the same importpath (with a special case for _test suffix).

I'll leave this open as a bug. I'm not sure I'll have time to fix it soon though.