Feature request: Room KSP support

[PhilGlass]

KSP has been stable for a while now, and is supported by Room since 2.3.0-beta02.

The schema generation behaviour is the same as the kapt version, so it suffers from the same cache miss issues.

[runningcode]

Thanks for the request Phil.
Can you share the cache miss that you are seeing when using KSP with Room and the android-cache-fix plugin applied? This will help us understand how to fix it.

[PhilGlass]

I could put together a standalone repro, but I think the issues are exactly the same as they are for the kapt version - schemaLocation is passed as an absolute path and is used as both an input and output directory.

[runningcode]

That helps for now. Thanks.

[runningcode]

@PhilGlass do you know if there is an issue tracking the KSP cache miss on Google's side?

[PhilGlass]

I don't know of a KSP specific one - I guess the existing issue applies? In that you can't really fix this using only kapt/KSP APIs, you need a Gradle plugin.

[runningcode]

Can you please add a comment in that one or file a new one? :)
We're often in discussions about this issue with Google but it's really helpful to have the community pressure to fix this since they don't understand the importance or impact of this issue.

[runningcode]

This is currently not possible since KSP does not expose a way to pass CommandLineArgumentProviders to the extension.
https://github.com/google/ksp/blob/main/gradle-plugin/src/main/kotlin/com/google/devtools/ksp/gradle/KspExtension.kt

google/ksp#847

[PhilGlass]

KspTask exposes its ListProperty<SubpluginOption> as public API, so I think it is possible - I used that to work around this internally for now. Might be worth waiting for the CommandLineArgumentProvider to land & release though!

One thing to note is that, like kapt, KSP tasks wipe any declared output directories when they run non-incrementally (this behaviour is inherited from AbstractKotlinCompile).

[runningcode]

Thanks for following up on this @PhilGlass . Unfortunately, ListProperty<SubpluginOption> is neither an input nor an output. That means that changes to the input, or clearing the output directory would not cause the task to re-run leading to an incorrect build.

On the second note thanks for sharing, that is why the workaround that google has explained here is simply not sufficient. Please comment and let Google know that their workaround is not sufficient. I have already shared my thoughts with them on this workaround via other channels but I would like to hear the community say it as well.

The Android Cache Fix plugin has a workaround for the task outputs being cleared .

[PhilGlass]

Thanks for following up on this @PhilGlass . Unfortunately, ListProperty is neither an input nor an output. That means that changes to the input, or clearing the output directory would not cause the task to re-run leading to an incorrect build.

It's definitely not a workaround on its own - but it is a mechanism you can use to pass arguments to KSP without them becoming a task input (using InternalSubpluginOption or one of the other SubpluginOption subclasses that are treated specially - see my comment on your KSP PR). Then you can use the other workarounds implemented by this plugin to fix the myriad of other problems!

On the second note thanks for sharing, that is why the workaround that google has explained here is simply not sufficient. Please comment and let Google know that their workaround is not sufficient. I have already shared my thoughts with them on this workaround via other channels but I would like to hear the community say it as well.

Done. As I mentioned in that comment, Auto migrations are another spanner in the works. Now the schemas directory is both an input and output of the kapt/KSP tasks, because past versions of the schema are required to generate the auto migrations. So if the schema directory changes, Room should re-run... but Room also writes the current schemas into that directory, effectively invalidating its own inputs for the next build. 😭

[runningcode]

Thanks for commenting there! Makes sense that the Subplugin options are a workaround. I'll take care of resolving your comments in the PR soon.
If you can still edit your comment, it would be good to explain that their workarounds for KAPT do not fully solve the issue as well for the same reasons you mention.

And yes the RoomSchemaWorkaround does also take care of the pre-existing schemas.

[PhilGlass]

I can't edit it any more - but it already mentions kapt briefly.

And yes the RoomSchemaWorkaround does also take care of the pre-existing schemas.

Sort of. It copies the contents of the version controlled schemas directory to the temporary directories passed as room.schemaLocation - but that version controlled directory isn't tracked as a task input (or output). So if something in it changes kapt/KSP won't re-run, which can produce incorrect results for auto migrations (e.g if you delete an old schema that's required to generate an auto migration Room should fail, but it won't unless some other change forces it to rerun).

[runningcode]

Thanks for that edge case. It's a good point that we don't cover that, we didn't think it was so common. Is that a common thing to delete old schema?

[PhilGlass]

I don't think so - I haven't seen this fail in practice. Just another thing that came to mind when I was thinking about all the things that are wrong with the way room.schemaLocation is designed. 😅

[bishiboosh]

Seeing as the PR has been merged in KSP, is there any news on this topic ?

[runningcode]

Unfortunately, this is still blocked by this. KSP requires variant support in order for this workaround to work otherwise we will have overlapping outputs and the caching will still be disabled.

[cdsap]

@PhilGlass @PhilGlass we have published version 2.7.0 with support for ksp in the RoomWorkaround