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

R8 task fails with missing StringConcatFactory when using AGP 8.0 #2145

Closed
Nek-12 opened this issue Jan 4, 2023 · 33 comments
Closed

R8 task fails with missing StringConcatFactory when using AGP 8.0 #2145

Nek-12 opened this issue Jan 4, 2023 · 33 comments
Assignees

Comments

@Nek-12
Copy link

Nek-12 commented Jan 4, 2023

Describe the bug
I've had an issue with R8 task failing because the StringConcatFactory rule was missing in my proguard-rules.pro.
Agp now treats missing classes as errors since 8.0, and kotlinx.serialization utilizes StringConcatFactory which R8 then is unable to find because it's stripped from the actual code (sanitized away when targeting jvm 8+)
Please find exact details of the discussion in the following topic https://issuetracker.google.com/issues/250197571#comment19
The conclusion me and the Google team arrived at is that libraries should specify a -dontwarn rule for such cases.
This ticket is therefore a request to add the missing rules.

To Reproduce
Compile a library android module that uses kotlinx.serialization and targets jdk 11 on AGP 8.0-alpha. Compilation will fail.

Expected behavior

Environment

  • Kotlin version: N/A
  • Library version: 1.4.0
  • Kotlin platforms: JVM
  • Gradle version: 7.6
  • IDE version : N/A
  • Other relevant context : N/A
@sgjesse
Copy link

sgjesse commented Jan 5, 2023

Just to clarify this comment on the bug I would like to understand why the use of the StringConcatFactory is not removed for an Android build. As StringConcatFactory is not currently supported on Android the code path using it must be dead. Therefore it should be possible to restructure the code so that R8 can see that and fully remove this code, so that the -dontwarn is not needed.

If the -dontwarn is needed it is a better option to put it on the classes in kotlinx.serialization using StringConcatFactory and not directly on java.lang.invoke.StringConcatFactory, as -dontwarn is global and could then hide issues in unrelated code from other libraries.

@Nek-12
Copy link
Author

Nek-12 commented Jan 29, 2023

@shanshin The recently shipped consumer proguard rules in 1.5.0-RC should have included a fix for this issue too

@DanielNovak
Copy link

It still seems to be an issue even on 1.5.0 stable.

@mosmb
Copy link

mosmb commented May 12, 2023

Same issue with 1.5.1

@ba6ba
Copy link

ba6ba commented May 24, 2023

still facing this issue with latest kotlinx-serialization (1.5.1) version

@malenalbc
Copy link

malenalbc commented Jun 6, 2023

Facing this issue with Gradle 8 and latest kotlinx-serialization (1.5.1) version.

@JyotimoyKashyap
Copy link

Facing this issue with AGP 8.0.1 and latest kotlinx-serialization version. Did anybody find any issues ? I am using JDK 11 for building

@Hazem-vivy
Copy link

is there any plans to fix this soon?

@shanshin
Copy link
Contributor

Hi,
we are working on a solution to the problem.

Adding a common -dontwarn for java.lang.invoke.StringConcatFactory, or for all serialization classes does not solve the problem, but hide it in the serialization itself, or in the unrelated code of other dependencies.

It would be really nice for us to have a user-defined reproducer (in any form that we can checkout & run), so we can understand the root cause of the problem and to pinpoint it instead of potentially treating the symptoms

@Hazem-vivy
Copy link

let me try to reproduce the issue on a sample project and share it with you, probably next week

@JyotimoyKashyap
Copy link

I have researched around the source code of NowInAndroid and they are not using the @Keep annotation which actually uses StringConcatFactory behind the scenes above Java 8. They are using @Serializable annotation with Json factory converter. Thus mitigating this issue altogether while using Java 11.

@nininea
Copy link

nininea commented Jul 6, 2023

same issue, any solution ?

@JyotimoyKashyap
Copy link

same issue, any solution ?

It actually depends what's using StringConcatFactory in your code. For me, the solution I suggested actually works to mitigate the issue.

@sandwwraith
Copy link
Member

sandwwraith commented Jul 6, 2023

Unfortunately we're unable to reproduce the problem on a clean project. Grepping decompiled JAR files of kotlinx.serialization also does not show any usages of StringConcatFactory. So a small project with reproducer will really help us tackle the problem.

@Nek-12
Copy link
Author

Nek-12 commented Jul 8, 2023

If you take a look at the issue description and the related Google's bug tracker, you will be able to understand that StringConcatFactory is not required to be directly used in Kotlinx.serialization to result in this problem

@sandwwraith
Copy link
Member

@Nek-12

that StringConcatFactory is not required to be directly used in Kotlinx.serialization

In that case, what is the connection between kotlinx.serialization and this problem? Compiler plugin can't produce calls on StringConcatFactory on its own, as it uses existing compiler API. So it is either a common Kotlin compiler problem, or some configuration issue that forces the compiler to emit references to StringConcatFactory.

@Nek-12
Copy link
Author

Nek-12 commented Jul 15, 2023

StringConcatFactory is used for concatenating strings. Each time we are using formatted strings: "$param string", this translates into the StringConcat call on java. Take my hypothesis with a grain of salt as I'm not an expert either, just researched a bit

@sandwwraith
Copy link
Member

Each time we are using formatted strings: "$param string", this translates into the StringConcat call on java

This is correct. It also means that Kotlin compiler produces this call, and not kotlinx.serialization.

@sandwwraith
Copy link
Member

I suspect that this problem is caused by incorrect configuration. If you have these lines:

    kotlinOptions {
        jvmTarget = "11"
    }

It means that Kotlin configured to produce Java11 bytecode. In that case, it will emit calls to StringConcatFactory.
To solve this problem, change jvmTarget to 1.8.

@voloshink
Copy link

I suspect that this problem is caused by incorrect configuration. If you have these lines:

    kotlinOptions {
        jvmTarget = "11"
    }

It means that Kotlin configured to produce Java11 bytecode. In that case, it will emit calls to StringConcatFactory. To solve this problem, change jvmTarget to 1.8.

What if you need to target 11? What makes this configuration incorrect?

@sandwwraith
Copy link
Member

sandwwraith commented Jul 24, 2023

What if you need to target 11

Then how do you expect this code to run on Android? IIRC R8 can't desugar code intended for Java 11. Original ticket says there is desugaring, but not for StringConcatFactory (for which the stubs are missing). So in that case it seems logical to me that you should provide your own -dontwarn if you want to target Java11.

@Nek-12
Copy link
Author

Nek-12 commented Jul 25, 2023

Why is it logical to have users of the library provide proguard rules for that library?

@sgjesse
Copy link

sgjesse commented Jul 25, 2023

It would normally be logical to add rules required for a library to the library. The main reason for not adding rules like -dontwarn java.lang.invoke.StringConcatFactory is that rules are are global, and applied to all libraries and program code. See my comment from Jan 5 above on adding it for the classes using StringConcatFactory.

@sandwwraith
Copy link
Member

@Nek-12 As I said, kotlinx.serialization does not require that rules. Your code (compiled with jvmTarget = 11) does. Therefore, as author of the library, you should provide the rules :)

@sgjesse
Copy link

sgjesse commented Jul 25, 2023

Finally spend dome time to actually figure out where this warning is coming from, see https://issuetracker.google.com/250197571#comment25. As it has nothing to do with Kotlin Serialization so adding the suggested -dontwarn to kotlinx.serialization is not a solution as @sandwwraith has already mentioned.

The -dontwarn should be added to the ProGuard rules for the library build, not to the library consumer rules.

I think this can be closed as working as intended.

@sandwwraith
Copy link
Member

@sgjesse Thank you very much for clear and detailed explanation, that's exactly what I was trying to say.

@sandwwraith sandwwraith closed this as not planned Won't fix, can't repro, duplicate, stale Jul 25, 2023
@sandwwraith sandwwraith added wontfix and removed bug labels Jul 25, 2023
@sgjesse
Copy link

sgjesse commented Jul 27, 2023

An alternative to using the -dontwarn in the ProGuard rules for building the library is to turn off generation of invokedynamic for string concatenation for kotlinc when building the with the option -Xstring-concat=inline. In AGP one can use the following in build.gradle or build.gradle.kts for the library:

android {
    kotlinOptions {
        jvmTarget = "11"
        freeCompilerArgs = listOf(
            "-Xstring-concat=inline"
        )
    }
}

AGP already turns off off generation of invokedynamic for string concatenation for javac.

@sandwwraith
Copy link
Member

Issue at Google tracker about adding StringConcatFactory to R8 stubs: https://issuetracker.google.com/issues/285090974

@ArcherEmiya05
Copy link

ArcherEmiya05 commented Sep 20, 2023

@keep

I am using @keep annotation and getting this issue on that data class where it was applied. Do you have any idea for actual fix? I added -dontwarn java.lang.invoke.StringConcatFactory rule but I am getting Unresolved class name.

UPDATE: It seems the issue after adding such rule is a bug from Android. The checker is contradicting to what it should actually do.

@worstkiller
Copy link

An alternative to using the -dontwarn in the ProGuard rules for building the library is to turn off generation of invokedynamic for string concatenation for kotlinc when building the with the option -Xstring-concat=inline. In AGP one can use the following in build.gradle or build.gradle.kts for the library:

android {
    kotlinOptions {
        jvmTarget = "11"
        freeCompilerArgs = listOf(
            "-Xstring-concat=inline"
        )
    }
}

AGP already turns off off generation of invokedynamic for string concatenation for javac.

This worked, thanks

@jeffreyfhow
Copy link

When I add freeCompilerArgs = listOf("-Xstring-concat=inline") to my kotlinOptions in my build.gradle, I get...

"Could not find method listOf() for arguments [-Xstring-concat=inline] on object of type org.jetbrains.kotlin.gradle.plugin.AndroidProjectHandler$configureTarget$kotlinOptions$1."

Any advice on how to resolve this? Thank you!

@sgjesse
Copy link

sgjesse commented Nov 15, 2023

Sorry, but I only tested with Kotlin Script (build.gradle.kts). For Groovey (build.gradle) try freeCompilerArgs = ["-Xstring-concat=inline"].

@johngray1965
Copy link

with build.gradle.kts in Android (in 2024), I needed:
freeCompilerArgs.add("-Xstring-concat=inline")

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

No branches or pull requests