refreshVersions icon indicating copy to clipboard operation
refreshVersions copied to clipboard

`rejectVersionIf` of different builds influence each other due to static state

Open Vampire opened this issue 2 years ago • 5 comments

Just to test, I configured

refreshVersions {
    rejectVersionIf {
        println("FOO: $moduleId")
        true
    }
}

But it does not reject any version and also does not print any FOO. :-/

Vampire avatar Feb 06 '23 14:02 Vampire

Hm, it calls the wrong one actually. I have build A which includeBuild B. And B includeBuild C. The println("FOO: $moduleId") is in the settings script of C. I also added println("BAR: $moduleId") to the settings script of B and println("BAZ: $moduleId") to the settings script of A.

Now if I call ./gradlew :C:refreshVersions, I see BAR logs instead of FOO logs. o_O

Vampire avatar Feb 06 '23 15:02 Vampire

./gradlew :B:refreshVersions also shows BAR as expected and ./gradlew :refreshVersions shows BAZ as expected

Vampire avatar Feb 06 '23 15:02 Vampire

I've thrown together a reproducer as it is not exactly trivial: showcase.zip If you unzip it and then call ./gradlew :bar:refreshVersions, you will see showcase messages while you should see bar messages, as the bar task is run and also the bar version catalog is updated.

Actually in my production project it is showing foo messages here, but maybe there is additionally some race condition.

Vampire avatar Feb 06 '23 15:02 Vampire

I think the problem is, RefreshVersionsConfigHolder. It is an object. As far as I remember this manifests as static state in the JVM. This is quite bad, as it can even influence builds from completely other projects running on the same Gradle daemon.

There was a similar problem in the past with the Spotbugs Gradle Plugin. It used Spotbugs directly in-process. Spotbugs uses quite some static state. Then builds from completely different projects running on the same Gradle daemon influenced each other, as the static state was carried over.

In the context of Gradle plugins (at least, if not almost anywhere) any static state should be avoided as hell. If you need something like static state that has to be present during one Gradle build execution, the idiomatic pattern is to use a Shared Build Service that holds the data and has a clearly defined lifetime.

Vampire avatar Feb 06 '23 16:02 Vampire

Here a small hacky work-around. Make a small plugin consisting of two files. One file plain Foo.kt:

inline fun Settings.onRefreshVersionsRequested(configure: Settings.() -> Unit) {
    val rootBuild = gradle.parent == null
    val startTaskNames = gradle.rootBuild.startParameter.taskNames
    if (
        (rootBuild && startTaskNames.contains(":refreshVersions"))
        || (!rootBuild && startTaskNames.contains(":${settings.rootProject.name}:refreshVersions"))
    ) {
        configure()
    }
}

inline fun Settings.conditionalRefreshVersions(configure: RefreshVersionsExtension.() -> Unit) {
    onRefreshVersionsRequested {
        refreshVersions(configure)
    }
}

val Gradle.rootBuild: Gradle
    get() = if (gradle.parent == null) gradle else gradle.parent!!.rootBuild

The other a precompiled script plugin (or whatever) doing

require(gradle.rootBuild.startParameter.taskNames.count { it.endsWith("refreshVersions") } <= 1) {
    "Only one refreshVersions task can be executed per Gradle run"
}

onRefreshVersionsRequested {
    apply(plugin = "de.fayard.refreshVersions")
}

and in the settings scripts instead of de.fayard.refreshVersions you apply the plugin from above and instead of refreshVersions { ... } you use conditionalRefreshVersions { ... }.

This way only if you execute one of the refreshVersions tasks explicitly and with its full path, the plugin gets applied and the settings done so they cannot interfere with each other.

Vampire avatar Feb 06 '23 20:02 Vampire