Align dependency handling with Bazel best practices (#9165)

This commit removes the use of bind() since that function goes against Bazel best practices: https://docs.bazel.build/versions/main/external.html#repository-rules-1 The bind() function basically maps a dependency into //external, but there is no good reason to do this. By mapping dependencies into //external and relying on this in our own BUILD files, we're forcing projects that depend on us to do the same. The one bind() call that I did leave in place was //:python_headers. This one seems to be doing something complicated I don't fully understand, and I don't want to risk breaking it. This change also moves our list of required Maven artifacts into a constant in protobuf_deps.bzl. This way, projects that depend on us can refer to this list when they invoke maven_install() and automatically pull in all the necesary dependencies. This fixes #9132.
protocolbuffers · Oct 28, 2021 · c7dfd0d · suztomo · Nov 29, 2021 · suztomo
1 parent 67c2a92
commit c7dfd0d
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 115 deletions.
diff --git a/WORKSPACE b/WORKSPACE
@@ -27,7 +27,7 @@ http_archive(
 )
 
 # Load common dependencies.
-load("//:protobuf_deps.bzl", "protobuf_deps")
+load("//:protobuf_deps.bzl", "PROTOBUF_MAVEN_ARTIFACTS", "protobuf_deps")
 protobuf_deps()
 
 bind(
@@ -36,70 +36,23 @@ bind(
 )
 
 load("@rules_jvm_external//:defs.bzl", "maven_install")
-maven_install(
-    artifacts = [
-        "com.google.code.findbugs:jsr305:3.0.2",
-        "com.google.code.gson:gson:2.8.6",
-        "com.google.errorprone:error_prone_annotations:2.3.2",
-        "com.google.j2objc:j2objc-annotations:1.3",
-        "com.google.guava:guava:30.1.1-jre",
-        "com.google.truth:truth:1.1.2",
-        "junit:junit:4.12",
-        "org.easymock:easymock:3.2",
 
-    ],
+maven_install(
+    artifacts = PROTOBUF_MAVEN_ARTIFACTS,
+    # For updating instructions, see:
+    # https://github.com/bazelbuild/rules_jvm_external#updating-maven_installjson
+    maven_install_json = "//:maven_install.json",
     repositories = [
         "https://repo1.maven.org/maven2",
         "https://repo.maven.apache.org/maven2",
     ],
-    # For updating instructions, see: 
-    # https://github.com/bazelbuild/rules_jvm_external#updating-maven_installjson
-    maven_install_json = "//:maven_install.json",
 )
 
 load("@maven//:defs.bzl", "pinned_maven_install")
-pinned_maven_install()
-
-bind(
-    name = "guava",
-    actual = "@maven//:com_google_guava_guava",
-)
-
-bind(
-    name = "gson",
-    actual = "@maven//:com_google_code_gson_gson",
-)
-
-bind(
-    name = "error_prone_annotations",
-    actual = "@maven//:com_google_errorprone_error_prone_annotations",
-)
 
-bind(
-    name = "j2objc_annotations",
-    actual = "@maven//:com_google_j2objc_j2objc_annotations",
-)
-
-bind(
-    name = "jsr305",
-    actual = "@maven//:com_google_code_findbugs_jsr305",
-)
-
-bind(
-    name = "junit",
-    actual = "@maven//:junit_junit",
-)
-
-bind(
-    name = "easymock",
-    actual = "@maven//:org_easymock_easymock",
-)
-
-bind(
-    name = "truth",
-    actual = "@maven//:com_google_truth_truth",
-)
+pinned_maven_install()
 
 # For `cc_proto_blacklist_test` and `build_test`.
 load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace")
+
 bazel_skylib_workspace()
diff --git a/java/core/BUILD b/java/core/BUILD
@@ -1,5 +1,5 @@
 load("@bazel_skylib//rules:build_test.bzl", "build_test")
-load("@rules_java//java:defs.bzl", "java_library", "java_proto_library", "java_lite_proto_library")
+load("@rules_java//java:defs.bzl", "java_library", "java_lite_proto_library", "java_proto_library")
 load("@rules_jvm_external//:defs.bzl", "java_export")
 load("@rules_proto//proto:defs.bzl", "proto_lang_toolchain", "proto_library")
 load("//:internal.bzl", "conformance_test")
@@ -103,7 +103,7 @@ LITE_SRCS = [
 java_library(
     name = "lite",
     srcs = LITE_SRCS + [
-        "//:gen_well_known_protos_javalite"
+        "//:gen_well_known_protos_javalite",
     ],
     visibility = [
         "//java/lite:__pkg__",
@@ -115,10 +115,10 @@ java_export(
     name = "lite_mvn",
     maven_coordinates = "com.google.protobuf:protobuf-javalite:%s" % PROTOBUF_VERSION,
     pom_template = "//java/lite:pom_template.xml",
-    runtime_deps = [":lite"],
     resources = [
         "//:lite_well_known_protos",
     ],
+    runtime_deps = [":lite"],
 )
 
 java_library(
@@ -150,25 +150,25 @@ java_export(
     name = "core_mvn",
     maven_coordinates = "com.google.protobuf:protobuf-java:%s" % PROTOBUF_VERSION,
     pom_template = "pom_template.xml",
-    runtime_deps = [":core"],
     resources = [
         "//:well_known_protos",
     ],
+    runtime_deps = [":core"],
 )
 
 filegroup(
     name = "release",
-    visibility = ["//java:__pkg__"],
     srcs = [
-        ":core_mvn-pom",
-        ":core_mvn-maven-source",
         ":core_mvn-docs",
+        ":core_mvn-maven-source",
+        ":core_mvn-pom",
         ":core_mvn-project",
-        ":lite_mvn-pom",
-        ":lite_mvn-maven-source",
         ":lite_mvn-docs",
+        ":lite_mvn-maven-source",
+        ":lite_mvn-pom",
         ":lite_mvn-project",
-    ]
+    ],
+    visibility = ["//java:__pkg__"],
 )
 
 proto_lang_toolchain(
@@ -207,22 +207,22 @@ java_library(
     name = "test_util",
     srcs = [
         "src/test/java/com/google/protobuf/TestUtil.java",
-        "src/test/java/com/google/protobuf/TestUtilLite.java"
+        "src/test/java/com/google/protobuf/TestUtilLite.java",
     ],
     deps = [
         ":core",
         ":generic_test_protos_java_proto",
         ":java_test_protos_java_proto",
-        "//external:guava",
-        "//external:junit",
+        "@maven//:com_google_guava_guava",
+        "@maven//:junit_junit",
     ],
 )
 
 test_suite(
     name = "tests",
     tests = [
-        "core_build_test",
         "conformance_test",
+        "core_build_test",
         "core_tests",
     ],
 )
@@ -236,29 +236,32 @@ build_test(
 
 conformance_test(
     name = "conformance_test",
-    testee = "//:conformance_java",
     failure_list = "//:conformance/failure_list_java.txt",
+    testee = "//:conformance_java",
     text_format_failure_list = "//:conformance/text_format_failure_list_java.txt",
 )
 
 junit_tests(
     name = "core_tests",
-    srcs = glob(["src/test/java/**/*.java"], exclude = [
-        "src/test/java/com/google/protobuf/TestUtil.java",
-        "src/test/java/com/google/protobuf/TestUtilLite.java",
-    ]),
-    data = ["//:testdata"],
     size = "large",
+    srcs = glob(
+        ["src/test/java/**/*.java"],
+        exclude = [
+            "src/test/java/com/google/protobuf/TestUtil.java",
+            "src/test/java/com/google/protobuf/TestUtilLite.java",
+        ],
+    ),
+    data = ["//:testdata"],
     deps = [
         ":core",
         ":generic_test_protos_java_proto",
         ":java_test_protos_java_proto",
         ":test_util",
-        "//external:easymock",
-        "//external:guava",
-        "//external:junit",
-        "//external:truth",
-    ]
+        "@maven//:com_google_guava_guava",
+        "@maven//:com_google_truth_truth",
+        "@maven//:junit_junit",
+        "@maven//:org_easymock_easymock",
+    ],
 )
 
 java_lite_proto_library(
@@ -281,17 +284,17 @@ genrule(
     name = "rewrite_javalite_test_util",
     srcs = [
         "//java/lite:lite.awk",
-        "src/test/java/com/google/protobuf/TestUtil.java"
+        "src/test/java/com/google/protobuf/TestUtil.java",
     ],
     outs = ["TestUtil.java"],
-    cmd = "awk -f $(location //java/lite:lite.awk) $(location src/test/java/com/google/protobuf/TestUtil.java) > $@"
+    cmd = "awk -f $(location //java/lite:lite.awk) $(location src/test/java/com/google/protobuf/TestUtil.java) > $@",
 )
 
 java_library(
     name = "test_util_lite",
     srcs = [
+        "src/test/java/com/google/protobuf/TestUtilLite.java",
         ":rewrite_javalite_test_util",
-        "src/test/java/com/google/protobuf/TestUtilLite.java"
     ],
     visibility = [
         "//java/lite:__pkg__",
@@ -300,8 +303,8 @@ java_library(
         ":generic_test_protos_java_proto_lite",
         ":java_test_protos_java_proto_lite",
         ":lite_runtime_only",
-        "//external:guava",
-        "//external:junit",
+        "@maven//:com_google_guava_guava",
+        "@maven//:junit_junit",
     ],
 )
 
@@ -350,18 +353,20 @@ LITE_TEST_EXCLUSIONS = [
 
 junit_tests(
     name = "lite_tests",
-    srcs = glob(["src/test/java/**/*.java"], exclude = LITE_TEST_EXCLUSIONS),
+    size = "large",
+    srcs = glob(
+        ["src/test/java/**/*.java"],
+        exclude = LITE_TEST_EXCLUSIONS,
+    ),
     data = ["//:testdata"],
     test_prefix = "Lite",
-    size = "large",
     deps = [
-        ":lite",
         ":generic_test_protos_java_proto_lite",
         ":java_test_protos_java_proto_lite",
+        ":lite",
         ":test_util_lite",
-        "//external:easymock",
-        "//external:junit",
-        "//external:truth",
-    ]
+        "@maven//:com_google_truth_truth",
+        "@maven//:junit_junit",
+        "@maven//:org_easymock_easymock",
+    ],
 )
-
diff --git a/java/lite/BUILD b/java/lite/BUILD
@@ -3,8 +3,15 @@ load("@rules_proto//proto:defs.bzl", "proto_lang_toolchain")
 load("//:internal.bzl", "conformance_test")
 load("//java/internal:testing.bzl", "junit_tests")
 
-exports_files(["lite.awk"], visibility = ["//java/core:__pkg__"])
-exports_files(["pom_template.xml"], visibility = ["//java/core:__pkg__"])
+exports_files(
+    ["lite.awk"],
+    visibility = ["//java/core:__pkg__"],
+)
+
+exports_files(
+    ["pom_template.xml"],
+    visibility = ["//java/core:__pkg__"],
+)
 
 alias(
     name = "lite",
@@ -22,8 +29,8 @@ proto_lang_toolchain(
 test_suite(
     name = "tests",
     tests = [
-        "lite_build_test",
         "conformance_test",
+        "lite_build_test",
         "lite_tests",
         "//java/core:lite_tests",
     ],
@@ -38,21 +45,21 @@ build_test(
 
 conformance_test(
     name = "conformance_test",
-    testee = "//:conformance_java_lite",
     failure_list = "//:conformance/failure_list_java_lite.txt",
+    testee = "//:conformance_java_lite",
     text_format_failure_list = "//:conformance/text_format_failure_list_java_lite.txt",
 )
 
 junit_tests(
     name = "lite_tests",
-    srcs = glob(["src/test/**/*.java"]),
     size = "small",
+    srcs = glob(["src/test/**/*.java"]),
     deps = [
         ":lite",
-        "//external:junit",
-        "//external:truth",
         "//java/core:generic_test_protos_java_proto_lite",
         "//java/core:java_test_protos_java_proto_lite",
         "//java/core:test_util_lite",
+        "@maven//:com_google_truth_truth",
+        "@maven//:junit_junit",
     ],
 )
diff --git a/java/util/BUILD b/java/util/BUILD
@@ -11,33 +11,34 @@ java_library(
     ]),
     visibility = ["//visibility:public"],
     deps = [
-        "//external:error_prone_annotations",
-        "//external:j2objc_annotations",
-        "//external:gson",
-        "//external:jsr305",
-        "//external:guava",
         "//java/core",
         "//java/lite",
+        "@maven//:com_google_code_findbugs_jsr305",
+        "@maven//:com_google_code_gson_gson",
+        "@maven//:com_google_errorprone_error_prone_annotations",
+        "@maven//:com_google_guava_guava",
+        "@maven//:com_google_j2objc_j2objc_annotations",
     ],
 )
+
 # Bazel users, don't depend on this target, use :util.
 java_export(
     name = "util_mvn",
     maven_coordinates = "com.google.protobuf:protobuf-java-util:%s" % PROTOBUF_VERSION,
     pom_template = "pom_template.xml",
-    runtime_deps = [":util"],
     visibility = ["//java:__pkg__"],
+    runtime_deps = [":util"],
 )
 
 filegroup(
     name = "release",
-    visibility = ["//java:__pkg__"],
     srcs = [
-        ":util_mvn-pom",
-        ":util_mvn-maven-source",
         ":util_mvn-docs",
+        ":util_mvn-maven-source",
+        ":util_mvn-pom",
         ":util_mvn-project",
-    ]
+    ],
+    visibility = ["//java:__pkg__"],
 )
 
 proto_library(
@@ -60,15 +61,15 @@ java_proto_library(
 
 junit_tests(
     name = "tests",
-    srcs = glob(["src/test/java/**/*.java"]),
     package_name = "com.google.protobuf.util",
+    srcs = glob(["src/test/java/**/*.java"]),
     deps = [
         ":test_protos_java_proto",
         ":util",
-        "//external:guava",
-        "//external:junit",
-        "//external:truth",
         "//java/core",
         "//java/core:generic_test_protos_java_proto",
+        "@maven//:com_google_guava_guava",
+        "@maven//:com_google_truth_truth",
+        "@maven//:junit_junit",
     ],
 )