I'd like to tackle <https://github.com/google/ksp/...
# ksp
n
I'd like to tackle https://github.com/google/ksp/issues/277 if team is not working on it. I have it working actually, but I need to take some decisions which I'd like to discuss: 1. Right now I'm creating a configuration per
KotlinTarget
, named
ksp<TargetName>
. For single-platform projects though, it might make sense to skip this step - when kotlin-jvm plugin is used for example, there's no point in having to write
kspJvm
as it would be identical to root
ksp
. What do you think? 2. Within each target, I create a configuration per
KotlinCompilation
, named
ksp<TargetName><CompilationName>
, extending the target configuration. As above, target name is omitted in single-platform. We could avoid creating this configuration for the main compilation, but after playing with it I think it's very nice to have fine grained control over main vs. test vs. both. 3. Within each compilation, I create a configuration per
KotlinSourceSet
, named
ksp<SourceSetName>
, but only if the source set is not the default one for that compilation. Basically this will be skipped 99% of the times? 4. Intermediate source sets do not have their own configuration. May be introduced in a future PR/version? 5. Android needs special handling. The KotlinSourceSet names are very confusing:
Copy code
compilation=debug sourceSets=[androidDebug, androidMain]
compilation=debugAndroidTest sourceSets=[androidDebugAndroidTest, androidAndroidTest, androidAndroidTestDebug]
compilation=debugUnitTest sourceSets=[androidDebugUnitTest, androidTest, androidTestDebug]
compilation=release sourceSets=[androidRelease, androidMain]
compilation=releaseUnitTest sourceSets=[androidReleaseUnitTest, androidTest, androidTestRelease]
Looks to me that compilations == AGP variants, so one option is to create one config per variant named
ksp<TargetName><VariantName>
. Another option is to read the `AndroidSourceSet`s from AGP (main, debug, release, test, testDebug, testRelease, androidTest, androidTestDebug, androidTestRelease). This gives more fine grained control, but it's harder to do (testDebug should extend both debug and test... Also not clear how to retrieve the correct configurations later in
applyToCompilation()
).
t
@gavra @yigit any thoughts? 1 and 2: Sounds good to me. However, if separating logic for single target and multi-target becomes too complicated, it should be fine to simply keep the redundant configurations, which only happens for single target and I assume that the overhead should be neglectable. 3: Is that fine-grained control really needed? Like 4., adding extra configuration won't break existing projects so maybe leave it in some future PRs after we find use cases. 4: May I know what intermediate source sets do you mean? the one generated from resources? IMHO, including generated sources will likely undermine interactions between KSP and other components / plugins. If there is no strong reason, we should avoid it. If we do find some convincing use cases, doing it in a future PR totally make sense. 5: This is beyond my knowledge 🙂 @gavra could you shed some lights on this?
n
About 3 - Ok, I will skip KotlinSourceSet-level configurations. We'll have kspJvm, kspJvmMain and kspJvmTest. Is this what you mean? About 4 - By intermediate I mean non-leaf source set created by the user to share code between some targets, like iosMain here: https://kotlinlang.org/docs/mpp-share-on-platforms.html#share-code-on-similar-platforms . I agree to avoid this for now. Thanks!
g
This may deserve a write-up with clear definition and examples 🙂 From users POV, we want to have configuration per-source set, and this is what users see in their build files. E.g: • for kotlin-jvm projects:
ksp
(for the main) and
testKsp
(for the test) • for android: ksp (for main), testKsp (for test) and androidTestKsp (for device tests). In general, we want
allAndroidSourceSets>Ksp
which in a typical project maps to (
ksp
,
debugKsp
,
releaseKsp
,
testKsp
,
androidTestKsp
) • for multi-platform, it is the same, all sources sets, followed by KSP I think this part is already in KSP. The part that is not is how we construct the final configuration (used by KSP task, let's call that
_KspClasspath
). E.g: • for kotlin-jvm projects:
mainKspClasspath
should contain only
ksp
, and
testKspClasspath
(should contain
testKsp
- right now it also contains
ksp
which causes perf issues) • for android: it is
<variant>KspClasspath
and we need to figure out the same thing • for multi-platform: ditto
👍 1
n
Thanks Ivan. I propose we split this in 3 tasks/PR - 1. enable multiplatform configurations. Currently ksp reacts only to single-platform plugins. So in a jvm+js project we should have
*Jvm*
and
*Js*
configs. Ignore Android targets at this step. 2. enable multiplatform Android-specific configurations. We'll need to add the target name (typically
Android
) in the config to avoid conflicts with other targets. We can discuss this later. 3. fix classpath perf issues What do you think? As a side note, I think we should name configurations ksp<Something> (ksp at start, not end). That's what ksp currently does and also what
kapt
does (kaptAndroidTest for example).
g
Naming: In java/Android projects it is <sourceSet>ConfigurationName, but considering we already shipped
ConfigurationName<sourceSet>
we can continue using that pattern. The thing is that we need to decide about 3 before 1&2 are done. Creating user-visible configurations is one thing, but we also need to use them to resolve the ksp classpath when running the tasks (i.e we need to decide which configurations will be part of
ksptClasspath
one). So, I think we need to consider all of 1&2&3 together 🙂 When it comes to which configurations to create, we should create the same configurations as for the compile/runtime classpath e.g if there is
implementationJvm
we should have corresponding
kspJvm
. This info should be in the source set extension of KMP (it also has Android info) and single-platform mpp projects.
n
Is there a design decision to be made about 3 (like, we want to improve it but we don't know how)? I still need to play with it, but I think we could just use
KotlinCompilation.kotlinSourceSets
info in applyToCompilation to determine the correct user-facing configuration(s), instead of the current approach with
KotlinCompilation.allKotlinSourceSets
. That could be enough.
Plus avoid adding
ksp
to all compilations, as what we really mean with it is
kspMain
so it shouldn't be added to test for instance
This might solve 1 & 3: https://github.com/deepmedia/ksp/commit/5ee19544aa04fd932f18858df6f438276b95489f in case you have high level feedback. Tomorrow I can check tests and open a PR, then solve Android later (it's untouched here).
Hi, I just opened a PR, solving 1-2-3 together https://github.com/google/ksp/pull/609 . This test should give you an idea of what we have, I think it matches your expectations. I'll do a bit more testing and then undraft it.
Ok - not a draft anymore! Please let me know if you have any suggestions 🙂 everything works as expected in my tests.
g
Thanks Mattia! I'll try to take a look later this week or during the next week.
n
Sounds good, thanks Ivan!
Hi @gavra , would you or someone else be able to take a look this week?
g
sorry about the delay, I'm taking a look now 🙂