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

Kotlin 1.9.0 #752

Merged
merged 8 commits into from
Jul 19, 2023
Merged

Kotlin 1.9.0 #752

merged 8 commits into from
Jul 19, 2023

Conversation

SimonMarquis
Copy link
Contributor

@SimonMarquis SimonMarquis commented May 25, 2023

This PR is testing future support for Kotlin 1.9.0:

Warning
Had to force register generated protobuf sources to Android sourceSets otherwise Hilt fails to resolve UserPreferences generated type.

androidComponents.beforeVariants {
    android.sourceSets.register(it.name) {
        java.srcDir(buildDir.resolve("generated/source/proto/${it.name}/java"))
        kotlin.srcDir(buildDir.resolve("generated/source/proto/${it.name}/kotlin"))
    }
}

Remaining issues

  • https://github.com/android/nowinandroid/actions/runs/5083859526/jobs/9135396287?pr=752#step:8:971 Unexpected lint error 🤯
    Adding an explicit return@remember fixes the issue.

    nowinandroid\app\src\androidTest\java\com\google\samples\apps\nowinandroid\ui\NiaAppStateTest.kt:186: Error: remember calls must not return Unit [RememberReturnType from androidx.compose.runtime]
      val navController: TestNavHostController = remember {
                                                 ~~~~~~~~
    
     Explanation for issues of type "RememberReturnType":
     A call to remember that returns Unit is always an error. This typically
     happens when using remember to mutate variables on an object. remember is
     executed during the composition, which means that if the composition fails
     or is happening on a separate thread, the mutated variables may not reflect
     the true state of the composition. Instead, use SideEffect to make deferred
     changes once the composition succeeds, or mutate MutableState backed
     variables directly, as these will handle composition failure for you.
    
  • https://github.com/android/nowinandroid/actions/runs/5083859526/jobs/9135396287?pr=752#step:8:849
    "Fixed" by forcing a more recent version of Lint to AGP.
    .../transformed/out/jars/classes.jar!/META-INF/....kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.9.0, expected version is 1.7.1.

  • Enabling K2 leads to the following errors out of scope

    [Hilt] All modules must be static and use static provision methods or have a visible, no-arg constructor. Found: [...]Module - com.google.samples.apps.nowinandroid.NiaApplication
    

@lihenggui
Copy link
Contributor

Now Kotlin 1.9.0 was officially released. We can upgrade the dependencies:

androidxComposeCompiler = "1.5.0-dev-k1.9.0-6a60475e07f"
kotlin = "1.9.0"
ksp = "1.9.0-1.0.11"
protobuf = "3.23.4"

So far, I don't see issues after upgrading these dependencies.
And I have a question, disabling the build cache is for testing, it that correct?

@SimonMarquis
Copy link
Contributor Author

SimonMarquis commented Jul 8, 2023

I'll update the dependencies, but there is still an ongoing issue with Lint.
AGP 8.0.2 uses internally Lint 31.0.2 which is compiled with metadata in version 1.7.1. This makes the Lint analyse tasks print errors.
A temporary solution is to bump the lint version to the most recent, and most stable, and compatible version which is 31.1.0-rc, or simply just wait for the next AGP stable version.

And I have a question, disabling the build cache is for testing, it that correct?

Yes, I'll remove this as well.

@SimonMarquis
Copy link
Contributor Author

Also, Lint errors similar to the return@remember still exists in my work app even with 1.9.0, so 🤷

@SimonMarquis
Copy link
Contributor Author

And we probably will wait for a stable compose compiler version anyway. (not relying on 1.5.0-dev-k1.9.0-6a60475e07f)

SimonMarquis and others added 4 commits July 19, 2023 09:20
Change-Id: Ieddb8410a592bb38c5764b7288fd4828306d6743
Change-Id: I344cb7542107f134b71c62b652211eba370762ec
@zsmb13 zsmb13 marked this pull request as ready for review July 19, 2023 07:21
Change-Id: I11a2cf41f1428931794cf26ebc655ccb0cda33ab
@zsmb13
Copy link
Contributor

zsmb13 commented Jul 19, 2023

Bumped Compose to stable 1.5.0, added the Dagger version bump to get kotlinx.metadata up-to-date, and bumped lint to 31.0.2 to hopefully clean up the errors that it was spitting out earlier.

With that we should be good to merge this once CI is done.

@SimonMarquis
Copy link
Contributor Author

SimonMarquis commented Jul 19, 2023

This bump in lint version will not resolve the issue, which is related to the lint version used by AGP.

@zsmb13
Copy link
Contributor

zsmb13 commented Jul 19, 2023

I see, do you know if these errors cause any practical problems? I see that lint tasks still succeed and apparently create reports. They are also able to fail on lint errors in Kotlin code (like that false positive on remember if I remove the fix 😅).

@SimonMarquis
Copy link
Contributor Author

To be honest, I'm not sure about that. It will probably fail to parse and interpret some specific Lint issues for sure. But to me the main problem is that it pops up in the IDE and "look like" an error, which means, you'll probably have many reports saying that it "fails to run lint". If we want to still merge this update, I would instead merge the AGP's Lint version to the most recent 31.1.0-rc and revert this once AGP updates. WDYT?

Change-Id: Ifb8f9495a01b4916c3e1ec0cd6ff4bd47080df37
gradle.properties Outdated Show resolved Hide resolved
@SimonMarquis SimonMarquis changed the title 🚧 Kotlin 1.9.0 Kotlin 1.9.0 Jul 19, 2023
@SimonMarquis
Copy link
Contributor Author

Thanks for the updates @dturner & @zsmb13.
I've updated the PR description, and I guess we are now ready to merge this once the CI validates it.
🚀

Copy link
Collaborator

@dturner dturner left a comment

Choose a reason for hiding this comment

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

Thanks very much for doing this.

@zsmb13 zsmb13 merged commit 56389cb into android:main Jul 19, 2023
5 checks passed
@SimonMarquis SimonMarquis deleted the kotlin-1.9.0 branch July 25, 2023 19:20
@SimonMarquis
Copy link
Contributor Author

Workaround removed in

@dturner dturner added the version update This is related to a library, API or SDK update which requires some work. Not just a version bump. label Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version update This is related to a library, API or SDK update which requires some work. Not just a version bump.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants