Search in all versions.properties, not just the first one #4830 #4831
Conversation
Codecov Report
@@ Coverage Diff @@
## main #4831 +/- ##
============================================
+ Coverage 84.74% 84.75% +0.01%
Complexity 3431 3431
============================================
Files 491 491
Lines 11270 11278 +8
Branches 2074 2076 +2
============================================
+ Hits 9551 9559 +8
Misses 673 673
Partials 1046 1046
Continue to review full report at Codecov.
|
| load(inputStream) | ||
| getProperty("detektVersion") | ||
| } | ||
| }.distinct().single() |
There was a problem hiding this comment.
Should this be adapted to take the first instead?
There was a problem hiding this comment.
Yeah, maybe instead first but log when there are more than one?
There was a problem hiding this comment.
I kind of like the idea of single. If there are more than one and they are different the build should crash.
Maybe we should improve the error message but I think that's out of scope here. That could be tracked/done in other issue/PR.
There was a problem hiding this comment.
If there are more than one and they are different the build should crash.
Yeah ideally this 👍
There was a problem hiding this comment.
Ok cool! What are the next steps? Would it be worthwhile to open a ticket for the improved error message now? Right now it is something like IllegalArgumentException List has more than one element
There was a problem hiding this comment.
Right now it is something like IllegalArgumentException List has more than one element
Yup Ideally I would catch this and error("You're importing two Detekt plugins which are having different versions and will cause unexpected behavior in your build. Make sure th align the versions"). You can also print the list of versions if you wish
There was a problem hiding this comment.
Sounds great. How's this?
Caused by: java.lang.IllegalStateException: You're importing two Detekt plugins which are have different versions. (1.20.1, 1.20.0) Make sure to align the versions.
at io.gitlab.arturbosch.detekt.extensions.DetektExtensionKt.loadDetektVersion(DetektExtension.kt:130)
at io.gitlab.arturbosch.detekt.extensions.DetektExtension.<init>(DetektExtension.kt:14)
at io.gitlab.arturbosch.detekt.extensions.DetektExtension_Decorated.<init>(Unknown Source)
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
at org.gradle.internal.instantiation.generator.AsmBackedClassGenerator$InvokeConstructorStrategy.newInstance(AsmBackedClassGenerator.java:2078)
at org.gradle.internal.instantiation.generator.AbstractClassGenerator$GeneratedClassImpl$GeneratedConstructorImpl.newInstance(AbstractClassGenerator.java:488)
at org.gradle.internal.instantiation.generator.DependencyInjectingInstantiator.doCreate(DependencyInjectingInstantiator.java:64)
... 197 more
internal fun loadDetektVersion(classLoader: ClassLoader): String {
// Other Gradle plugins can also have a versions.properties.
val distinctVersions = classLoader.getResources("versions.properties").toList().mapNotNull { versions ->
Properties().run {
val inputStream = versions.openConnection()
/*
* Due to https://bugs.openjdk.java.net/browse/JDK-6947916 and https://bugs.openjdk.java.net/browse/JDK-8155607,
* it is necessary to disallow caches to maintain stability on JDK 8 and 11 (and possibly more).
* Otherwise, simultaneous invocations of Detekt in the same VM can fail spuriously. A similar bug is referenced in
* https://github.com/detekt/detekt/issues/3396. The performance regression is likely unnoticeable.
* Due to https://github.com/detekt/detekt/issues/4332 it is included for all JDKs.
*/
.apply { useCaches = false }
.getInputStream()
load(inputStream)
getProperty("detektVersion")
}
}.distinct()
return distinctVersions.singleOrNull() ?: error(
"You're importing two Detekt plugins which have different versions. " +
"(${distinctVersions.joinToString()}) Make sure to align the versions."
)
}
See details in #4830 about the issue.
This searches all resources in the Gradle classpath for
versions.propertiesinstead of using the first one.It will fail if there are more than one
versions.propertieswith differentdetektVersions.I considered another approach where the
::class.java.protectionDomain.codeSource?.locationwas used to locate the current jarfile, and pick the right resource based on that. But I was less confident that would work in all environments, as it depended on more low-level classpath details.