Fix fastutil shadowing #3476
Fix fastutil shadowing #3476
Conversation
* additionally support running CLI tests with K2
| // configuration for dependencies which we need to replace with original ones because `kotlin-compiler` minimizes them | ||
| val shadowOverride: Configuration by configurations.creating { | ||
| attributes { | ||
| attribute(Usage.USAGE_ATTRIBUTE, project.objects.named(Usage::class.java, "java-runtime")) | ||
| } | ||
| } | ||
|
|
||
| dependencies { | ||
| shadowOverride(libs.fastutil) | ||
| } | ||
|
|
||
| val shadowDependenciesJar by tasks.registering(ShadowJar::class) { | ||
| group = "shadow" | ||
| description = "Create a shadow jar from dependencies without fastutil" | ||
|
|
||
| archiveClassifier.set("dependencies") | ||
| destinationDirectory.set(project.layout.buildDirectory.dir("shadowDependenciesLibs")) | ||
|
|
||
| // we need to create JAR with dependencies, but without fastutil, | ||
| // so we include `runtimeClasspath` configuration (the same configuration which is used by default `shadowJar` task) | ||
| // and include `fastutil` from the result | ||
| configurations = listOf(project.configurations.runtimeClasspath.get()) | ||
| exclude("it/unimi/dsi/fastutil/**") | ||
| } | ||
|
|
||
| tasks.shadowJar { | ||
| // override configurations to remove dependencies handled in `shadowJarDependencies` | ||
| configurations = emptyList() | ||
| from(shadowOverride, shadowDependenciesJar) | ||
| } |
There was a problem hiding this comment.
It looks like this code is duplicated - should, or could, it be extracted into a convention plugin?
There was a problem hiding this comment.
Yeah, the code is the same, but it's intentional at my side. While for now the issue is the same for both modules (with fastutil only); symbols and descriptors dependencies are rather different and even use different versions of Kotlin Compiler under the hood. So if something will change in future (in our build or in dependencies) it will be easier to fix.
This is rather a workaround for now than a long term solution, and we hope it will be fully removed for symbols module when Standalone Kotlin Analysis API will become stable
There was a problem hiding this comment.
Ah right, yes it makes sense that not everything will be duplicated. But the shadowDependenciesJar task and shadowOverride will be the same in each subproject? Could those definitions be moved to the existing dokkabuild.publish-shadow.gradle.kts plugin, and then each subproject would have independent workarounds, and add their own dependencies and exclusions?
E.g. add this to the convention plugin:
// dokkabuild.publish-shadow.gradle.kts
// ...
val shadowOverride: Configuration by configurations.creating {
// ...
}
val shadowDependenciesJar by tasks.registering(ShadowJar::class) {
//...
}
tasks.shadowJar {
// override configurations to remove dependencies handled in `shadowJarDependencies`
configurations = emptyList()
from(shadowOverride, shadowDependenciesJar)
}And then each subproject could set up things differently:
dependencies {
shadowOverride(libs.fastutil)
}
tasks.shadowDependenciesJar {
exclude("it/unimi/dsi/fastutil/**")
}There was a problem hiding this comment.
Up to you - I just wanted to clarify about what I think could be deduplicated, but perhaps it's not worth it :)
There was a problem hiding this comment.
But the shadowDependenciesJar task and shadowOverride will be the same in each subproject?
Who knows? :)
On current moment current workaround works for fastutil and we have no idea now, how other things will work.
So here Im more in favour of copy-paste workaround instead of generalising workaround based on one use-case.
Will wait for other reviews on this.
There was a problem hiding this comment.
I agree with Oleg here 👍 I think it would be mega useful to extract this logic into a convention plugin if we had more than one use case for it that we expected to have long-term
This particular hack, however, ideally should only stay in K1 (if at all), and the K2 analysis shouldn't have this problem to begin with. We're waiting for K2's Analysis API to set up proper artifact publishing, and hopefully after KT-61404 is implemented, we'll be able to remove this workaround from the analysis-kotlin-symbols build scripts
So I wouldn't generalize this logic now, and would wait until we stumble upon this problem again (which might be never, but might be next month)
dokka-integration-tests/cli/projects/it-cli/src/main/kotlin/RootPackageClass.kt
Outdated
Show resolved
Hide resolved
…otPackageClass.kt Co-authored-by: Ignat Beresnev <ignat.beresnev@jetbrains.com>
* support running CLI tests with K2 * explicitly control dependencies in CLI integration tests --------- Co-authored-by: Ignat Beresnev <ignat.beresnev@jetbrains.com> (cherry picked from commit 3c989c1)
* support running CLI tests with K2 * explicitly control dependencies in CLI integration tests --------- Co-authored-by: Ignat Beresnev <ignat.beresnev@jetbrains.com>
Fixes: #3329 (comment)
I've updated the description of the fix in files, where the workaround is applied.
Also there are two changes in CLI integration tests:
isTransitive = falsewhen buildingshadowJarwhich is used aspluginsClasspathin CLI - this way we can better control/understand which dependencies are provided to CLI. This will help to find issues with missing dependencies in future.If needed, I can split CLI integration tests changes to separate PR.