GoPackagesDriver: Reducing duplicate information passed by go_pkg_inf…

…o_aspect (#3111)

* avoid writing package json for go_test dependencies

go/tools/gopackagesdriver/aspect.bzl

@@ -59,24 +59,17 @@ def _go_pkg_info_aspect_impl(target, ctx):
     # Fetch the stdlib JSON file from the inner most target
     stdlib_json_file = None
 
-    deps_transitive_json_file = []
-    deps_transitive_export_file = []
-    deps_transitive_compiled_go_files = []
+    transitive_json_files = []
+    transitive_export_files = []
+    transitive_compiled_go_files = []
 
     for attr in ["deps", "embed"]:
         for dep in getattr(ctx.rule.attr, attr, []):
             if GoPkgInfo in dep:
                 pkg_info = dep[GoPkgInfo]
-                if attr == "deps":
-                    deps_transitive_json_file.append(pkg_info.transitive_json_file)
-                    deps_transitive_export_file.append(pkg_info.transitive_export_file)
-                    deps_transitive_compiled_go_files.append(pkg_info.transitive_compiled_go_files)
-                elif attr == "embed":
-                    # If deps are embedded, do not gather their json or export_file since they
-                    # are included in the current target, but do gather their deps'.
-                    deps_transitive_json_file.append(pkg_info.deps_transitive_json_file)
-                    deps_transitive_export_file.append(pkg_info.deps_transitive_export_file)
-                    deps_transitive_compiled_go_files.append(pkg_info.deps_transitive_compiled_go_files)
+                transitive_json_files.append(pkg_info.pkg_json_files)
+                transitive_compiled_go_files.append(pkg_info.compiled_go_files)
+                transitive_export_files.append(pkg_info.export_files)
 
                 # Fetch the stdlib json from the first dependency
                 if not stdlib_json_file:
@@ -93,23 +86,15 @@ def _go_pkg_info_aspect_impl(target, ctx):
         pkg = _go_archive_to_pkg(archive)
         pkg_json_files.append(_make_pkg_json(ctx, archive, pkg))
 
-        # if the rule is a test, we need to get the embedded go_library with the current
-        # test's sources. For that, consume the dependency via GoArchive.direct so that
-        # the test source files are there. Then, create the pkg json file directly. Only
-        # do that for direct dependencies that are not defined as deps, and use the
-        # importpath to find which.
         if ctx.rule.kind == "go_test":
-            deps_targets = [
-                dep[GoArchive].data.importpath
-                for dep in ctx.rule.attr.deps
-                if GoArchive in dep
-            ]
-            for archive in target[GoArchive].direct:
-                if archive.data.importpath not in deps_targets:
-                    pkg = _go_archive_to_pkg(archive)
-                    pkg_json_files.append(_make_pkg_json(ctx, archive, pkg))
-                    compiled_go_files.extend(archive.source.srcs)
-                    export_files.append(archive.data.export_file)
+            for dep_archive in archive.direct:
+                # find the archive containing the test sources
+                if archive.data.label == dep_archive.data.label:
+                    pkg = _go_archive_to_pkg(dep_archive)
+                    pkg_json_files.append(_make_pkg_json(ctx, dep_archive, pkg))
+                    compiled_go_files.extend(dep_archive.source.srcs)
+                    export_files.append(dep_archive.data.export_file)
+                    break
 
     # If there was no stdlib json in any dependencies, fetch it from the
     # current go_ node.
@@ -118,35 +103,26 @@ def _go_pkg_info_aspect_impl(target, ctx):
 
     pkg_info = GoPkgInfo(
         stdlib_json_file = stdlib_json_file,
-        transitive_json_file = depset(
+        pkg_json_files = depset(
             direct = pkg_json_files,
-            transitive = deps_transitive_json_file,
+            transitive = transitive_json_files,
         ),
-        deps_transitive_json_file = depset(
-            transitive = deps_transitive_json_file,
-        ),
-        transitive_compiled_go_files = depset(
+        compiled_go_files = depset(
             direct = compiled_go_files,
-            transitive = deps_transitive_compiled_go_files,
-        ),
-        deps_transitive_compiled_go_files = depset(
-            transitive = deps_transitive_compiled_go_files,
+            transitive = transitive_compiled_go_files,
         ),
-        transitive_export_file = depset(
+        export_files = depset(
             direct = export_files,
-            transitive = deps_transitive_export_file,
-        ),
-        deps_transitive_export_file = depset(
-            transitive = deps_transitive_export_file,
+            transitive = transitive_export_files,
         ),
     )
 
     return [
         pkg_info,
         OutputGroupInfo(
-            go_pkg_driver_json_file = pkg_info.transitive_json_file,
-            go_pkg_driver_srcs = pkg_info.transitive_compiled_go_files,
-            go_pkg_driver_export_file = pkg_info.transitive_export_file,
+            go_pkg_driver_json_file = pkg_info.pkg_json_files,
+            go_pkg_driver_srcs = pkg_info.compiled_go_files,
+            go_pkg_driver_export_file = pkg_info.export_files,
             go_pkg_driver_stdlib_json_file = depset([pkg_info.stdlib_json_file] if pkg_info.stdlib_json_file else []),
         ),
     ]

tests/core/starlark/packagedriver/BUILD.bazel

@@ -0,0 +1,5 @@
+load(":go_pkg_info_aspect_test.bzl", "package_driver_suite")
+
+package_driver_suite(
+    name = "package_driver_test"
+)

tests/core/starlark/packagedriver/fixtures/a/BUILD.bazel

@@ -0,0 +1,9 @@
+load("//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["a.go"],
+    importpath = "example.com/a",
+    visibility = ["//tests/core/starlark/packagedriver:__subpackages__"],
+)
+

tests/core/starlark/packagedriver/fixtures/a/a.go

@@ -0,0 +1,5 @@
+package a
+
+func A() {
+	return
+}

tests/core/starlark/packagedriver/fixtures/b/BUILD.bazel

@@ -0,0 +1,8 @@
+load("//go:def.bzl", "go_library")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["b.go"],
+    importpath = "example.com/b",
+    visibility = ["//tests/core/starlark/packagedriver:__subpackages__"],
+)

tests/core/starlark/packagedriver/fixtures/b/b.go

@@ -0,0 +1,5 @@
+package b
+
+func B() {
+	return
+}

tests/core/starlark/packagedriver/fixtures/c/BUILD.bazel

@@ -0,0 +1,20 @@
+load("//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["c.go"],
+    importpath = "example.com/c",
+    visibility = ["//tests/core/starlark/packagedriver:__subpackages__"],
+    deps = [
+        "//tests/core/starlark/packagedriver/fixtures/a:go_default_library",
+        "//tests/core/starlark/packagedriver/fixtures/b:go_default_library",
+    ],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["c_test.go"],
+    embed = [":go_default_library"],
+    tags = ["manual"],
+    visibility = ["//tests/core/starlark/packagedriver:__subpackages__"],
+)

tests/core/starlark/packagedriver/fixtures/c/c.go

@@ -0,0 +1,11 @@
+package c
+
+import (
+	"example.com/a"
+	"example.com/b"
+)
+
+func C() {
+	a.A()
+	b.B()
+}

tests/core/starlark/packagedriver/fixtures/c/c_test.go

@@ -0,0 +1,9 @@
+package c
+
+import (
+	"testing"
+)
+
+func TestC(t *testing.T) {
+	C()
+}

tests/core/starlark/packagedriver/go_pkg_info_aspect_test.bzl

@@ -0,0 +1,33 @@
+load("@bazel_skylib//lib:unittest.bzl", "asserts", "analysistest")
+load("//go/tools/gopackagesdriver:aspect.bzl", "go_pkg_info_aspect")
+
+
+def _package_driver_pkg_json_test_impl(ctx):
+    env = analysistest.begin(ctx)
+
+    target_under_test = analysistest.target_under_test(env)
+    json_files = [f.basename for f in target_under_test[OutputGroupInfo].go_pkg_driver_json_file.to_list()]
+    asserts.true(env, "go_default_test.pkg.json" in json_files, "{} does not contain go_default_test.pkg.json".format(json_files))
+
+    return analysistest.end(env)
+
+package_driver_pkg_json_test = analysistest.make(
+    _package_driver_pkg_json_test_impl,
+    extra_target_under_test_aspects = [go_pkg_info_aspect],
+)
+
+def _test_package_driver():
+    package_driver_pkg_json_test(
+        name = "package_driver_should_return_pkg_json_for_go_test",
+        target_under_test = "//tests/core/starlark/packagedriver/fixtures/c:go_default_test",
+    )
+
+def package_driver_suite(name):
+    _test_package_driver()
+
+    native.test_suite(
+        name = name,
+        tests = [
+            ":package_driver_should_return_pkg_json_for_go_test",
+        ],
+    )