ben-manes / gradle-versions-plugin

Gradle plugin to discover dependency updates
Apache License 2.0
3.89k stars 200 forks source link

plugins with apply false not checked #293

Closed azell closed 4 years ago

azell commented 5 years ago

See https://github.com/azell/modern_rpc for a reproducible project. The top-level build.gradle file contains:

plugins {
  id 'com.diffplug.gradle.spotless' version '3.18.0' apply false
  id 'com.github.ben-manes.versions' version '0.20.0' apply false
  id 'com.github.johnrengelman.shadow' version '4.0.4' apply false
  id 'com.google.protobuf' version '0.8.8' apply false
  id 'io.spring.dependency-management' version '1.0.6.RELEASE' apply false
}

and rsocket/build.gradle:

plugins {
  id 'application'
  id 'com.github.johnrengelman.shadow'
  id 'com.google.protobuf'
  id 'io.spring.dependency-management'
}

Even if the above plugins have newer versions, the gradle-versions-plugin does not detect it. If I remove the plugin statements from the top-level build.gradle and move them to the child build.gradle with version information, then plugin upgrades are detected.

build.gradle:

plugins {
  id 'com.diffplug.gradle.spotless' version '3.18.0' apply false
  id 'com.github.ben-manes.versions' version '0.20.0' apply false
  id 'com.github.johnrengelman.shadow' version '4.0.4' apply false
  id 'com.google.protobuf' version '0.8.8' apply false
  // id 'io.spring.dependency-management' version '1.0.6.RELEASE' apply false
}

rsocket/build.gradle:

plugins {
  id 'application'
  id 'com.github.johnrengelman.shadow'
  id 'com.google.protobuf'
  id 'io.spring.dependency-management' version '1.0.5.RELEASE'
}
ben-manes commented 5 years ago

I wonder if this is related to https://github.com/ben-manes/gradle-versions-plugin/issues/283#issuecomment-451815851, which the Gradle team stated in a similar issue

this is expected, because script plugins (e.g. publishing.gradle) and project build scripts do not share a classloader hierarchy

Since the plugins added in the script files are not visible, iterating over the project configurations won't work. By placing ours in the script, we're brought into this privileged view. That would mean we might not have a direct way to handle this since it is intentionally restricted.

Ideally apply false would add the dependency to the root's buildscript.classpath configuration. Then it should be picked up regardless of whether it was applied. I don't know why that doesn't work, but again they do funky things. I use the following trick instead of the plugins block so that my plugin versions are located in with all of my other dependency definitions.

buildscript {
  apply from: "${rootDir}/gradle/dependencies.gradle"

  repositories {
    gradlePluginPortal()
  }

  dependencies {
    classpath gradlePlugins.values()
  }
}

where dependencies.gradle includes the manifest of versions and coordinates.

azell commented 5 years ago

This is the plugin pattern I am emulating: https://docs.gradle.org/current/userguide/plugins.html#sec:subprojects_plugins_dsl

If you have a multi-project build, you probably want to apply plugins to some or all of the subprojects
in your build, but not to the root or master project. The default behavior of the plugins {} block is to
immediately resolve and apply the plugins. But, you can use the apply false syntax to tell Gradle not to
apply the plugin to the current project and then use apply plugin: «plugin id» in the subprojects block
or use the plugins {} block in sub projects build script:

where the gradlePlugins.values() strategy would presumably force them to be immediately resolved in the root.

ben-manes commented 5 years ago

Nope, it should have the same effect of resolving them immediately. You still have to apply them, which I do in the appropriate places (e.g. codeQuality sets up error-prone). I would have expected the plugins block to be a DSL for this stuff, but early on it didn't follow maven repository / pom conventions so they definitely did things non-standard and magical, and then backtracked.

ben-manes commented 5 years ago

Sorry, I should clarify that. Yes gradlePlugins.values() would force them to be resolved in the root, but not applied. This should be the same as the plugins with apply false since it has to resolve the dependency for the classpath.

PluginDependencySpec javadoc

Specifies whether the plugin should be applied to the current project. Otherwise it is only put on the project's classpath.

However this is probably hidden from us due to being in the script and not project (see PluginDependenciesSpec doc](https://docs.gradle.org/current/dsl/org.gradle.plugin.use.PluginDependenciesSpec.html)

The plugins {} block serves a similar purpose to the PluginAware.apply(java.util.Map) method that can be used to apply a plugin directly to a Project object or similar. A key difference is that plugins applied via the plugins {} block are conceptually applied to the script, and by extension the script target. At this time there is no observable practical difference between the two approaches with regard to the end result.

As they say the difference doesn't effect the end result, except that I don't think it is available through their APIs. If correct, then this would impact the observable result but only in edge cases like ours.

azell commented 5 years ago

So if I understand correctly, it is not possible at present to track plugin changes using the plugins DSL. Plugin version changes can be detected iff using the legacy buildscript and apply syntax. Yes/no?

ben-manes commented 5 years ago

I think we can't track if the plugin is defined in a script file, rather than the root build file. That's hidden for plugins and buildscript blocks, if I understand correctly. I don't know why the apply clause is an issue, though.