Allow artifacts from aspects to be excluded (#898)

When using a `java_proto_library` as a dependency of a `java_export`
the generated jar contains classes from protobuf, which is
unexpected. This PR allows users to specify a list of workspaces to
be excluded from the generated jar, and that allows us to filter the
class files generated by these aspects.

WORKSPACE

@@ -586,6 +586,7 @@ maven_install(
 )
 
 load("@m2local_testing_repin//:defs.bzl", "pinned_maven_install")
+
 pinned_maven_install()
 
 maven_install(
@@ -698,3 +699,31 @@ rbe_preconfig(
 load("//migration:maven_jar_migrator_deps.bzl", "maven_jar_migrator_repositories")
 
 maven_jar_migrator_repositories()
+
+# Located at the end, because it's only used in tests
+
+http_archive(
+    name = "com_google_protobuf",
+    sha256 = "6fc9b6efc18acb2fd5fb3bcf981572539c3432600042b662a162c1226b362426",
+    strip_prefix = "protobuf-21.10",
+    url = "https://github.com/protocolbuffers/protobuf/releases/download/v21.10/protobuf-all-21.10.tar.gz",
+)
+
+load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
+
+protobuf_deps()
+
+maven_install(
+    name = "java_export_exclusion_testing",
+    artifacts = [
+        "com.google.protobuf:protobuf-java:3.23.1",
+    ],
+    maven_install_json = "@rules_jvm_external//tests/custom_maven_install:java_export_exclusion_testing_install.json",
+    repositories = [
+        "https://repo1.maven.org/maven2",
+    ],
+)
+
+load("@java_export_exclusion_testing//:defs.bzl", _java_export_exclusion_testing_pinned_maven_install = "pinned_maven_install")
+
+_java_export_exclusion_testing_pinned_maven_install()

docs/api.md

@@ -52,7 +52,8 @@ Generate a javadoc from all the `deps`
 ## java_export
 
 <pre>
-java_export(<a href="#java_export-name">name</a>, <a href="#java_export-maven_coordinates">maven_coordinates</a>, <a href="#java_export-deploy_env">deploy_env</a>, <a href="#java_export-pom_template">pom_template</a>, <a href="#java_export-visibility">visibility</a>, <a href="#java_export-tags">tags</a>, <a href="#java_export-testonly">testonly</a>, <a href="#java_export-kwargs">kwargs</a>)
+java_export(<a href="#java_export-name">name</a>, <a href="#java_export-maven_coordinates">maven_coordinates</a>, <a href="#java_export-deploy_env">deploy_env</a>, <a href="#java_export-excluded_workspaces">excluded_workspaces</a>, <a href="#java_export-pom_template">pom_template</a>, <a href="#java_export-visibility">visibility</a>,
+            <a href="#java_export-tags">tags</a>, <a href="#java_export-testonly">testonly</a>, <a href="#java_export-kwargs">kwargs</a>)
 </pre>
 
 Extends `java_library` to allow maven artifacts to be uploaded.
@@ -105,6 +106,7 @@ Generated rules:
 | <a id="java_export-name"></a>name |  A unique name for this target   |  none |
 | <a id="java_export-maven_coordinates"></a>maven_coordinates |  The maven coordinates for this target.   |  none |
 | <a id="java_export-deploy_env"></a>deploy_env |  A list of labels of Java targets to exclude from the generated jar. [<code>java_binary</code>](https://bazel.build/reference/be/java#java_binary) targets are *not* supported.   |  <code>[]</code> |
+| <a id="java_export-excluded_workspaces"></a>excluded_workspaces |  A dict of strings representing the workspace names of artifacts that should not be included in the maven jar to a <code>Label</code> pointing to the dependency that workspace should be replaced by, or <code>None</code> if the exclusion shouldn't be replaced with an extra dependency.   |  <code>{"com_google_protobuf": None}</code> |
 | <a id="java_export-pom_template"></a>pom_template |  The template to be used for the pom.xml file.   |  <code>None</code> |
 | <a id="java_export-visibility"></a>visibility |  The visibility of the target   |  <code>None</code> |
 | <a id="java_export-tags"></a>tags |  <p align="center"> - </p>   |  <code>[]</code> |

private/rules/java_export.bzl

@@ -1,13 +1,14 @@
 load(":javadoc.bzl", "javadoc")
 load(":maven_bom_fragment.bzl", "maven_bom_fragment")
-load(":maven_project_jar.bzl", "maven_project_jar")
+load(":maven_project_jar.bzl", "DEFAULT_EXCLUDED_WORKSPACES", "maven_project_jar")
 load(":maven_publish.bzl", "maven_publish")
 load(":pom_file.bzl", "pom_file")
 
 def java_export(
         name,
         maven_coordinates,
         deploy_env = [],
+        excluded_workspaces = {name: None for name in DEFAULT_EXCLUDED_WORKSPACES},
         pom_template = None,
         visibility = None,
         tags = [],
@@ -61,6 +62,10 @@ def java_export(
       deploy_env: A list of labels of Java targets to exclude from the generated jar.
         [`java_binary`](https://bazel.build/reference/be/java#java_binary) targets are *not*
         supported.
+      excluded_workspaces: A dict of strings representing the workspace names of artifacts
+        that should not be included in the maven jar to a `Label` pointing to the dependency
+        that workspace should be replaced by, or `None` if the exclusion shouldn't be replaced
+        with an extra dependency.
       visibility: The visibility of the target
       kwargs: These are passed to [`java_library`](https://bazel.build/reference/be/java#java_library),
         and so may contain any valid parameter for that rule.
@@ -85,6 +90,7 @@ def java_export(
         maven_coordinates,
         lib_name,
         deploy_env,
+        excluded_workspaces,
         pom_template,
         visibility,
         tags,
@@ -97,6 +103,7 @@ def maven_export(
         maven_coordinates,
         lib_name,
         deploy_env = [],
+        excluded_workspaces = {},
         pom_template = None,
         visibility = None,
         tags = [],
@@ -150,6 +157,10 @@ def maven_export(
       deploy_env: A list of labels of Java targets to exclude from the generated jar.
         [`java_binary`](https://bazel.build/reference/be/java#java_binary) targets are *not*
         supported.
+      excluded_workspaces: A dict of strings representing the workspace names of artifacts
+        that should not be included in the maven jar to a `Label` pointing to the dependency
+        that workspace should be replaced by, or `None` if the exclusion shouldn't be replaced
+        with an extra dependency.
       visibility: The visibility of the target
       kwargs: These are passed to [`java_library`](https://bazel.build/reference/be/java#java_library),
         and so may contain any valid parameter for that rule.
@@ -158,13 +169,19 @@ def maven_export(
 
     # Sometimes users pass `None` as the value for attributes. Guard against this
     deploy_env = deploy_env if deploy_env else []
+    excluded_workspaces = excluded_workspaces if excluded_workspaces else []
     javadocopts = javadocopts if javadocopts else []
+    tags = tags if tags else []
+
+    additional_dependencies = {label: name for (name, label) in excluded_workspaces.items() if label}
 
     # Merge the jars to create the maven project jar
     maven_project_jar(
         name = "%s-project" % name,
         target = ":%s" % lib_name,
         deploy_env = deploy_env,
+        excluded_workspaces = excluded_workspaces.keys(),
+        additional_dependencies = additional_dependencies,
         visibility = visibility,
         tags = tags + maven_coordinates_tags,
         testonly = testonly,
@@ -201,6 +218,8 @@ def maven_export(
                 ":%s-project" % name,
             ] + deploy_env,
             javadocopts = javadocopts,
+            excluded_workspaces = excluded_workspaces.keys(),
+            additional_dependencies = additional_dependencies,
             visibility = visibility,
             tags = tags,
             testonly = testonly,
@@ -210,6 +229,7 @@ def maven_export(
         name = "%s-pom" % name,
         target = ":%s" % lib_name,
         pom_template = pom_template,
+        additional_dependencies = additional_dependencies,
         visibility = visibility,
         tags = tags,
         testonly = testonly,

private/rules/javadoc.bzl

@@ -1,3 +1,6 @@
+load(":maven_project_jar.bzl", "DEFAULT_EXCLUDED_WORKSPACES")
+load(":maven_utils.bzl", "determine_additional_dependencies")
+
 def generate_javadoc(ctx, javadoc, source_jars, classpath, javadocopts, output):
     args = ctx.actions.args()
     args.add_all(["--out", output])
@@ -19,7 +22,9 @@ def _javadoc_impl(ctx):
 
     jar_file = ctx.actions.declare_file("%s.jar" % ctx.attr.name)
 
-    classpath = depset(transitive = [dep[JavaInfo].transitive_runtime_jars for dep in ctx.attr.deps])
+    # Gather additional files to add to the classpath
+    additional_deps = depset(transitive = [dep[JavaInfo].transitive_runtime_jars for dep in ctx.attr.deps])
+    classpath = depset(transitive = [dep[JavaInfo].transitive_runtime_jars for dep in ctx.attr.deps] + [additional_deps])
 
     # javadoc options and javac options overlap, but we cannot
     # necessarily rely on those to derive the javadoc options we need
@@ -54,6 +59,18 @@ javadoc = rule(
             options can be passed here.
             """,
         ),
+        "excluded_workspaces": attr.string_list(
+            doc = "A list of bazel workspace names to exclude from the generated jar",
+            allow_empty = True,
+            default = DEFAULT_EXCLUDED_WORKSPACES,
+        ),
+        "additional_dependencies": attr.label_keyed_string_dict(
+            doc = "Mapping of `Label`s to the excluded workspace names. Note that this must match the values passed to the `pom_file` rule so the `pom.xml` correctly lists these dependencies.",
+            allow_empty = True,
+            providers = [
+                [JavaInfo],
+            ],
+        ),
         "_javadoc": attr.label(
             default = "//private/tools/java/com/github/bazelbuild/rules_jvm_external/javadoc:javadoc",
             cfg = "exec",

private/rules/kt_jvm_export.bzl

@@ -1,12 +1,14 @@
 load("@io_bazel_rules_kotlin//kotlin:jvm.bzl", "kt_jvm_library")
-load("//private/rules:java_export.bzl", "maven_export")
+load(":java_export.bzl", "maven_export")
+load(":maven_project_jar.bzl", "DEFAULT_EXCLUDED_WORKSPACES")
 
 KOTLIN_STDLIB = "@com_github_jetbrains_kotlin//:kotlin-stdlib"
 
 def kt_jvm_export(
         name,
         maven_coordinates,
         deploy_env = [],
+        excluded_workspaces = {name: None for name in DEFAULT_EXCLUDED_WORKSPACES},
         pom_template = None,
         visibility = None,
         tags = [],
@@ -83,13 +85,14 @@ def kt_jvm_export(
     )
 
     maven_export(
-        name,
-        maven_coordinates,
-        lib_name,
-        updated_deploy_env,
-        pom_template,
-        visibility,
-        tags,
-        testonly,
-        javadocopts,
+        name = name,
+        maven_coordinates = maven_coordinates,
+        lib_name = lib_name,
+        deploy_env = updated_deploy_env,
+        excluded_workspaces = excluded_workspaces,
+        pom_template = pom_template,
+        visibility = visibility,
+        tags = tags,
+        testonly = testonly,
+        javadocopts = javadocopts,
     )

private/rules/maven_project_jar.bzl

@@ -1,4 +1,25 @@
 load(":has_maven_deps.bzl", "MavenInfo", "calculate_artifact_jars", "calculate_artifact_source_jars", "has_maven_deps")
+load(":maven_utils.bzl", "determine_additional_dependencies")
+
+DEFAULT_EXCLUDED_WORKSPACES = [
+    # Note: we choose to drop the dependency entirely because
+    #       we can't be sure which coordinate the user has
+    #       chosen for protobuf.
+    "com_google_protobuf",
+]
+
+def _strip_excluded_workspace_jars(jar_files, excluded_workspaces):
+    to_return = []
+
+    for jar in jar_files:
+        owner = jar.owner
+
+        if owner and owner.workspace_name in excluded_workspaces:
+            continue
+
+        to_return.append(jar)
+
+    return to_return
 
 def _combine_jars(ctx, merge_jars, inputs, excludes, output):
     args = ctx.actions.args()
@@ -20,7 +41,20 @@ def _maven_project_jar_impl(ctx):
 
     # Identify the subset of JavaInfo to include in the artifact
     artifact_jars = calculate_artifact_jars(info)
-    artifact_srcs = calculate_artifact_source_jars(info)
+
+    # We need to know the additional dependencies we might need to add
+    additional_deps = [dep[JavaInfo] for dep in determine_additional_dependencies(artifact_jars, ctx.attr.additional_dependencies)]
+
+    # And now we strip out dependencies if they're not needed
+    artifact_jars = _strip_excluded_workspace_jars(
+        artifact_jars,
+        ctx.attr.excluded_workspaces,
+    )
+
+    artifact_srcs = _strip_excluded_workspace_jars(
+        calculate_artifact_source_jars(info),
+        ctx.attr.excluded_workspaces,
+    )
 
     # Merge together all the binary jars
     bin_jar = ctx.actions.declare_file("%s.jar" % ctx.label.name)
@@ -84,7 +118,7 @@ def _maven_project_jar_impl(ctx):
         source_jar = src_jar,
 
         # TODO: calculate runtime_deps too
-        deps = info.dep_infos.to_list(),
+        deps = info.dep_infos.to_list() + additional_deps,
         exports = exported_infos,
     )
 
@@ -133,6 +167,18 @@ single artifact that other teams can download and use.
             ],
             allow_empty = True,
         ),
+        "excluded_workspaces": attr.string_list(
+            doc = "A list of bazel workspace names to exclude from the generated jar",
+            allow_empty = True,
+            default = DEFAULT_EXCLUDED_WORKSPACES,
+        ),
+        "additional_dependencies": attr.label_keyed_string_dict(
+            doc = "Mapping of `Label`s to the excluded workspace names. Note that this must match the values passed to the `pom_file` rule so the `pom.xml` correctly lists these dependencies.",
+            allow_empty = True,
+            providers = [
+                [JavaInfo],
+            ],
+        ),
         # Bazel's own singlejar doesn't respect java service files,
         # so use our own.
         "_merge_jars": attr.label(

private/rules/maven_utils.bzl

@@ -128,3 +128,21 @@ def generate_pom(
     )
 
     return out
+
+def determine_additional_dependencies(jar_files, additional_dependencies):
+    """Takes a dict of {`Label`: workspace_name} and returns the `Label`s where any `jar_files match a `workspace_name."""
+    to_return = []
+
+    for jar in jar_files:
+        owner = jar.owner
+
+        # If we can't tell who the owner is, let's assume things are fine
+        if not owner:
+            continue
+
+        for (dep, name) in additional_dependencies.items():
+            if (name == owner.workspace_name) and dep:
+                if not dep in to_return:
+                    to_return.append(dep)
+
+    return to_return

private/rules/pom_file.bzl

@@ -1,5 +1,5 @@
-load(":has_maven_deps.bzl", "MavenInfo", "has_maven_deps")
-load(":maven_utils.bzl", "generate_pom")
+load(":has_maven_deps.bzl", "MavenInfo", "calculate_artifact_jars", "has_maven_deps")
+load(":maven_utils.bzl", "determine_additional_dependencies", "generate_pom")
 
 def _pom_file_impl(ctx):
     # Ensure the target has coordinates
@@ -8,16 +8,27 @@ def _pom_file_impl(ctx):
 
     info = ctx.attr.target[MavenInfo]
 
+    artifact_jars = calculate_artifact_jars(info)
+    additional_deps = determine_additional_dependencies(artifact_jars, ctx.attr.additional_dependencies)
+
+    all_maven_deps = info.maven_deps.to_list()
+    for dep in additional_deps:
+        for coords in dep[MavenInfo].as_maven_dep.to_list():
+            all_maven_deps.append(coords)
+
     out = generate_pom(
         ctx,
         coordinates = info.coordinates,
-        versioned_dep_coordinates = sorted(info.maven_deps.to_list()),
+        versioned_dep_coordinates = sorted(all_maven_deps),
         pom_template = ctx.file.pom_template,
         out_name = "%s.xml" % ctx.label.name,
     )
 
     return [
-        DefaultInfo(files = depset([out])),
+        DefaultInfo(
+            files = depset([out]),
+            data_runfiles = ctx.runfiles([out]),
+        ),
         OutputGroupInfo(
             pom = depset([out]),
         ),
@@ -53,5 +64,15 @@ The following substitutions are performed on the template file:
                 has_maven_deps,
             ],
         ),
+        "additional_dependencies": attr.label_keyed_string_dict(
+            doc = "Mapping of `Label`s to the excluded workspace names",
+            allow_empty = True,
+            providers = [
+                [JavaInfo],
+            ],
+            aspects = [
+                has_maven_deps,
+            ],
+        ),
     },
 )