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

[SPARK-28221][BUILD] Upgrade janino to 3.0.13 #25021

Closed
wants to merge 1 commit into from
Closed

[SPARK-28221][BUILD] Upgrade janino to 3.0.13 #25021

wants to merge 1 commit into from

Conversation

wangyum
Copy link
Member

@wangyum wangyum commented Jul 1, 2019

What changes were proposed in this pull request?

Mainly change logs:

Version 3.0.13:

  • Support for JDK 9/10 in Full Compiler
  • The syntax elements that can have modifiers now all have sets of "is...()" methods that check for each modifier. Some also have methods "getAccess()" and/or "getAnnotations()".
  • Implement "type annotations" (JLS8 9.7.4)
  • Implemented parsing (but not compilation) of "modular compilation units" (JLS11 7.3).
  • Replaced all "assert...Uncookable(..., Pattern messageRegex)" and "assert...Uncookable(..., String messageInfix)" method pairs with a single "assert...Uncookable(..., String messageRegex)" method.
    Minor refactoring: Allowed modifiers are now checked in the Parser, not in Java.*. This saves a lot of THROWS clauses.
  • Parse Type inference syntax: Type inference for generic instance creation implemented, test cases added.
  • Parse MethodReference, ClassInstanceCreationReference and ArrayCreationReference

Version 3.0.12

  • Fixed: Operator "&" not defined on types "java.lang.Long" and "int"
  • Major bug in JavaSourceClassLoader: When loading the second and following classes, CUs were compiled again, leading to an inconsistent class hierarchy.
  • Fixed: Java 9 added "@OverRide public final CharBuffer CharBuffer.rewind() { ..." -- leads easily to a java.lang.NoSuchMethodError
  • Changed all occurences of the words "Java bytecode" to "JVM bytecode" to make clearer that the generated bytecode is for the JVMS and not suitable for, e.g. DALVIK.

http://janino-compiler.github.io/janino/changelog.html

How was this patch tested?

Existing test

@SparkQA
Copy link

SparkQA commented Jul 1, 2019

Test build #107066 has finished for PR 25021 at commit 0f56765.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member

maropu commented Jul 1, 2019

Just a question: skips 3.0.12? It seems the change logs for 3.0.13 are listed in the description.

@wangyum
Copy link
Member Author

wangyum commented Jul 1, 2019

Thank you @maropu I added some change logs for 3.0.12.

@kiszk
Copy link
Member

kiszk commented Jul 1, 2019

Looks good to me

One question. Do you wait for confirming the fix of this issue? @JoshRosen
janino-compiler/janino#90

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay. +1, LGTM, too. So, we are waiting @JoshRosen 's confirmation about janino-compiler/janino#90, right?

@dongjoon-hyun
Copy link
Member

Retest this please.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Jul 5, 2019

Gentle ping, @JoshRosen . It would be really great if we can get a confirmation from you.

@SparkQA
Copy link

SparkQA commented Jul 5, 2019

Test build #107290 has finished for PR 25021 at commit 0f56765.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@JoshRosen
Copy link
Contributor

I haven't had a chance to confirm the Janino bugfix yet, but I don't think that's a blocker to merging this version bump: AFAIK we're not relying on the patched functionality yet.

@dongjoon-hyun
Copy link
Member

I see. Great. Thank you so much, @JoshRosen .
Merged to master.

Also, Thank you, @wangyum , @kiszk , @srowen , @maropu , @HyukjinKwon !

@wangyum wangyum deleted the SPARK-28221 branch July 6, 2019 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants