Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

[Android] Revert Java toolchain JDK #771

Merged
merged 1 commit into from
Aug 18, 2023
Merged

Conversation

jonnyandrew
Copy link
Contributor

Problem

Updating Gradle and AGP to version 8 (#757), it was necessary to update the JDK used to run Gradle to 17. However, during this update, the toolchain JDK used to compile the source code was also updated to 17 which is not compatible with Android SDK API 33.

Solution

Revert to Java toolchain JDK to version 11.

@jonnyandrew jonnyandrew requested a review from a team August 18, 2023 12:01
@codecov-commenter
Copy link

Codecov Report

Patch and project coverage have no change.

Comparison is base (58a395e) 89.95% compared to head (a7a5f2b) 89.95%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #771   +/-   ##
=======================================
  Coverage   89.95%   89.95%           
=======================================
  Files          82       82           
  Lines       14708    14708           
=======================================
  Hits        13231    13231           
  Misses       1477     1477           
Flag Coverage Δ
unittests 89.95% <ø> (ø)
unittests-rust 89.95% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sonarcloud
Copy link

sonarcloud bot commented Aug 18, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@julioromano
Copy link

If api33 weren't comptaible with java 17 bytecode how could #757 pass CI checks?

@jonnyandrew
Copy link
Contributor Author

I'm not 100% sure how it worked. My assumption is that it's because the test app is also configured to compile with JDK 17. But importing it into Element Android (compileSdk 33 / JDK 11) reveals a problem:

bad class file: [..] jetified-wysiwyg-2.5.0-api.jar(/io/element/android/wysiwyg/[..].class)
    class file has wrong version 61.0 [Java 17], should be 55.0 [Java 11]

When I checked Java versions in Android builds docs, I saw the following compatibility table:

Use the table below to determine which Java version is supported by
each Android API, and where to find details on which Java APIs are available.

Android Java
14 (API 34) 17
13 (API 33) 11

@julioromano
Copy link

julioromano commented Aug 18, 2023

Oh I see, I think api33 works fine with java 17 bytecode, the problem is that EA is still built with v11 bytecode (so it can't use libraries built with v17). If we wish to keep compatibility with EA too then we have no choice but to lower ours bytecode version too.

Copy link

@julioromano julioromano left a comment

Choose a reason for hiding this comment

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

Can you please first check if it works by keeping: jvmToolchain(17)

and only changing :

   sourceCompatibility JavaVersion.VERSION_11
   targetCompatibility JavaVersion.VERSION_11

If it doesn't then lower the jvmToolchain too. Thanks!

@jonnyandrew
Copy link
Contributor Author

jonnyandrew commented Aug 18, 2023

Ok, I tried this but the build fails with an error:

> 'compileDebugJavaWithJavac' task (current target is 11) and 'compileDebugKotlin' task (current target is 17) jvm target compatibility should be set to the same Java version.

If I remove targetCompatibility JavaVersion.VERSION_11, then it builds but it's the same story as before - bytecode is not compatible with the EA build.

@julioromano
Copy link

Ok, I tried this but the build fails with an error:

> 'compileDebugJavaWithJavac' task (current target is 11) and 'compileDebugKotlin' task (current target is 17) jvm target compatibility should be set to the same Java version.

If I remove targetCompatibility JavaVersion.VERSION_11, then it builds but it's the same story as before - bytecode is not compatible with the EA build.

Thanks for checking! We'll have to keep everything at 11 then as long as we want to maintain compatibility with legacy element.

@jonnyandrew
Copy link
Contributor Author

Merging with test failures as these are unrelated and fixed in #772

@jonnyandrew jonnyandrew merged commit d2571b4 into main Aug 18, 2023
8 of 9 checks passed
@jonnyandrew jonnyandrew deleted the jonny/fix-java-toolchain-jdk branch August 18, 2023 15:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants