ben-manes / gradle-versions-plugin

Gradle plugin to discover dependency updates
Apache License 2.0
3.86k stars 199 forks source link

Simplified resolutionStrategy #325

Open jmfayard opened 5 years ago

jmfayard commented 5 years ago

Update: Released in v0.25.0

Hello Ben,

I propose to add a declarative filter for filtering version

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

instead of

  resolutionStrategy {
    componentSelection {
      all {
        if (isNonStable(candidate.version) && !isNonStable(currentVersion)) {
          reject("Release candidate")
        }
      }
    }
  }

Also I noticed that people are much more creative on how they name their unstable versions than on how they name their unstable versions.

So the README should recommand something like:

fun isNonStable(version: String): Boolean {
  val stableKeyword = listOf("RELEASE", "FINAL", "GA").any { version.toUpperCase().contains(it) }
  val regex = "^[0-9,.v-]+$".toRegex()
  val isStable = stableKeyword || regex.matches(version)
  return isStable.not()
}
ben-manes commented 5 years ago

Thanks, it’s very nice. Unfortunately it has to be empty by default. We don’t want to change behavior so opt-in rather than your opt-out.

jmfayard commented 5 years ago

Yes, that was my intention as well. Leave it empty by default, but in the README document first this solution. For those using my plug-in, it will be an opt-out like now

chadlwilson commented 5 years ago

This looks quite nice - quite a bit cleaner than the current approach.

jmfayard commented 5 years ago

i was thinking how to integrate it with the newest feature

we could have


rejectVersionContaining ("alpha","beta",...)

which would be a shorter version of


rejectVersionIf { candidate ->

listOf("alpha", "beta", "rc", "cr", "m", "preview", "b", "ea").any { qualifier ->

candidate.version.matches(Regex("(?i).*[.-]\$qualifier[.\\d-+]*"))

}

}

which would be a shorter version of


resolutionStrategy {

componentSelection {

all {

fun isNonStable(version: String) = listOf("alpha", "beta", "rc", "cr", "m", "preview", "b", "ea").any { qualifier ->

version.matches(Regex("(?i).*[.-]\$qualifier[.\\d-+]*"))

}

if (isNonStable(candidate.version) && !isNonStable(currentVersion)) {

reject("Release candidate")

}

}

}

}
ben-manes commented 5 years ago

Yes, that sounds very nice.

Please note that @ghus-raba and I need to debug #331 a bit to fix backwards compatibility mistakes. So the newest feature might be a little in flux, as in it might be a very trivial fix or maybe we broke the general contract. I'm hopeful that it is only a minor mistake and I'll dig into it over the weekend. So please don't make any firm commitments in your code until we've worked that issue out.

jmfayard commented 5 years ago

@ben-manes I just had a random thought under the shower. People are super creative with how they name their unstable versions, but they are not creative on how they name their stable versions.

A GitHub code search on Versions.kt shows it: https://github.com/search?o=desc&p=2&q=filename%3AVersions.kt&s=indexed&type=Code

As far I can see it would be be quite doable to write the right function fun Candidate.isStableVersion(): Boolean with a simple RegExp (numbers + dot + some characters like 'v') plus looking up for some keywords (like RELEASE). Not tested but should be something like this:

fun isStableVersion(version: String): Boolean {
    val stableKeyword = listOf("RELEASE", "FINAL").any { version.contains(it) }
    val regex = "^[0-9,.v-]+$".toRegex()
    return stableKeyword || regex.matches(version)
}
ben-manes commented 5 years ago

True, I'm fine switching the example and if you want to use that in your simplified setting. Maven treats GA and FINAL as aliases, but not RELEASE. Some projects annoyingly use even/odd versions to distinguish between release/beta. I think we should be careful not to get into being the owners of this list and always defer to users who will have some awkward scenario.

jmfayard commented 5 years ago

That's wise :)

With version 0.24.0 being released, I can now work on this right?

ben-manes commented 5 years ago

Oh yeah, please do :)

ferinagy commented 5 years ago

Just for inspiration, in our project I use:

resolutionStrategy {
    componentSelection {
        all {
            fun String.stability(): Int = when {
                this.contains("alpha") -> 0
                this.contains("beta") -> 1
                this.contains("rc") -> 2
                else -> 3
            }

            if (candidate.version.stability() < currentVersion.stability()) {
                reject("Do not offer less stable version")
            }
        }
    }
}

It is not catching every possible naming convention, but it works for our dependencies, is pretty easily extensible and offers updates to more stable version in case you use some alpha/beta because you take the risk and do not want to wait until it becomes stable.

ben-manes commented 5 years ago

Thanks @jmfayard! :)

jmfayard commented 5 years ago

@ben-manes the gradlePluginPortal shows version 0.24.0 while mavenCentral has 0.25.0 https://plugins.gradle.org/plugin/com.github.ben-manes.versions https://bintray.com/fooberger/maven/com.github.ben-manes%3Agradle-versions-plugin/0.25.0

ben-manes commented 5 years ago

Yes, it takes a little while for the plugin portal to be sync'd. The portal is in fact s JCenter repository anyway. We use that integration, which was the required setup when the plugin portal first launched. Probably now you can deploy directly to the plugin portal if starting from scratch.

ben-manes commented 5 years ago

Just checked, and its on the plugin portal now https://plugins.gradle.org/plugin/com.github.ben-manes.versions

k3muri84 commented 5 years ago

the simplified strategy isn't working for multi-module project. all my dependencies fail and i have to use the full syntax from example 3.

chadlwilson commented 5 years ago

Didn't work for me either on a single-module project; I also had to use example 3.

ben-manes commented 5 years ago

When I try the examples/groovy I get the error:

Caused by: groovy.lang.MissingPropertyException: Could not get unknown property 'candidate' for task ':dependencyUpdates' of type com.github.benmanes.gradle.versions.updates.DependencyUpdatesTask.
  ...
  at com.github.benmanes.gradle.versions.updates.resolutionstrategy.ComponentFilter$reject.call(Unknown Source)
  at com.github.benmanes.gradle.versions.updates.DependencyUpdatesTask$_rejectVersionIf_closure2$_closure3$_closure4.doCall(DependencyUpdatesTask.groovy:102)

It works if I explicitly qualify candidate, e.g.

rejectVersionIf { 
  isNonStable(it.candidate.version)
}

Perhaps this means that the closure's delegate isn't being set up?

ben-manes commented 5 years ago

@ghus-raba Any chance you know how to fix the above issue? I haven't dug in, but I think it's like the trick you did before to make it the closure delegate.

jmfayard commented 5 years ago

Doesn't it depend on the Gradle version? examples/groovy did work for me when I was using current Gradle, before I refactored it to use the Gradle wrapper of the plug-in

ben-manes commented 5 years ago

5.6.2 fails for me if you disable Example 3. Since they are reassigning the task variable its not accumulative, but replacement definitions.

ben: gradle-versions-plugin $ ./gradlew -p examples/groovy dependencyUpdate
Downloading https://services.gradle.org/distributions/gradle-5.6.2-bin.zip
.........................................................................................

Welcome to Gradle 5.6.2!

Here are the highlights of this release:
 - Incremental Groovy compilation
 - Groovy compile avoidance
 - Test fixtures for Java projects
 - Manage plugin versions via settings script

For more details see https://docs.gradle.org/5.6.2/release-notes.html

Starting a Gradle Daemon (subsequent builds will be faster)

> Task :dependencyUpdates

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

Failed to determine the latest version for the following dependencies (use --info for details):
 - backport-util-concurrent:backport-util-concurrent
 - backport-util-concurrent:backport-util-concurrent-java12
 - com.github.ben-manes:gradle-versions-plugin
 - com.github.ben-manes:unresolvable
 - com.github.ben-manes:unresolvable2
 - com.google.code.gson:gson
 - com.google.guava:guava
 - com.google.guava:guava-tests
 - com.google.inject:guice
 - com.google.inject.extensions:guice-multibindings
 - dom4j:dom4j
 - org.springframework.boot:spring-boot-dependencies

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

Generated report file build/dependencyUpdates/report.txt

Deprecated Gradle features were used in this build, making it incompatible with Gradle 6.0.
Use '--warning-mode all' to show the individual deprecation warnings.
See https://docs.gradle.org/5.6.2/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 13s
1 actionable task: 1 executed
ben-manes commented 5 years ago

I just tried again and cannot reproduce it anymore in the examples.

bric3 commented 5 years ago

EDIT, the readme is wrong it seems, you need to name the context object e.g. selection and access candidate or currentVersion form the selection object. And I just noticed the it trick.


Got the same issue with the missing property, using the script configured as this.

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

dependencyUpdates {
    revision = "release"

    rejectVersionIf {
        isNonStable(candidate.version) && !isNonStable(currentVersion)
    }
}
stacktrace ``` Caused by: groovy.lang.MissingPropertyException: Could not get unknown property 'candidate' for task ':dependencyUpdates' of type com.github.benmanes.gradle.versions.updates.DependencyUpdatesTask. at org.gradle.internal.metaobject.AbstractDynamicObject.getMissingProperty(AbstractDynamicObject.java:84) at org.gradle.internal.metaobject.ConfigureDelegate.getProperty(ConfigureDelegate.java:130) at org.codehaus.groovy.runtime.InvokerHelper.getProperty(InvokerHelper.java:190) at groovy.lang.Closure.getProperty(Closure.java:298) at org.codehaus.groovy.runtime.InvokerHelper.getProperty(InvokerHelper.java:190) at groovy.lang.Closure.getPropertyTryThese(Closure.java:319) at groovy.lang.Closure.getPropertyOwnerFirst(Closure.java:313) at groovy.lang.Closure.getProperty(Closure.java:302) at org.codehaus.groovy.runtime.callsite.PogoGetPropertySite.getProperty(PogoGetPropertySite.java:49) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callGroovyObjectGetProperty(AbstractCallSite.java:309) at dependencies_help_5g5jr2o0p3sgnk3afqa3iojhi$_run_closure3$_closure11.doCall(/Users/bric3/blablacar/services/edge-api/gradle/dependencies-help.gradle:38) at jdk.internal.reflect.GeneratedMethodAccessor74.invoke(Unknown Source) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:104) at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:326) at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:264) at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1041) at groovy.lang.Closure.call(Closure.java:411) at org.codehaus.groovy.runtime.ConvertedClosure.invokeCustom(ConvertedClosure.java:50) at org.codehaus.groovy.runtime.ConversionHandler.invoke(ConversionHandler.java:122) at com.sun.proxy.$Proxy68.reject(Unknown Source) at com.github.benmanes.gradle.versions.updates.resolutionstrategy.ComponentFilter$reject.call(Unknown Source) at com.github.benmanes.gradle.versions.updates.DependencyUpdatesTask$_rejectVersionIf_closure2$_closure3$_closure4.doCall(DependencyUpdatesTask.groovy:102) at jdk.internal.reflect.GeneratedMethodAccessor72.invoke(Unknown Source) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:104) at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:326) at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:264) at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1041) at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.call(PogoMetaClassSite.java:41) at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:127) at com.github.benmanes.gradle.versions.updates.resolutionstrategy.ComponentSelectionRulesWithCurrent$2.execute(ComponentSelectionRulesWithCurrent.groovy:37) at com.github.benmanes.gradle.versions.updates.resolutionstrategy.ComponentSelectionRulesWithCurrent$2.execute(ComponentSelectionRulesWithCurrent.groovy) at org.gradle.internal.rules.NoInputsRuleAction.execute(NoInputsRuleAction.java:38) at org.gradle.api.internal.artifacts.ivyservice.ivyresolve.ComponentSelectionRulesProcessor.processRule(ComponentSelectionRulesProcessor.java:84) ```

Gradle 5.6.2 (with the wrapper of course) Running on Java 11

ben-manes commented 5 years ago

Can you delete your .gradle dir? I had done a git clean -dfx and killed the daemons. Then it worked.

bric3 commented 5 years ago

@ben-manes thanks it seems most of my issues were related to the context object ComponentSelectionWithCurrent by mixing example in the README and found in this issue.

Although my script only the last rejectVersionIf directive is executed, in that aspect the readme seems a bit misleading.

jmfayard commented 5 years ago

What's the status of this? Is the README wrong or the feature buggy or both? @bric3 see #344

bric3 commented 5 years ago

The readme is definitely wrong. I don't think there's a bug in the feature but rather a limitation of how rejectVersionIf currently works, i.e. at this. time it's impossible to have multiple rejectVersionIf directive. In that aspect the readme is wrong, as it let assume that the directive just adds new rejection directive, but they are overriding each other.

The wrong example snippet from the current readme:

dependencyUpdates {
  // Example 1: reject all non stable versions
  rejectVersionIf {
    isNonStable(candidate.version)
  }

  // Example 2: disallow release candidates as upgradable versions from stable versions
  rejectVersionIf {
    isNonStable(candidate.version) && !isNonStable(currentVersion)
  }

  // Example 3: using the full syntax
  resolutionStrategy {
    componentSelection {
      all {
        if (isNonStable(candidate.version) && !isNonStable(currentVersion)) {
          reject('Release candidate')
        }
      }
    }
  }
}

Instead this snippet should be rewritten like this (and maybe for the kotlin snippet as well, I didn't check):

  // Example 1: reject all non stable versions
dependencyUpdates {
  rejectVersionIf { selection ->
    isNonStable(selection.candidate.version) // <---- notice the arg name
  }
}
// Example 2: disallow release candidates as upgradable versions from stable versions
dependencyUpdates {
  rejectVersionIf {
    isNonStable(it.candidate.version) && !isNonStable(it.currentVersion) // <---- notice the it (standard groovy)
  }
}
// Example 3: using the full syntax
dependencyUpdates {
  resolutionStrategy {
    componentSelection {
      all {
        if (isNonStable(it.candidate.version) && !isNonStable(it.currentVersion)) {
          reject('Release candidate')
        }
      }
    }
  }
}