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.

Sign up for GitHub

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

Jump to bottom

Demote severity of ExtendsAutoValue and RefersToDaggerCodegen to a warning to not break Java cross compilation in Bazel #1619

Closed
davido opened this issue May 17, 2020 · 1 comment
Closed

Demote severity of ExtendsAutoValue and RefersToDaggerCodegen to a warning to not break Java cross compilation in Bazel #1619

davido opened this issue May 17, 2020 · 1 comment

Comments

@davido
Copy link
Contributor

davido commented May 17, 2020 •
edited
Loading

Both checks appear to be a breaking changes for Bazel EP integration.

It broke all Bazel users who rely on cross compilation: [1], [2], and was the reason, why the first attempt to bump EP version to 2.3.3 in Bazel was reverted: [3], [4].

The way Bazel cross compilation works is a bit convoluted, but the reason for false positive boils down to: the missing annotation generation is using -source 8 -target 8 with a JDK 11 -bootclasspath (because it's targeting the remote JDK 11). So AutoValue processor is trying to use javax.annotation.Generated based on the source level, but it isn't available because the JDK 11 bootclasspath only contains javax.annotation.processing.Generated.

Demote the severity to warning for the checks that rely on having
JDK 8's: javax.annotation.Generated or JDK 11's javax.annotation.processing.Generated on the classpath today, and consider to promote it to error severity again later, when Java 8 is gone.

That would substantially simplify the adoption of new EP releases, because it wouldn't require the addition of javax.annotation library to the build tool chain.

[1] bazelbuild/rules_closure#483
[2] bazelbuild/rules_kotlin#321
[3] bazelbuild/bazel@d990746
[4] bazelbuild/bazel#10023
@davido davido mentioned this issue May 17, 2020
Closed
davido added a commit to davido/error-prone that referenced this issue May 17, 2020
@davido
Demote severity of ExtendsAutoValue and RefersToDaggerCodegen to a wa…
e4354c8 
…rning

Fixes: google#1619.

Both checks appear to be a breaking changes for Bazel EP integration.

The way Bazel cross compilation works is a bit convoluted, but the
reason for false positive boils down to: the missing annotation
generation is using -source 8 -target 8 with a JDK 11 -bootclasspath
(because it's targeting the remote JDK 11). So AutoValue processor is
trying to use javax.annotation.Generated based on the source level, but
it isn't available because the JDK 11 bootclasspath only contains
javax.annotation.processing.Generated.

Demote the severity to warning for the checks that rely on having JDK
8's: javax.annotation.Generated or JDK 11's
javax.annotation.processing.Generated on the classpath today, and
consider to promote it to error again later, when Java 8 is gone.

That would substantially simplify the adoption of new EP releases,
because it wouldn't require the addition of javax.annotation library
to the build tool chain.
This was referenced May 17, 2020
Closed
Open
Closed
davido added a commit to davido/bazel that referenced this issue May 19, 2020
@davido
Add incompatible_use_java_tools_beta_release option
aab45e0 
Fixes bazelbuild#11446.

Java tools are decoupled from Bazel distribution and downloaded from
java_tools repository.

If some of the tools is updated, the archive name is changed in Bazel
and new Bazel release is conducted. If, however, new tool has some
breaking changes, then the upgrade needs to be rolled back.

To make user experience with upgrading tools less disruptive but still
not unnecessary procrastinate releases of important Java tools like
javac and Error Prone --incompatible_use_java_tools_beta_release is
added and CI-job should be set up to check all downstream projects
against upcoming java_tools release.

Test Plan:

1. Upgrade Error Prone version to 2.3.4 Bazel that is known to have
breaking changes and will break all Bazel users who rely on cross
compilation use case. There are ongoing efforts to demote the offensive
EP checks to warning severity instead of error, though: [1].

2. Bump java_tools in beta_release archive to new java_tools
   distribution that includes Error Prone 2.3.4

3. Build rules_closure with --incompatible_use_java_tools_beta_release
   would fail with the known issue, because of unsatisfied dependency
   on javax.annotation:

ERROR: /home/davido/projects/rules_closoure/java/io/bazel/rules/closure/BUILD:41:13: Building java/io/bazel/rules/closure/libtarjan.jar (1 source file) and running annotation processors (AutoAnnotationProcessor, AutoOneOfProcessor, AutoValueProcessor) failed (Exit 1)
bazel-out/host/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_sourcegenfiles/io/bazel/rules/closure/AutoValue_Tarjan_Result.java:6: error: [ExtendsAutoValue] Do not extend an @AutoValue/@AutoOneOf class in non-generated code.
final class AutoValue_Tarjan_Result<V> extends Tarjan.Result<V> {
      ^
    (see https://errorprone.info/bugpattern/ExtendsAutoValue)

See also upstream issue with in depth explanation what is going on
there: [2]

4. Build rules_closure without the incompatible option: all is fine.
   That would mean, that all broken downstream projects would have
   enough time to adapt their build tool chains for upgraded
   dependencies in java_tools distribution.

[1] google/error-prone#1619
[2] bazelbuild/rules_closure#483

Change-Id: I352297a8d0d86cecf30821c132c7871383e49b50
@davido davido mentioned this issue May 19, 2020
Closed
@cushon
Copy link
Collaborator

cushon commented May 20, 2020

#1348

@cushon cushon closed this as completed May 20, 2020
davido added a commit to davido/bazel that referenced this issue May 21, 2020
@davido
Add incompatible_use_java_tools_beta_release option
a5de695 
Fixes bazelbuild#11446.

Java tools are decoupled from Bazel distribution and downloaded from
java_tools repository.

If some of the tools is updated, the archive name is changed in Bazel
and new Bazel release is conducted. If, however, new tool has some
breaking changes, then the upgrade needs to be rolled back.

To make user experience with upgrading tools less disruptive but still
not unnecessary procrastinate releases of important Java tools like
javac and Error Prone --incompatible_use_java_tools_beta_release is
added and CI-job should be set up to check all downstream projects
against upcoming java_tools release.

Test Plan:

1. Upgrade Error Prone version to 2.3.4 Bazel that is known to have
breaking changes and will break all Bazel users who rely on cross
compilation use case. There are ongoing efforts to demote the offensive
EP checks to warning severity instead of error, though: [1].

2. Bump java_tools in beta_release archive to new java_tools
   distribution that includes Error Prone 2.3.4

3. Build rules_closure with --incompatible_use_java_tools_beta_release
   would fail with the known issue, because of unsatisfied dependency
   on javax.annotation:

ERROR: /home/davido/projects/rules_closoure/java/io/bazel/rules/closure/BUILD:41:13: Building java/io/bazel/rules/closure/libtarjan.jar (1 source file) and running annotation processors (AutoAnnotationProcessor, AutoOneOfProcessor, AutoValueProcessor) failed (Exit 1)
bazel-out/host/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_sourcegenfiles/io/bazel/rules/closure/AutoValue_Tarjan_Result.java:6: error: [ExtendsAutoValue] Do not extend an @AutoValue/@AutoOneOf class in non-generated code.
final class AutoValue_Tarjan_Result<V> extends Tarjan.Result<V> {
      ^
    (see https://errorprone.info/bugpattern/ExtendsAutoValue)

See also upstream issue with in depth explanation what is going on
there: [2]

4. Build rules_closure without the incompatible option: all is fine.
   That would mean, that all broken downstream projects would have
   enough time to adapt their build tool chains for upgraded
   dependencies in java_tools distribution.

[1] google/error-prone#1619
[2] bazelbuild/rules_closure#483

Change-Id: I352297a8d0d86cecf30821c132c7871383e49b50
davido added a commit to davido/bazel that referenced this issue May 21, 2020
@davido
Add incompatible_use_java_tools_beta_release option
8727648 
Fixes bazelbuild#11446.

Java tools are decoupled from Bazel distribution and downloaded from
java_tools repository.

If some of the tools is updated, the archive name is changed in Bazel
and new Bazel release is conducted. If, however, new tool has some
breaking changes, then the upgrade needs to be rolled back.

To make user experience with upgrading tools less disruptive but still
not unnecessary procrastinate releases of important Java tools like
javac and Error Prone --incompatible_use_java_tools_beta_release is
added and CI-job should be set up to check all downstream projects
against upcoming java_tools release.

Test Plan:

1. Upgrade Error Prone version to 2.3.4 Bazel that is known to have
breaking changes and will break all Bazel users who rely on cross
compilation use case. There are ongoing efforts to demote the offensive
EP checks to warning severity instead of error, though: [1].

2. Bump java_tools in beta_release archive to new java_tools
   distribution that includes Error Prone 2.3.4

3. Build rules_closure with --incompatible_use_java_tools_beta_release
   would fail with the known issue, because of unsatisfied dependency
   on javax.annotation:

ERROR: /home/davido/projects/rules_closoure/java/io/bazel/rules/closure/BUILD:41:13: Building java/io/bazel/rules/closure/libtarjan.jar (1 source file) and running annotation processors (AutoAnnotationProcessor, AutoOneOfProcessor, AutoValueProcessor) failed (Exit 1)
bazel-out/host/bin/java/io/bazel/rules/closure/_javac/tarjan/libtarjan_sourcegenfiles/io/bazel/rules/closure/AutoValue_Tarjan_Result.java:6: error: [ExtendsAutoValue] Do not extend an @AutoValue/@AutoOneOf class in non-generated code.
final class AutoValue_Tarjan_Result<V> extends Tarjan.Result<V> {
      ^
    (see https://errorprone.info/bugpattern/ExtendsAutoValue)

See also upstream issue with in depth explanation what is going on
there: [2]

4. Build rules_closure without the incompatible option: all is fine.
   That would mean, that all broken downstream projects would have
   enough time to adapt their build tool chains for upgraded
   dependencies in java_tools distribution.

[1] google/error-prone#1619
[2] bazelbuild/rules_closure#483

Change-Id: I352297a8d0d86cecf30821c132c7871383e49b50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Demote severity of ExtendsAutoValue and RefersToDaggerCodegen to a wa… davido/error-prone
2 participants
@davido @cushon