Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rejectVersionIf of different builds influence each other due to static state #667

Open
Vampire opened this issue Feb 6, 2023 · 5 comments

Comments

@Vampire
Copy link
Contributor

Vampire commented Feb 6, 2023

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
Copy link
Contributor Author

Vampire commented Feb 6, 2023

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
Copy link
Contributor Author

Vampire commented Feb 6, 2023

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

@Vampire
Copy link
Contributor Author

Vampire commented Feb 6, 2023

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
Copy link
Contributor Author

Vampire commented Feb 6, 2023

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 Vampire changed the title rejectVersionIf not working at all rejectVersionIf of different builds influence each other due to static state Feb 6, 2023
@Vampire
Copy link
Contributor Author

Vampire commented Feb 6, 2023

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 added a commit to Vampire/setup-wsl that referenced this issue Feb 7, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 7, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 7, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 7, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 8, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 8, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 9, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 10, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 12, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 12, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 12, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 14, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 14, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 15, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 15, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 15, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 15, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 15, 2023
Vampire added a commit to Vampire/setup-wsl that referenced this issue Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant