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

Simplified resolutionStrategy #325

Open
jmfayard opened this issue Aug 22, 2019 · 26 comments
Open

Simplified resolutionStrategy #325

jmfayard opened this issue Aug 22, 2019 · 26 comments
Assignees

Comments

@jmfayard
Copy link
Collaborator

jmfayard commented Aug 22, 2019

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
Copy link
Owner

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

jmfayard commented Aug 22, 2019

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

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

@jmfayard
Copy link
Collaborator Author

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
Copy link
Owner

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

jmfayard commented Sep 4, 2019

@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
Copy link
Owner

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

jmfayard commented Sep 4, 2019

That's wise :)

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

@jmfayard jmfayard self-assigned this Sep 4, 2019
@ben-manes
Copy link
Owner

Oh yeah, please do :)

@ferinagy
Copy link
Collaborator

ferinagy commented Sep 5, 2019

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.

jmfayard pushed a commit to Splitties/refreshVersions that referenced this issue Sep 6, 2019
Cannot change dependencies of configuration ':classpath' after it has been resolved.

https://gist.github.com/jmfayard/f8455c2cdc658035e8d542ecf0f841ac
jmfayard pushed a commit that referenced this issue Sep 6, 2019
jmfayard pushed a commit to jmfayard/gradle-versions-plugin that referenced this issue Sep 9, 2019
jmfayard pushed a commit that referenced this issue Sep 10, 2019
Implements #325 rejectVersionIf { ... }
@ben-manes
Copy link
Owner

Thanks @jmfayard! :)

jmfayard pushed a commit to Splitties/refreshVersions that referenced this issue Sep 10, 2019
@jmfayard
Copy link
Collaborator Author

@ben-manes
Copy link
Owner

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
Copy link
Owner

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

@k3muri84
Copy link

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

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

@ben-manes
Copy link
Owner

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
Copy link
Owner

@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
Copy link
Collaborator Author

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
Copy link
Owner

ben-manes commented Sep 19, 2019

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
Copy link
Owner

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

@bric3
Copy link

bric3 commented Sep 24, 2019

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
Copy link
Owner

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

@bric3
Copy link

bric3 commented Sep 24, 2019

@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
Copy link
Collaborator Author

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

@bric3
Copy link

bric3 commented Sep 26, 2019

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')
        }
      }
    }
  }
}

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

6 participants