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

Plugin doesn't respect rejectVersionIf lambda #906

Open
rynkowsg opened this issue Sep 17, 2024 · 4 comments
Open

Plugin doesn't respect rejectVersionIf lambda #906

rynkowsg opened this issue Sep 17, 2024 · 4 comments

Comments

@rynkowsg
Copy link

rynkowsg commented Sep 17, 2024

I made a custom filter, that does two things:

  • does not update non-stable after stable
  • ensure that guava versions change is rejected if they are of different flavors

You can find below the convention plugin I used to wrap entire logic related to versions selection.

Convention plugin
import GuavaCheckResult.GUAVA_CANDIDATE_NEWER
import GuavaCheckResult.GUAVA_CANDIDATE_OLDER
import GuavaCheckResult.GUAVA_DIFFERENT_FLAVORS
import GuavaCheckResult.GUAVA_SAME_VERSIONS
import GuavaCheckResult.NOT_GUAVA
import buildcfg.GradlePluginId
import com.github.benmanes.gradle.versions.updates.DependencyUpdatesTask
import com.github.benmanes.gradle.versions.updates.resolutionstrategy.ComponentSelectionWithCurrent
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.kotlin.dsl.withType

val isDEBUG: Boolean = System.getenv("DEBUG").let { it in listOf("true", "1") }.also { println("DEBUG=$it") }

fun log(message: String) {
    if (isDEBUG) {
        println(message)
    }
}

private data class VersionComparisonData(
    val group: String?,
    val module: String?,
    val oldVersion: String,
    val newVersion: String,
)

class VersionsConventionPlugin : Plugin<Project> {
    override fun apply(project: Project) = with(project) {
        println("Applying VersionsConventionPlugin to $name")
        pluginManager.apply(GradlePluginId.VERSIONS)

        tasks.withType<DependencyUpdatesTask>().configureEach {
            checkForGradleUpdate = true
            outputFormatter = "plain,html"
            outputDir = "build/dependencyUpdates"
            reportfileName = "report"
            val filter: ComponentSelectionWithCurrent.() -> Boolean = {
                val candidate = this
                val comparisonData = VersionComparisonData(
                    group = candidate.candidate.group,
                    module = candidate.candidate.module,
                    oldVersion = candidate.currentVersion,
                    newVersion = candidate.candidate.version,
                )
                log("comparisonData: $comparisonData")
                val shouldReject = !shouldApprove(comparisonData)
                log("shouldReject: $shouldReject")
                log("------------------------------------------------------------")
                shouldReject
            }
            rejectVersionIf(filter)
        }
    }
}

private fun shouldApprove(data: VersionComparisonData): Boolean {
    val foundResult = checkIfGuavaLibrary(data)
    log("foundResult: $foundResult")
    when (foundResult) {
        GUAVA_SAME_VERSIONS -> return true // we don't want to update to the same version
        // different flavours should not be compared
        // e.g. 32.1.3-android, 32.1.3-jre - there is no new version in such case
        GUAVA_DIFFERENT_FLAVORS -> return false
        GUAVA_CANDIDATE_NEWER -> return true // approve newer version
        GUAVA_CANDIDATE_OLDER -> return false // reject older version
        NOT_GUAVA -> {} // do nothing
    }
    // approve new version if:
    // - the new is stable, or
    // - the current is unstable (if current is unstable, new can be either stable or unstable)
    val shouldApprove = isStable(data.newVersion) || !isStable(data.oldVersion)
    return shouldApprove
}

// Compares whether two versions are same just different by flavour.
// It is to deal with situation when plugin suggest to upgrade -android
// Guava version to -jre version, e.g. from `guava-25.1-android` to `guava-25.1-jre`.
private fun checkIfGuavaLibrary(data: VersionComparisonData): GuavaCheckResult {
    if (data.group != "com.google.guava") {
        return NOT_GUAVA
    }
    val (oldVer, oldFlavor) = parseGuavaVersion(data.oldVersion) ?: Pair(null, null)
    val (newVer, newFlavor) = parseGuavaVersion(data.newVersion) ?: Pair(null, null)
    if (oldVer == null || newVer == null || oldFlavor == null || newFlavor == null) {
        return NOT_GUAVA // problems when parsing - treat as not Guava
    }
    if (oldFlavor != newFlavor) {
        return GUAVA_DIFFERENT_FLAVORS // if not the same flavors, they should not be compared
    }
    val same = oldVer == newVer
    if (same) return GUAVA_SAME_VERSIONS
    // TODO: improve the comparison
    return if (newVer > oldVer) GUAVA_CANDIDATE_NEWER else GUAVA_CANDIDATE_OLDER
}

private enum class GuavaCheckResult {
    NOT_GUAVA,
    GUAVA_DIFFERENT_FLAVORS,
    GUAVA_SAME_VERSIONS,
    GUAVA_CANDIDATE_NEWER,
    GUAVA_CANDIDATE_OLDER,
}

private fun parseGuavaVersion(version: String): Pair<String?, String?>? {
    val pattern = Regex("^(.*?)(-android|-jre)$").find(version)
    return pattern?.let { Pair(it.groupValues[1], it.groupValues[2]) }
}

private fun isStable(version: String): Boolean {
    val stableKeyword = listOf("RELEASE", "FINAL", "GA").any { it in version.uppercase() }
    val regex = Regex("^[0-9,.v-]+(-r)?$")
    return stableKeyword || regex.matches(version)
}

I run it against my multi-module Android project and I got result that I do not expect:

The following dependencies have later milestone versions:                                                                                                                                    
 - com.google.guava:guava [33.3.0-android -> 33.3.0-jre]                                                                                                                                     
     https://github.com/google/guava                                                                                                                                                         

If you look at the log attached HERE you will see that all calls of rejectVersionIf with current version 33.3.0-android and candidate version 33.3.0-jre, are qualified as GUAVA_DIFFERENT_FLAVORS, therefore the rejectVersionIf lambda returns true, still at the end the plugin proposes this update.

@ben-manes
Copy link
Owner

can you write it into a minimal sample project?

You might consider using jvm-dependency-conflict-resolution for common constraints, like the variant selection.

rynkowsg added a commit to rynkowsg/gradle-versions-plugin-i906 that referenced this issue Sep 17, 2024
Simple project demonstrating the VERSIONS plugin issue.
It assumes running Gradle 8.10.1 and Java 17.

To see problem run:

    ./gradlew dependencyUpdates

The project is prepared in support of reported issue:
ben-manes/gradle-versions-plugin#906
rynkowsg added a commit to rynkowsg/gradle-versions-plugin-i906 that referenced this issue Sep 17, 2024
Simple project demonstrating the VERSIONS plugin issue.
It assumes running Gradle 8.10.1 and Java 17.

To see problem run:

    ./gradlew dependencyUpdates

The project is prepared in support of reported issue:
ben-manes/gradle-versions-plugin#906
rynkowsg added a commit to rynkowsg/gradle-versions-plugin-i906 that referenced this issue Sep 17, 2024
Simple project demonstrating the VERSIONS plugin issue.
It assumes running Gradle 8.10.1 and Java 17.

To see problem run:

    ./gradlew dependencyUpdates

The project is prepared in support of reported issue:
ben-manes/gradle-versions-plugin#906
rynkowsg added a commit to rynkowsg/gradle-versions-plugin-i906 that referenced this issue Sep 17, 2024
Simple project demonstrating the VERSIONS plugin issue.
It assumes running Gradle 8.10.1 and Java 17.

To see problem run:

    ./gradlew dependencyUpdates

The project is prepared in support of reported issue:
ben-manes/gradle-versions-plugin#906
rynkowsg added a commit to rynkowsg/gradle-versions-plugin-i906 that referenced this issue Sep 17, 2024
Simple project demonstrating the VERSIONS plugin issue.
It assumes running Gradle 8.10.1 and Java 17.

To see problem run:

    ./gradlew dependencyUpdates

The project is prepared in support of reported issue:
ben-manes/gradle-versions-plugin#906
@rynkowsg
Copy link
Author

I made a minimal sample project here: rynkowsg/gradle-versions-plugin-i906.

@rynkowsg
Copy link
Author

I'm not familiar with jvm-dependency-conflict-resolution.

Is it something you would apply to the problem shown in the minimal sample project?

@ben-manes
Copy link
Owner

To simplify debugging, I applied the convention plugin to all subprojects so it can be inspected individually:

allprojects {
  apply(plugin = "demo.versions")
}

that led to a small typo via gradle :modules:lib:jvm-library:dependencies

compileClasspath - Compile classpath for 'main'.
+--- org.jetbrains.kotlin:kotlin-stdlib:2.0.20
|    \--- org.jetbrains:annotations:13.0
\--- com.google.guava:guava:33.3.0-jvm FAILED

The individual modules seems fine in their reports

JVM
gradle :modules:lib:jvm-library:dU
executing gradlew instead of gradle

> Configure project :
Applying VersionsConventionPlugin to versions-issue
Applying VersionsConventionPlugin to modules
Applying VersionsConventionPlugin to lib
Applying VersionsConventionPlugin to android-library
Applying VersionsConventionPlugin to jvm-library

> Configure project :modules:lib:android-library
Applying AndroidLibraryConventionPlugin to android-library

> Configure project :modules:lib:jvm-library
Applying JvmLibraryConventionPlugin to jvm-library

> Task :modules:lib:jvm-library:dependencyUpdates

------------------------------------------------------------
:modules:lib:jvm-library Project Dependency Updates (report to plain text file)
------------------------------------------------------------

The following dependencies are using the latest milestone version:
 - com.google.guava:guava:33.3.0-jre
 - org.jetbrains.kotlin:kotlin-build-tools-impl:2.0.20
 - org.jetbrains.kotlin:kotlin-compiler-embeddable:2.0.20
 - org.jetbrains.kotlin:kotlin-klib-commonizer-embeddable:2.0.20
 - org.jetbrains.kotlin:kotlin-native-prebuilt:2.0.20
 - org.jetbrains.kotlin:kotlin-scripting-compiler-embeddable:2.0.20

Gradle release-candidate updates:
 - Gradle: [8.10.1: UP-TO-DATE]
Android
gradle :modules:lib:android-library:dU
executing gradlew instead of gradle

> Configure project :
Applying VersionsConventionPlugin to versions-issue
Applying VersionsConventionPlugin to modules
Applying VersionsConventionPlugin to lib
Applying VersionsConventionPlugin to android-library
Applying VersionsConventionPlugin to jvm-library

> Configure project :modules:lib:android-library
Applying AndroidLibraryConventionPlugin to android-library

> Configure project :modules:lib:jvm-library
Applying JvmLibraryConventionPlugin to jvm-library

> Task :modules:lib:android-library:dependencyUpdates

------------------------------------------------------------
:modules:lib:android-library Project Dependency Updates (report to plain text file)
------------------------------------------------------------

The following dependencies are using the latest milestone version:
 - com.google.guava:guava:33.3.0-android
 - com.google.testing.platform:android-driver-instrumentation:0.0.9-alpha02
 - com.google.testing.platform:android-test-plugin:0.0.9-alpha02
 - com.google.testing.platform:core:0.0.9-alpha02
 - com.google.testing.platform:launcher:0.0.9-alpha02

The following dependencies have later milestone versions:
 - com.android.tools.utp:android-device-provider-ddmlib [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-device-provider-gradle [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-additional-test-output [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-apk-installer [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-coverage [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-device-info [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-emulator-control [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-logcat [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-retention [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-result-listener-gradle [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build

Gradle release-candidate updates:
 - Gradle: [8.10.1: UP-TO-DATE]

The aggregate across all projects is where it seems to combine unintentionally,

Aggregate
gradle :modules:lib:dU
executing gradlew instead of gradle

> Configure project :
Applying VersionsConventionPlugin to versions-issue
Applying VersionsConventionPlugin to modules
Applying VersionsConventionPlugin to lib
Applying VersionsConventionPlugin to android-library
Applying VersionsConventionPlugin to jvm-library

> Configure project :modules:lib:android-library
Applying AndroidLibraryConventionPlugin to android-library

> Configure project :modules:lib:jvm-library
Applying JvmLibraryConventionPlugin to jvm-library

> Task :modules:lib:dependencyUpdates

------------------------------------------------------------
:modules:lib Project Dependency Updates (report to plain text file)
------------------------------------------------------------

The following dependencies are using the latest milestone version:
 - com.google.guava:guava:33.3.0-jre
 - com.google.testing.platform:android-driver-instrumentation:0.0.9-alpha02
 - com.google.testing.platform:android-test-plugin:0.0.9-alpha02
 - com.google.testing.platform:core:0.0.9-alpha02
 - com.google.testing.platform:launcher:0.0.9-alpha02
 - org.jetbrains.kotlin:kotlin-build-tools-impl:2.0.20
 - org.jetbrains.kotlin:kotlin-compiler-embeddable:2.0.20
 - org.jetbrains.kotlin:kotlin-klib-commonizer-embeddable:2.0.20
 - org.jetbrains.kotlin:kotlin-native-prebuilt:2.0.20
 - org.jetbrains.kotlin:kotlin-scripting-compiler-embeddable:2.0.20
 - org.jetbrains.kotlin:kotlin-stdlib:2.0.20

The following dependencies have later milestone versions:
 - com.android.tools.utp:android-device-provider-ddmlib [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-device-provider-gradle [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-additional-test-output [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-apk-installer [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-coverage [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-device-info [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-emulator-control [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-logcat [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-host-retention [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.android.tools.utp:android-test-plugin-result-listener-gradle [31.6.0 -> 31.6.1]
     https://developer.android.com/studio/build
 - com.google.guava:guava [33.3.0-android -> 33.3.0-jre]
     https://github.com/google/guava

Gradle release-candidate updates:
 - Gradle: [8.10.1: UP-TO-DATE]

When I look at the json report, it seems like it didn't really understand that there were two versions and composed that incorrectly.

gron ./modules/lib/build/dependencyUpdates/report.json | rg guava
json.current.dependencies[0].group = "com.google.guava";
json.current.dependencies[0].name = "guava";
json.current.dependencies[0].projectUrl = "https://github.com/google/guava";
json.outdated.dependencies[10].group = "com.google.guava";
json.outdated.dependencies[10].name = "guava";
json.outdated.dependencies[10].projectUrl = "https://github.com/google/guava";

So it seems like something to do with the version mapping logic, since it is atypical to have the same dependency at different versions rather than globally pin it via dependency management.

The jvm-dependency-conflict-resolution plugin is just to help avoid the flavor messiness that your convention plugin has to work around, since it would use capabilities to force a rejection when the configuration and dependency were not compatible. That didn't help your case except would have avoided you writing that logic manually.

rynkowsg added a commit to rynkowsg/gradle-versions-plugin-i906 that referenced this issue Sep 18, 2024
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

2 participants