From 1456462a143efa5b8459123ef990071f9221c119 Mon Sep 17 00:00:00 2001 From: Deanna Garcia Date: Fri, 24 Feb 2023 20:59:18 +0000 Subject: [PATCH 1/7] Add java8 tests and build flags --- .github/workflows/test_java.yml | 8 ++++++-- java/core/BUILD.bazel | 18 +++++++++--------- java/util/BUILD.bazel | 6 +++--- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/.github/workflows/test_java.yml b/.github/workflows/test_java.yml index 1ad65537a1..313308cb13 100644 --- a/.github/workflows/test_java.yml +++ b/.github/workflows/test_java.yml @@ -14,13 +14,17 @@ jobs: fail-fast: false matrix: include: + - name: OpenJDK 8 + version: '8' + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/bazel:8-03a376b5d6ef66f827fc307716e3b841cc26b709 + targets: //java/... //java/internal:java_version - name: OpenJDK 11 version: '11' - image: us-docker.pkg.dev/protobuf-build/containers/common/linux/bazel:5.1.1-aec4d74f2eb6938fc53ef7d9a79a4bf2da24abc1 + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/bazel:11-03a376b5d6ef66f827fc307716e3b841cc26b709 targets: //java/... //java/internal:java_version - name: OpenJDK 17 version: '17' - image: us-docker.pkg.dev/protobuf-build/containers/test/linux/java:17-65526ea124d1034eac33e7c37cc6d65c5bef054f + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/java:17-03a376b5d6ef66f827fc307716e3b841cc26b709 targets: //java/... //java/internal:java_version - name: aarch64 version: 'aarch64' diff --git a/java/core/BUILD.bazel b/java/core/BUILD.bazel index c0e52a4ff4..f08cf4a702 100644 --- a/java/core/BUILD.bazel +++ b/java/core/BUILD.bazel @@ -1,8 +1,8 @@ load("@bazel_skylib//rules:build_test.bzl", "build_test") -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_java//java:defs.bzl", "java_lite_proto_library", "java_proto_library") load("@rules_pkg//:mappings.bzl", "pkg_files", "strip_prefix") load("@rules_proto//proto:defs.bzl", "proto_lang_toolchain", "proto_library") +load("//build_defs:java_opts.bzl", "protobuf_java_export", "protobuf_java_library") load("//conformance:defs.bzl", "conformance_test") load("//:protobuf.bzl", "internal_gen_well_known_protos_java") load("//:protobuf_version.bzl", "PROTOBUF_JAVA_VERSION") @@ -121,7 +121,7 @@ internal_gen_well_known_protos_java( ) # Should be used as `//java/lite`. -java_library( +protobuf_java_library( name = "lite", srcs = LITE_SRCS + [ ":gen_well_known_protos_javalite", @@ -132,7 +132,7 @@ java_library( ) # Bazel users, don't depend on this target, use //java/lite. -java_export( +protobuf_java_export( name = "lite_mvn", maven_coordinates = "com.google.protobuf:protobuf-javalite:%s" % PROTOBUF_JAVA_VERSION, pom_template = "//java/lite:pom_template.xml", @@ -143,7 +143,7 @@ java_export( runtime_deps = [":lite"], ) -java_library( +protobuf_java_library( name = "lite_runtime_only", srcs = LITE_SRCS, ) @@ -166,7 +166,7 @@ internal_gen_well_known_protos_java( ], ) -java_library( +protobuf_java_library( name = "core", srcs = glob( [ @@ -186,7 +186,7 @@ java_library( ) # Bazel users, don't depend on this target, use :core. -java_export( +protobuf_java_export( name = "core_mvn", maven_coordinates = "com.google.protobuf:protobuf-java:%s" % PROTOBUF_JAVA_VERSION, pom_template = "pom_template.xml", @@ -269,7 +269,7 @@ java_proto_library( deps = [":java_test_protos"], ) -java_library( +protobuf_java_library( name = "test_util", srcs = [ "src/test/java/com/google/protobuf/TestUtil.java", @@ -390,7 +390,7 @@ genrule( cmd = "awk -f $(location //java/lite:lite.awk) $(location src/test/java/com/google/protobuf/TestUtil.java) > $@", ) -java_library( +protobuf_java_library( name = "test_util_lite", srcs = [ "src/test/java/com/google/protobuf/TestUtilLite.java", diff --git a/java/util/BUILD.bazel b/java/util/BUILD.bazel index e3804fb978..3d042d390a 100644 --- a/java/util/BUILD.bazel +++ b/java/util/BUILD.bazel @@ -1,11 +1,11 @@ load("@rules_java//java:defs.bzl", "java_proto_library") -load("@rules_jvm_external//:defs.bzl", "java_export") load("@rules_pkg//:mappings.bzl", "pkg_filegroup", "pkg_files", "strip_prefix") load("@rules_proto//proto:defs.bzl", "proto_library") +load("//build_defs:java_opts.bzl", "protobuf_java_export", "protobuf_java_library") load("//:protobuf_version.bzl", "PROTOBUF_JAVA_VERSION") load("//java/internal:testing.bzl", "junit_tests") -java_library( +protobuf_java_library( name = "util", srcs = glob([ "src/main/java/com/google/protobuf/util/*.java", @@ -22,7 +22,7 @@ java_library( ) # Bazel users, don't depend on this target, use :util. -java_export( +protobuf_java_export( name = "util_mvn", deploy_env = ["//java/core"], maven_coordinates = "com.google.protobuf:protobuf-java-util:%s" % PROTOBUF_JAVA_VERSION, From 340d24ce8a420919521e6d124cab81f502b47be3 Mon Sep 17 00:00:00 2001 From: Deanna Garcia Date: Fri, 24 Feb 2023 21:12:24 +0000 Subject: [PATCH 2/7] Add java opts file --- build_defs/java_opts.bzl | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 build_defs/java_opts.bzl diff --git a/build_defs/java_opts.bzl b/build_defs/java_opts.bzl new file mode 100644 index 0000000000..b6ee8142ba --- /dev/null +++ b/build_defs/java_opts.bzl @@ -0,0 +1,21 @@ +"""Java options and protobuf-specific java build rules with those options.""" +load("@rules_java//java:defs.bzl", "java_library") +load("@rules_jvm_external//:defs.bzl", "java_export") + +JAVA_OPTS = [ + "-source 8", + "-target 8", + "-Xep:Java8ApiChecker:ERROR", +] + +def protobuf_java_export(**kwargs): + java_export( + javacopts = JAVA_OPTS, + **kwargs, + ) + +def protobuf_java_library(**kwargs): + java_library( + javacopts = JAVA_OPTS, + **kwargs, + ) From c245167528d4ab5e1bcec4a2a7eef9e315858400 Mon Sep 17 00:00:00 2001 From: Deanna Garcia Date: Fri, 24 Feb 2023 21:27:05 +0000 Subject: [PATCH 3/7] Fix docker image --- .github/workflows/test_java.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test_java.yml b/.github/workflows/test_java.yml index 313308cb13..b415fdef4a 100644 --- a/.github/workflows/test_java.yml +++ b/.github/workflows/test_java.yml @@ -16,11 +16,11 @@ jobs: include: - name: OpenJDK 8 version: '8' - image: us-docker.pkg.dev/protobuf-build/containers/test/linux/bazel:8-03a376b5d6ef66f827fc307716e3b841cc26b709 + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/java:8-03a376b5d6ef66f827fc307716e3b841cc26b709 targets: //java/... //java/internal:java_version - name: OpenJDK 11 version: '11' - image: us-docker.pkg.dev/protobuf-build/containers/test/linux/bazel:11-03a376b5d6ef66f827fc307716e3b841cc26b709 + image: us-docker.pkg.dev/protobuf-build/containers/test/linux/java:11-03a376b5d6ef66f827fc307716e3b841cc26b709 targets: //java/... //java/internal:java_version - name: OpenJDK 17 version: '17' From 27c73191060f2d2bc4e207610b674e1926ac7ad2 Mon Sep 17 00:00:00 2001 From: Deanna Garcia Date: Fri, 24 Feb 2023 22:29:04 +0000 Subject: [PATCH 4/7] Fix java version test --- java/internal/JavaVersionTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/java/internal/JavaVersionTest.java b/java/internal/JavaVersionTest.java index c7957c0d81..af01227bee 100644 --- a/java/internal/JavaVersionTest.java +++ b/java/internal/JavaVersionTest.java @@ -40,12 +40,14 @@ public class JavaVersionTest { @Test public void testJavaVersion() throws Exception { String exp = System.getenv("KOKORO_JAVA_VERSION"); + // Java 8's version is read as "1.8" + if(exp == "8") exp = "1.8"; if(exp == null || exp.isEmpty()) { System.err.println("No kokoro java version found, skipping check"); return; } String version = System.getProperty("java.version"); - assertWithMessage("Expected Python " + exp + " but found Python " + version) + assertWithMessage("Expected Java " + exp + " but found Java " + version) .that(version.startsWith(exp)) .isTrue(); } From 5ffb29d684639df5571495996fd46f6df28e0d3c Mon Sep 17 00:00:00 2001 From: deannagarcia <69992229+deannagarcia@users.noreply.github.com> Date: Fri, 24 Feb 2023 14:44:31 -0800 Subject: [PATCH 5/7] Debug java version test --- java/internal/JavaVersionTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/java/internal/JavaVersionTest.java b/java/internal/JavaVersionTest.java index af01227bee..320ca3d892 100644 --- a/java/internal/JavaVersionTest.java +++ b/java/internal/JavaVersionTest.java @@ -41,7 +41,12 @@ public class JavaVersionTest { public void testJavaVersion() throws Exception { String exp = System.getenv("KOKORO_JAVA_VERSION"); // Java 8's version is read as "1.8" - if(exp == "8") exp = "1.8"; + if(exp == "8") { + System.out.println("The version was 8"); + exp = "1.8"; + } else { + System.out.println("The version was not 8, it was: " + exp); + } if(exp == null || exp.isEmpty()) { System.err.println("No kokoro java version found, skipping check"); return; From 075677e415f215b63267269f681617299542d864 Mon Sep 17 00:00:00 2001 From: deannagarcia <69992229+deannagarcia@users.noreply.github.com> Date: Fri, 24 Feb 2023 14:53:36 -0800 Subject: [PATCH 6/7] Compare strings better --- java/internal/JavaVersionTest.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/java/internal/JavaVersionTest.java b/java/internal/JavaVersionTest.java index 320ca3d892..4ee6e97e12 100644 --- a/java/internal/JavaVersionTest.java +++ b/java/internal/JavaVersionTest.java @@ -41,12 +41,7 @@ public class JavaVersionTest { public void testJavaVersion() throws Exception { String exp = System.getenv("KOKORO_JAVA_VERSION"); // Java 8's version is read as "1.8" - if(exp == "8") { - System.out.println("The version was 8"); - exp = "1.8"; - } else { - System.out.println("The version was not 8, it was: " + exp); - } + if(exp.equals("8")) exp = "1.8"; if(exp == null || exp.isEmpty()) { System.err.println("No kokoro java version found, skipping check"); return; From ae97c826f7d865bb97c4addeecc113699d42a222 Mon Sep 17 00:00:00 2001 From: deannagarcia <69992229+deannagarcia@users.noreply.github.com> Date: Fri, 24 Feb 2023 15:02:53 -0800 Subject: [PATCH 7/7] Fix potential null pointer error --- java/internal/JavaVersionTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/internal/JavaVersionTest.java b/java/internal/JavaVersionTest.java index 4ee6e97e12..c9ea0a504b 100644 --- a/java/internal/JavaVersionTest.java +++ b/java/internal/JavaVersionTest.java @@ -40,12 +40,12 @@ public class JavaVersionTest { @Test public void testJavaVersion() throws Exception { String exp = System.getenv("KOKORO_JAVA_VERSION"); - // Java 8's version is read as "1.8" - if(exp.equals("8")) exp = "1.8"; if(exp == null || exp.isEmpty()) { System.err.println("No kokoro java version found, skipping check"); return; } + // Java 8's version is read as "1.8" + if(exp.equals("8")) exp = "1.8"; String version = System.getProperty("java.version"); assertWithMessage("Expected Java " + exp + " but found Java " + version) .that(version.startsWith(exp))