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

Android: Remove Invoke Dynamic Call #631

Open
wants to merge 1 commit into
base: GROOVY_2_4_X
Choose a base branch
from

Conversation

AndrewReitz
Copy link
Contributor

Uses jarjar to remove org.codehaus.groovy.vmplugin.v7.IndyInterface
which has a invoke dynamic call. This call causes android compilation
to fail when targeting sdk level 26 or above but the minSdk is less
than 26.

Fixes: GROOVY-8366

Uses jarjar to remove org.codehaus.groovy.vmplugin.v7.IndyInterface
which has a invoke dynamic call. This call causes android compilation
to fail when targeting sdk level 26 or above but the minSdk is less
than 26.

Fixes: GROOVY-8366
@@ -247,6 +247,7 @@ allprojects {
if (isRootProject) {
zipfileset(src: rootProject.configurations.runtime.files.find { it.name.startsWith('openbeans') }, excludes: 'META-INF/*')
}
zap pattern: 'org.codehaus.groovy.vmplugin.v7.IndyInterface'
Copy link
Contributor

Choose a reason for hiding this comment

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

normally the Groovy version, that does not use indy, means indy is not used for Groovy itself, but the user can still use it. You pullrequest will mean that this is no longer possible. You have to at least also add the Java7 class, or else there will be class loading trouble. And the instantiation of that class would have to change. A bigger problem are the classes from org.codehaus.groovy.classgen.asm.indy, which do reference this class as well. Without further steps the groovy runtime and the compiler would get into trouble on a Java7 JVM or higher.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about having two android jars one without classes that use invoke dynamic and one with them still?

It looks like Android will be supporting it on ap's 26+. The issue is that Google added a check for classes using the invoke-polymorphic. Little more info here. https://issuetracker.google.com/issues/37567710

I am able to use proguard to remove the classes that cause the failure, but it increases build times greatly.

@chris-carneiro
Copy link

Hi,

Is there anything new to the matter ?
If I understand correctly, this issue blocks the evolution of the android-gradle-plugin

Is there anything we can do to help ?
We actually need an update on the plugin for our application to still be compatible with older versions of Android.

@blackdrag
Copy link
Contributor

This is actually a big problem. For Groovy on the JVM there is no other way, than to more and more depend on invokedynamic. I think the suggestion of an Android version of Groovy is the best. But I wonder if that is really more than - I am not into Android development, so be patient with my ignorance - using for example D8 to ensure a certain API level compatibility.

@chris-carneiro
Copy link

Thanks for the input.
To my knowledge and as you guys previously outlined, the problem is - To quote google's documentation -

Desugar currently doesn't support MethodHandle.invoke or MethodHandle.invokeExact. If your source code or one of your module dependencies use one of these methods you need to specify minSdkVersion 26 or higher. Otherwise, you get the following error [...]

So yeah I guess D8 won't help, you're probably right on the best option suggestion.
How hard would it be to implement an android version of Groovy, I'm really not familiar with the ways to port libraries.

@blackdrag
Copy link
Contributor

It could be as easy as using retrolambda to transform the Groovy library. Then, as long as you use pre Groovy 3 it should work - maybe even for classes generated with Groovy 3. For later versions you may have to transform the classes outputted by Groovy. But I strongly assume you are not trying to run scripts anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants