Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java record components are not exposed to annotation processors during header JAR compilation #23246

Closed
anderswisch opened this issue Aug 9, 2024 · 6 comments
Labels
team-Rules-Java Issues for Java rules type: bug

Comments

@anderswisch
Copy link

Description of the bug:

record-builder, an annotation processor that generates code for Java records, produces incorrect output when it runs in the processing environment Bazel uses to generate header JARs. For example, the API for a record's generated builder in a header JAR will be missing all methods related to the source record's components.

The processing environment used to generate header JARs seems to be Turbine 0.6.0 in the latest version of Bazel (7.2.1). Record Builder uses TypeElement.getRecordComponents() to access record components, but Turbine returns an empty list from this method (because it does not implement it). Record Builder also uses RecordComponentElement to read annotations on the record component's accessor, but Turbine does not seem to be able to return this type.

Which category does this issue belong to?

Java Rules

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Modifying the java-maven example:

diff --git a/java-maven/.bazelrc b/java-maven/.bazelrc
index 3ce91d2..a78dc5a 100644
--- a/java-maven/.bazelrc
+++ b/java-maven/.bazelrc
@@ -1 +1,5 @@
 common --enable_bzlmod
+build --java_language_version=17
+build --tool_java_language_version=17
+build --java_runtime_version=remotejdk_17
+build --tool_java_runtime_version=remotejdk_17
diff --git a/java-maven/.bazelversion b/java-maven/.bazelversion
index a3fcc71..b26a34e 100644
--- a/java-maven/.bazelversion
+++ b/java-maven/.bazelversion
@@ -1 +1 @@
-7.1.0
+7.2.1
diff --git a/java-maven/BUILD b/java-maven/BUILD
index 8e67443..b99b6f0 100644
--- a/java-maven/BUILD
+++ b/java-maven/BUILD
@@ -1,6 +1,6 @@
 load("@aspect_bazel_lib//lib:tar.bzl", "tar")
 load("@container_structure_test//:defs.bzl", "container_structure_test")
-load("@rules_java//java:defs.bzl", "java_binary", "java_library", "java_test")
+load("@rules_java//java:defs.bzl", "java_binary", "java_library", "java_plugin", "java_test")
 load("@rules_oci//oci:defs.bzl", "oci_image")
 
 package(default_visibility = ["//visibility:public"])
@@ -8,7 +8,29 @@ package(default_visibility = ["//visibility:public"])
 java_library(
     name = "java-maven-lib",
     srcs = glob(["src/main/java/com/example/myproject/*.java"]),
-    deps = ["@maven//:com_google_guava_guava"],
+    deps = [
+        "@maven//:com_google_guava_guava",
+        ":recordbuilder",
+    ],
+)
+
+java_library(
+    name = "recordbuilder",
+    exported_plugins = [
+        "recordbuilder_processor",
+    ],
+    exports = [
+        "@maven//:io_soabase_record_builder_record_builder_core",
+    ],
+)
+
+java_plugin(
+    name = "recordbuilder_processor",
+    generates_api = True,
+    processor_class = "io.soabase.recordbuilder.processor.RecordBuilderProcessor",
+    deps = [
+        "@maven//:io_soabase_record_builder_record_builder_processor",
+    ],
 )
 
 java_binary(
diff --git a/java-maven/MODULE.bazel b/java-maven/MODULE.bazel
index f87a63b..7e2768b 100644
--- a/java-maven/MODULE.bazel
+++ b/java-maven/MODULE.bazel
@@ -10,6 +10,8 @@ maven.install(
     artifacts = [
         "junit:junit:4.12",
         "com.google.guava:guava:28.0-jre",
+        "io.soabase.record-builder:record-builder-processor:42",
+        "io.soabase.record-builder:record-builder-core:42",
     ],
     fetch_sources = True,
     repositories = [
diff --git a/java-maven/src/main/java/com/example/myproject/TestRecord.java b/java-maven/src/main/java/com/example/myproject/TestRecord.java
new file mode 100644
index 0000000..f57541e
--- /dev/null
+++ b/java-maven/src/main/java/com/example/myproject/TestRecord.java
@@ -0,0 +1,6 @@
+package com.example.myproject;
+
+import io.soabase.recordbuilder.core.RecordBuilderFull;
+
+@RecordBuilderFull
+public record TestRecord(int a, String b) {}
diff --git a/java-maven/src/test/java/com/example/myproject/TestRecordBuilderTest.java b/java-maven/src/test/java/com/example/myproject/TestRecordBuilderTest.java
new file mode 100644
index 0000000..34e5a4d
--- /dev/null
+++ b/java-maven/src/test/java/com/example/myproject/TestRecordBuilderTest.java
@@ -0,0 +1,12 @@
+package com.example.myproject;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+public class TestRecordBuilderTest {
+  @Test
+  public void build() {
+    assertEquals(new TestRecord(1, "2"), TestRecordBuilder.builder().a(1).b("2").build());
+  }
+}

Then running:

bazel build :tests

Produces error:

ERROR: [path-to-project]/examples/java-maven/BUILD:42:10: Building tests.jar (2 source files) failed: (Exit 1): java failed: error executing Javac command (from target //:tests) external/rules_java~~toolchains~remotejdk21_macos_aarch64/bin/java '--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED' '--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED' ... (remaining 19 arguments skipped)
src/test/java/com/example/myproject/TestRecordBuilderTest.java:10: error: cannot find symbol
    assertEquals(new TestRecord(1, "2"), TestRecordBuilder.builder().a(1).b("2").build());
                                                                    ^
  symbol:   method a(int)
  location: class TestRecordBuilder
Target //:tests failed to build

Which operating system are you running Bazel on?

macOS 14.5 (23F79)

What is the output of bazel info release?

release 7.2.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

If this is a regression, please try to identify the Bazel commit where the bug was introduced with bazelisk --bisect.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@fmeum
Copy link
Collaborator

fmeum commented Aug 9, 2024

@cushon

@cushon
Copy link
Contributor

cushon commented Aug 9, 2024

I have an initial fix that I verified against the original repro (using the steps in bazelbuild/java_tools#43 (comment), if anyone knows a better way to have tested that I'm all ears):

$ /tmp/bazel build :tests
...
src/test/java/com/example/myproject/TestRecordBuilderTest.java:10: error: cannot find symbol
    assertEquals(new TestRecord(1, "2"), TestRecordBuilder.builder().a(1).b("2").build());
                                                                    ^
  symbol:   method a(int)
  location: class TestRecordBuilder
$ /tmp/bazel build --override_repository=rules_java++toolchains+remote_java_tools=/tmp/java_tools :tests
...
Build completed successfully

One complication is that the easiest way to support those APIs in turbine is to raise the minimum supported JDK version to JDK 17. I'm not sure if that's fine at this point, or if that will mean using older versions of turbine in some configurations.

@fmeum
Copy link
Collaborator

fmeum commented Aug 9, 2024

One complication is that the easiest way to support those APIs in turbine is to raise the minimum supported JDK version to JDK 17. I'm not sure if that's fine at this point, or if that will mean using older versions of turbine in some configurations.

Since this would only affect the JDK used for compilation, I wouldn't consider this a problem. But if you have concerns, I could work out the diff required to keep Turbine compatible with Java 8.

@cushon
Copy link
Contributor

cushon commented Aug 9, 2024

Thanks, I don't have a current sense of which JDK versions people are relying on for compilation, I will try raising the min JDK for turbine and see how that goes.

If it came to it, one option would be to create a Java 8-compatible branch of turbine and then backport any essential bug fixes to it. Most of the recent changes have been related to newer language features, I don't think many changes will be required for Java 8 support.

copybara-service bot pushed a commit that referenced this issue Aug 27, 2024
https://github.com/google/turbine/releases/tag/v0.7.0

```
REPIN=1 bazelisk run @unpinned_maven//:pin && bazelisk mod deps --lockfile_mode=update
```

#23246

Closes #23430.

PiperOrigin-RevId: 667834396
Change-Id: I7f620651870e1ac055654ec5000e942b48875d15
@hvadehra
Copy link
Member

@cushon Is there anything else we need for this issue?

@cushon
Copy link
Contributor

cushon commented Sep 20, 2024

I don't think so, getRecordComponents support is fixed in turbine, so this should be resolved in java_tools versions that include 39a6340

I also took a pass over other recently added annotation processing APIs and made a couple of other fixes that didn't make it into 0.7.0, but that's distinct from getRecordComponents (google/turbine@171153d, google/turbine@6574ba5)

@cushon cushon closed this as completed Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-Java Issues for Java rules type: bug
Projects
None yet
Development

No branches or pull requests

7 participants