ben-manes / gradle-versions-plugin

Gradle plugin to discover dependency updates
Apache License 2.0
3.88k stars 201 forks source link

Better default that does not show "-rc" and "-alpha" versions #440

Open lathspell opened 4 years ago

lathspell commented 4 years ago

It would be nice if the default would be to only show updates for "release" class dependencies. Currently the plugin shows, without further configuration, updates like the following which clearly have "alpha" or "rc" in their versons:

I read about the possibility of tweaking the plugin with "rejectVersionIf" but I don't like cluttering every Gradle file with additional functions and filtering "alpha" and "rc" when "revsion=release" is active really should be the default.

ben-manes commented 4 years ago

revsion is an Apache Ivy concept which early versions of Gradle was built on top of. Back then it was the only toggle we had. This only removes snapshots from Maven dependencies because that is their versioning convention. The alpha, rc, etc is considered a stable release version by Maven. This flag is mostly historical and isn’t useful in practice.

You can share build scripts across independent builds, e.g. init scripts. This would avoid your duplication problem.

lathspell commented 4 years ago

My concern was more if the current plugin defaults are sensible? I would guess that most users would usually not want to see alpha and rc versions for their dependencies and consider them as "false-positives". So it would be better if they would be hidden by default.

ben-manes commented 4 years ago

Unfortunately since there is no common convention, I’m afraid if we impose one then it will cause more confusion. The only standard is governed by the repository type, Maven or Ivy, and that is what Gradle applies by default. They then offer configuration knobs to tune those results, so we expose them.

For example some use even/odd versions to designate release and beta versions. Others use keywords, which vary considerably. Some use keywords after a release to indicate patches, e.g. 1.0-SR3 as the 3rd patch after the 1.0-RELEASE version. Some use keywords to split target platforms, e.g. -jre vs -android. Others use timestamp version strings and hack means to backport patches between major releases.

It may appear lazy for us to not provide a sane set of defaults, but the patterns we see in the wild are insane. It seems less confusing therefore to match expectations from Gradle and expose its configuration tools to let users tune it as needed.

If there’s conventions or more advanced tooling, we recommend building a plugin on top of ours too offer those improvements. This way we limit our scope, stay unopinionated, and let those authors explore ideas at a low cost.

lathspell commented 3 years ago

Since then I've tried your plugin in various projects (mostly Java / Kotlin / Spring) and found the following solution as perfect for all of them. It is based on the examples from the README but tries to hide most stuff in a function so that the task definition can be very small.

My hope is that you might include the UpgradeToUnstableFilter in the plugin so that we'd only have the rejectVersionIf(..) in the build.gradle.kts which would make it much cleaner.

open class UpgradeToUnstableFilter : ComponentFilter {
    override fun reject(cs: ComponentSelectionWithCurrent) = reject(cs.currentVersion, cs.candidate.version)

    open fun reject(old: String, new: String): Boolean {
        return !isStable(new) && isStable(old) // no unstable proposals for stable dependencies
    }

    open fun isStable(version: String): Boolean {
        val stableKeyword = setOf("RELEASE", "FINAL", "GA").any { version.toUpperCase().contains(it) }
        val stablePattern = version.matches(Regex("""^[0-9,.v-]+(-r)?$"""))
        return stableKeyword || stablePattern
    }
}

tasks.withType<DependencyUpdatesTask> {
    rejectVersionIf(UpgradeToUnstableFilter())
}
ben-manes commented 3 years ago

That filter is good for the common case, but sadly it is not universal. This is why it is not built in as we want to convey that you are making an assumption which can be wrong in some cases, so you have to be aware of the pattern and able to modify it if you include a dependency with an incompatible version format.

mccartney commented 3 years ago

So the OP is asking to change the default behavior - and there are reasons given on why it shouldn't be the case.

Would it be possible to add this "heuristic-based" behavior as a simple to use option/flag?

ben-manes commented 3 years ago

My concern is that the heuristic is often wrong and it becomes hidden behind a flag which could cause more confusion. This way at least conveys that you are making a guess and can quickly fix it when needed.

mccartney commented 3 years ago

Thanks for taking the time to answer. Gotcha. Makes sense.

Maybe something like this would work:

And then it should be quite easy for everyone to define their own. While your code wouldn't hard-code any assumptions.

ben-manes commented 3 years ago

I imagine you'd want equivalent to,

rejectVersionIf {
  isNonStable(it.candidate.version) && !isNonStable(it.currentVersion)
}

def isNonStable = { String version ->
  def stableKeyword = ['RELEASE', 'FINAL', 'GA'].any { it -> version.toUpperCase().contains(it) }
  def regex = /^[0-9,.v-]+(-r)?$/
  return !stableKeyword && !(version ==~ regex)
}

Since isNonStable is simpler as a function than as a single complex regex, that leans towards accepting isNonStable as the closure property instead of a pattern. Either way, we save only the 3 lines of rejectVersionIf. That doesn't really improve the user experience.

lathspell commented 3 years ago

I guess we can agree that the user experience without any filter is the worst of all as almost all libraries get false proposals due to some -rc or -beta versions so the question is only how to design the version filter so that it is convenient and effective enough.

My idea with the class was that a sensible default is provided but it's easily adjustable just by extending a class and overriding some methods e.g. to extend "RELEASE" and "GA" by say "OFFICIAL". This OOP concept should be easily to convey. For that, the above examples could be split apart even more so that there would be isStable(version), isCandidate(), containsVcsHash() etc. The user then only had to write this (pseudocode) where DefaultVersionFilter contains all the currently proposed rules:

class MyVersionsFilter extends DefaultVersionFilter {
   override fun isStable(version) {
      return super.isStable(version) or version.contains("OFFICIAL")
    }
}

and maybe activate it with

rejectVersionIf(MyVersionsFilter())

That wouldn't be so much different from the currently proposed code just a bit "cleaner" and with some sensible default that maybe still gets 20% wrong but which is better than getting almost everything wrong by default.

G00fY2 commented 3 years ago

I don't think rejecting all non-stable versions really helps. For example if you use an RC and want to see updates to the RC but no beta or alpha releases with a higher version number. Here is what we use to reject updates:

In our build.gradle.kts:

tasks.dependencyUpdates.configure {
  rejectVersionIf {
    DependencyUpdates.maturityLevel(candidate.version) < DependencyUpdates.maturityLevel(currentVersion)
  }
}

In our buildSrc:

object DependencyUpdates {

  private val maturityLevels = listOf("preview", "alpha", "beta", "m", "cr", "rc") // order is important!

  fun maturityLevel(version: String): Int {
    maturityLevels.forEachIndexed { index, s ->
      if (version.matches(".*[.\\-]$s[.\\-\\d]*".toRegex(RegexOption.IGNORE_CASE))) return index
    }
    return maturityLevels.size
  }
}
Philipp91 commented 2 years ago

@G00fY2 I don't understand that function. It returns maturityLevels.size, which is a constant.

ben-manes commented 2 years ago

@Philipp91 The for loop returns the index. The return statement is on the same line as the if, so not nicely formatted.

Philipp91 commented 2 years ago

Ah thanks. I hadn't considered that Kotlin allows returning outer functions from lambdas.

G00fY2 commented 2 years ago

@Philipp91 here is a better readable variant of this function which uses indexOfFirst. The size of the list is the fallback (highest possible level) if no pre-release was found. :slightly_smiling_face:

tasks.dependencyUpdates.configure {
  val qualifiers = listOf("preview", "alpha", "beta", "m", "cr", "rc")
  fun maturityLevel(version: String): Int {
    val index = qualifiers.indexOfFirst { version.matches(".*[.\\-]$it[.\\-\\d]*".toRegex(RegexOption.IGNORE_CASE)) }
    return if (index < 0) qualifiers.size else index
  }

  rejectVersionIf { maturityLevel(candidate.version) < maturityLevel(currentVersion) }
}
Philipp91 commented 2 years ago

Thanks! Yes that makes it more easily understandable. I had in the meanwhile transformed it to this, which uses the default -1 as the fallback and doesn't need the extra object:

tasks.named("dependencyUpdates", com.github.benmanes.gradle.versions.updates.DependencyUpdatesTask::class).configure {
    val immaturityLevels = listOf("rc", "cr", "m", "beta", "alpha", "preview") // order is important
    val immaturityRegexes = immaturityLevels.map { ".*[.\\-]$it[.\\-\\d]*".toRegex(RegexOption.IGNORE_CASE) }
    fun immaturityLevel(version: String): Int = immaturityRegexes.indexOfLast { version.matches(it) }
    rejectVersionIf { immaturityLevel(candidate.version) > immaturityLevel(currentVersion) }
}
DPGLuke commented 1 year ago

Thanks for the super work on maturityLevel everyone! I've also ported it to groovy for those not using kts:

/**
 * Returns an int representing how mature [version] is.
 * Higher numbers are more mature.
 */
static def maturityLevel(String version) {
    /**
     * Version qualifiers, in order from least to most mature.
     * The most mature is to have no qualifier at all.
     */
    def qualifiers = ["preview", "alpha", "beta", "m", "cr", "rc"]
    def qualifiersRegex = qualifiers.collect { /(?i).*[.\-]$it[.\-\d]*/ }

    def index = qualifiersRegex.findIndexOf { version ==~ it }
    return (index < 0) ? qualifiers.size : index
}

tasks.named("dependencyUpdates").configure {
    rejectVersionIf {
        def candidateMaturity = maturityLevel(it.candidate.version)
        def currentMaturity = maturityLevel(it.currentVersion)
        candidateMaturity < currentMaturity
    }
}
G00fY2 commented 1 year ago

@ben-manes Does it make sense to at least add the Kotlin and Groovy maturityLevel solution to the readme? Maybe as part of the RejectVersionsIf section?

I think initially this issues was about adding it to the plugin itself. But adding it to the readme should be sufficient. I could create a PR for that.

ben-manes commented 1 year ago

@G00fY2 Sure!