Skip to content

Commit

Permalink
Align dependency handling with Bazel best practices (#9165)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@acozzette
acozzette authored Oct 28, 2021
1 parent 67c2a92 commit c7dfd0d
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 115 deletions.
63 changes: 8 additions & 55 deletions WORKSPACE
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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()
81 changes: 43 additions & 38 deletions java/core/BUILD
View file
Original file line number Diff line number Diff line change
@@ -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")
Expand Down Expand Up @@ -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__",
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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",
],
)
Expand All @@ -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(
Expand All @@ -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__",
Expand All @@ -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",
],
)

Expand Down Expand Up @@ -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",
],
)

21 changes: 14 additions & 7 deletions java/lite/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
],
Expand All @@ -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",
],
)
29 changes: 15 additions & 14 deletions java/util/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",

This comment has been minimized.

Sign in to view

Copy link
@suztomo

suztomo Nov 29, 2021 •
edited
Loading

Contributor

@acozzette GAX repository fails to resolve this @maven. Would you guide us how to upgrade the dependency here?

googleapis/gax-java#1571 (comment)

ERROR: /github/home/.cache/bazel/_bazel_root/9e4dde399fac32001b99efa92054490b/external/com_google_protobuf/java/util/BUILD:7:13: no such package '@maven//': The repository '@maven' could not be resolved and referenced by '@com_google_protobuf//java/util:util'
ERROR: /github/home/.cache/bazel/_bazel_root/9e4dde399fac32001b99efa92054490b/external/com_google_protobuf/java/util/BUILD:7:13: no such package '@maven//': The repository '@maven' could not be resolved and referenced by '@com_google_protobuf//java/util:util'
ERROR: /github/home/.cache/bazel/_bazel_root/9e4dde399fac32001b99efa92054490b/external/com_google_protobuf/java/util/BUILD:7:13: no such package '@maven//': The repository '@maven' could not be resolved and referenced by '@com_google_protobuf//java/util:util'
ERROR: /github/home/.cache/bazel/_bazel_root/9e4dde399fac32001b99efa92054490b/external/com_google_protobuf/java/util/BUILD:7:13: no such package '@maven//': The repository '@maven' could not be resolved and referenced by '@com_google_protobuf//java/util:util'
ERROR: /github/home/.cache/bazel/_bazel_root/9e4dde399fac32001b99efa92054490b/external/com_google_protobuf/java/util/BUILD:7:13: no such package '@maven//': The repository '@maven' could not be resolved and referenced by '@com_google_protobuf//java/util:util'

This comment has been minimized.

Sign in to view

Copy link
@suztomo

suztomo Nov 29, 2021

Contributor

It seems that I just needed to learn Bazel's maven_install. https://github.com/googleapis/gax-java/pull/1571/files#r758573894
Thank you for attention!
],
)

# 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(
Expand All @@ -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",
],
)
Loading

0 comments on commit c7dfd0d

Please sign in to comment.