ARROW-6331: [Java] Incorporate ErrorProne into the java build by tianchen92 · Pull Request #5263 · apache/arrow

tianchen92 · 2019-09-03T13:32:47Z

Related to ARROW-6331.
Using https://github.com/google/error-prone seems like it would be a good idea to automatically catch more errors.

tianchen92 · 2019-09-03T13:33:22Z

cc @emkornfield

java/memory/src/main/java/org/apache/arrow/memory/Accountant.java

java/pom.xml

praveenbingo · 2019-09-04T06:25:15Z

java/pom.xml

    </profile>
+
+    <!-- using github.com/google/error-prone-javac is required when running on JDK 8 -->
+    <profile>


i would request this to be the default please :)

@tianchen92
I was asking if anyone building arrow now needs to do so using java9 for this to work?
(or) can we continue using jdk8.

If we can continue running using jdk8, the change looks good.

If consumers need java9 to build - then i would request running error prone only if a specific profile is used (for e.g. in the example you have given using profile jdk1.8 activates the check with some special javac properties, meaning you would use a command like mvn -P jdk-18 to activate the profile) This way existing consumers will not be forced to upgrade their entire build chain to java9 to use arrow java.

I see, thanks for clarify, now it works fine with jdk8 :)

awesome..do we also want to modify ci to run this profile, so that it catches further bugs?

We don't need do anything, just use 'mvn clean install' will trigger error prone :)

sounds good thanks @tianchen92

java/pom.xml

codecov-io · 2019-09-05T13:44:56Z

Codecov Report

❗ No coverage uploaded for pull request base (master@c39e350). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #5263   +/-   ##
=========================================
  Coverage          ?    89.7%           
=========================================
  Files             ?      693           
  Lines             ?   104695           
  Branches          ?        0           
=========================================
  Hits              ?    93919           
  Misses            ?    10776           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c39e350...9cb3a86. Read the comment docs.

tianchen92 · 2019-09-07T03:23:52Z

cc @emkornfield for comments since you opened this issue.

emkornfield · 2019-09-07T03:39:34Z

Looks OK to me, I'm not an expert on setting this up, so if it looks ok to @praveenbingo I think we can merge

kou · 2019-09-09T08:31:05Z

CI on master is red since this merge: https://travis-ci.org/apache/arrow/builds/582523345
Could you take a look the failure log?

kou · 2019-09-09T08:32:17Z

Does the following message related to this merge?

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.6.2:compile (default-compile) on project arrow-avro: Compilation failure

[ERROR] /build/java/arrow/java/adapter/avro/src/main/java/org/apache/arrow/consumers/AvroBytesConsumer.java:[77,22] error: [SelfAssignment] Variable assigned to itself

[ERROR]     (see https://errorprone.info/bugpattern/SelfAssignment)

[ERROR]   Did you mean to remove this line?

[ERROR] -> [Help 1]

[ERROR] 

[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.

[ERROR] Re-run Maven using the -X switch to enable full debug logging.

[ERROR] 

[ERROR] For more information about the errors and possible solutions, please read the following articles:

[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

[ERROR] 

[ERROR] After correcting the problems, you can resume the build with the command

[ERROR]   mvn <goals> -rf :arrow-avro

praveenbingo · 2019-09-09T08:35:14Z

@kou Looks like it is related, will put a patch to resolve the same.

tianchen92 · 2019-09-09T08:35:28Z

CI on master is red since this merge: https://travis-ci.org/apache/arrow/builds/582523345
Could you take a look the failure log?

Does the following message related to this merge?

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.6.2:compile (default-compile) on project arrow-avro: Compilation failure

[ERROR] /build/java/arrow/java/adapter/avro/src/main/java/org/apache/arrow/consumers/AvroBytesConsumer.java:[77,22] error: [SelfAssignment] Variable assigned to itself

[ERROR]     (see https://errorprone.info/bugpattern/SelfAssignment)

[ERROR]   Did you mean to remove this line?

[ERROR] -> [Help 1]

[ERROR] 

[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.

[ERROR] Re-run Maven using the -X switch to enable full debug logging.

[ERROR] 

[ERROR] For more information about the errors and possible solutions, please read the following articles:

[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

[ERROR] 

[ERROR] After correcting the problems, you can resume the build with the command

[ERROR]   mvn <goals> -rf :arrow-avro

Does the following message related to this merge?

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.6.2:compile (default-compile) on project arrow-avro: Compilation failure

[ERROR] /build/java/arrow/java/adapter/avro/src/main/java/org/apache/arrow/consumers/AvroBytesConsumer.java:[77,22] error: [SelfAssignment] Variable assigned to itself

[ERROR]     (see https://errorprone.info/bugpattern/SelfAssignment)

[ERROR]   Did you mean to remove this line?

[ERROR] -> [Help 1]

[ERROR] 

[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.

[ERROR] Re-run Maven using the -X switch to enable full debug logging.

[ERROR] 

[ERROR] For more information about the errors and possible solutions, please read the following articles:

[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException

[ERROR] 

[ERROR] After correcting the problems, you can resume the build with the command

[ERROR]   mvn <goals> -rf :arrow-avro

Thanks for pointing, I guess yes, before this PR merged, there is another PR merged before, so error phone didn't catch the error, I'll take a look and provide a fix.

praveenbingo · 2019-09-09T08:36:23Z

@tianchen92 - ah looks like you are already on it..pls ping me to merge the same when its ready

tianchen92 · 2019-09-09T08:36:49Z

@kou Looks like it is related, will put a patch to resolve the same.

I just see that, I will fix this soon :)

kou · 2019-09-09T09:13:28Z

Thanks!

Related to [ARROW-6331](https://issues.apache.org/jira/browse/ARROW-6331). Using https://github.com/google/error-prone seems like it would be a good idea to automatically catch more errors. Closes apache#5263 from tianchen92/ARROW-6331 and squashes the following commits: 9cb3a86 <tianchen> fix pom properties bf66186 <tianchen> fix 3cbc00c <tianchen> Incorporate ErrorProne into the java build Authored-by: tianchen <niki.lj@alibaba-inc.com> Signed-off-by: Praveen <praveen@dremio.com>

[Java] Incorporate ErrorProne into the java build

3cbc00c

tianchen92 closed this Sep 4, 2019

tianchen92 reopened this Sep 4, 2019

praveenbingo reviewed Sep 4, 2019

View reviewed changes

fix

bf66186

praveenbingo reviewed Sep 5, 2019

View reviewed changes

java/pom.xml Outdated Show resolved Hide resolved

fix pom properties

9cb3a86

tianchen92 requested a review from praveenbingo September 6, 2019 08:56

praveenbingo approved these changes Sep 9, 2019

View reviewed changes

praveenbingo closed this in e9f35a8 Sep 9, 2019

asfimport mentioned this pull request Sep 9, 2019

[Java] Incorporate ErrorProne into the java build #22709

Closed

Conversation

tianchen92 commented Sep 3, 2019

Uh oh!

tianchen92 commented Sep 3, 2019

Uh oh!

Uh oh!

Uh oh!

praveenbingo Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

tianchen92 Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

praveenbingo Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

tianchen92 Sep 4, 2019

Choose a reason for hiding this comment

Uh oh!

praveenbingo Sep 5, 2019

Choose a reason for hiding this comment

Uh oh!

tianchen92 Sep 5, 2019

Choose a reason for hiding this comment

Uh oh!

praveenbingo Sep 9, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov-io commented Sep 5, 2019

Codecov Report

Uh oh!

tianchen92 commented Sep 7, 2019

Uh oh!

emkornfield commented Sep 7, 2019

Uh oh!

kou commented Sep 9, 2019

Uh oh!

kou commented Sep 9, 2019

Uh oh!

praveenbingo commented Sep 9, 2019

Uh oh!

tianchen92 commented Sep 9, 2019

Uh oh!

praveenbingo commented Sep 9, 2019

Uh oh!

tianchen92 commented Sep 9, 2019

Uh oh!

kou commented Sep 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants