Feature/177/task: Introduce compat modules #244

mr-archano · 2018-10-28T10:28:17Z

The plugin is currently relying on a custom implementation of an internal Gradle API (SoftwareComponentInternal) in order to provide full support to Android library projects. Specifically we need to implement this internal API if we want to leverage the internal capabilities of the MavenPublishPlugin to produce a correct POM descriptor that includes all the dependencies needed by an Android library, exclusion rules, etc

Unfortunately coding against an internal Gradle API has proven to be quite challenging, as SoftwareComponentInternal has changed its shape few times over time, and our custom implementation is not complying anymore.

The solution proposed here is to create 'compat implementations' of that class using separate modules targeting each a different version of Gradle that we want to support. The compiled compat classes can then automatically added into the final jar of the plugin and we will use reflection to load the correct one according to the Gradle version used at runtime.

This first PR shows how we can achieve this just moving the current implementation in its own plugin/compat/gradle-4_1 module and adding a little of gradle setup to ensure we compile and bundle the classes automatically as needed.

…e with Gradle 4.1

…dule

…ule classpath and jar

…nt compiled with different version of Gradle

mr-archano · 2018-10-28T10:33:08Z

plugin/core/compat.gradle

+    }
+})
+
+Task compatClassesTask = taskContainer.create(name: 'releasePluginCompatClasses', type: DefaultTask) {


This task is used to 'join' all the work needed to compile and include the compat classes in the plugin/core module

What's the minimum version of Gradle you need to support?
You may want to consider using tasks.register if you can use the 4.8+ Gradle API.

Hi @JLLeitschuh thanks for the comment. These tasks here are not part of the plugin code, but just of its builscript, so we could use a more recent version of Gradle and use the new tasks api, as right now the plugin core module is compiled against Gradle 4.3. I will think about it, but it's definitely something that I wouldn't address as part of this tranche of work (that focuses on fixing the compatibility issue of its internals).

mr-archano · 2018-10-28T10:34:03Z

plugin/core/compat.gradle

+
+    def moduleName = moduleDir.name
+
+    def cleanModuleTask = taskContainer.create(name: "clean${moduleName.capitalize()}", type: Delete) {


we add a custom clean task to ensure that we clean all the outputs of each compat module when we invoke :clean on the plugin/core module

Shouldn't each module already have a clean task we could use instead of creating a new one?

they do, but they are also different Gradle projects (see description). The only way to invoke their :clean task is then to run a gradle build from the core module Gradle build, but it sounded overkill considering that we need to just remove a folder

mr-archano · 2018-10-28T10:35:59Z

plugin/core/compat.gradle

+    }
+    taskContainer.clean.dependsOn cleanModuleTask
+
+    def compileModuleTask = taskContainer.create(name: "compile${moduleName.capitalize()}Classes", type: Exec) {


we want to kick-off the compilation of the compat module, but given that is a separate Gradle project we then leverage an ad-hoc Exec task to run the Gradle build for that project (that will use a different version of Gradle for instance)

same here, why do we manually need to create a compile task for each module?

so, the comment above yours was my attempt at highlighting that each module is a separate Gradle project, that is plugin/compat/xyz is not in the same Gradle project as plugin/core.

To invoke the compile task in a separate Gradle project we are leveraging Exec to run a Gradle build from a Gradle build (buildception!)

mr-archano · 2018-10-28T10:37:33Z

plugin/core/compat.gradle

+        commandLine gw, 'clean', 'build'
+    }
+
+    def copyClassesTask = taskContainer.create(name: "copy${moduleName.capitalize()}Classes", type: Copy) {


Once the compat classes are compiled we can move them into a folder inside the plugin/core module. We have picked build/compat/classes so this folder will be wiped automatically at every :clean

Could we not also just change the buildDir of each module?

yeah, but that feels like tampering with the compat projects, and it will be a little unexpected. After all these are not very expensive operations.

mr-archano · 2018-10-28T10:38:59Z

plugin/core/compat.gradle

+sourceSets {
+    main {
+        groovy {
+            output.dir(project.files(compatClassesDestination))


This single line here allows us to bundle the content of build/compat/classes in the artifact of the plugin/core module, so to ship them in the same Jar. This enables the class loader to load them at runtime via reflection without looking for other artifacts.

mr-archano · 2018-10-28T10:41:40Z

plugin/core/compat.gradle

+}
+
+dependencies {
+    compileOnly files(compatClassesTask)


This compileOnly dependency ensures that we trigger the : releasePluginCompatClasses task before we start compiling the sources in plugin/core.
Note that compileOnly dependencies will not pollute the POM descriptor of the plugin.

mr-archano · 2018-10-28T10:42:58Z

plugin/core/src/main/groovy/com/novoda/gradle/release/internal/AndroidAttachments.groovy

-    private static AndroidSoftwareComponent androidComponentFrom(Project project) {
-        return new AndroidSoftwareComponent(project.objects, project.configurations)
+    private static SoftwareComponent androidComponentFrom(Project project) {
+        def clazz = this.classLoader.loadClass(ANDROID_SOFTWARE_COMPONENT_COMPAT_4_1)


Right now we are not checking the current version of Gradle at runtime, we will once we have more compat implementations. (Follow-up PRs coming against the same feature branch)

tobiasheine

That's great, just left a couple of questions.

tobiasheine · 2018-10-30T09:16:35Z

plugin/core/compat.gradle

+
+    def moduleName = moduleDir.name
+
+    def cleanModuleTask = taskContainer.create(name: "clean${moduleName.capitalize()}", type: Delete) {


Shouldn't each module already have a clean task we could use instead of creating a new one?

tobiasheine · 2018-10-30T09:19:13Z

plugin/core/compat.gradle

+    }
+    taskContainer.clean.dependsOn cleanModuleTask
+
+    def compileModuleTask = taskContainer.create(name: "compile${moduleName.capitalize()}Classes", type: Exec) {


same here, why do we manually need to create a compile task for each module?

tobiasheine · 2018-10-30T09:25:23Z

plugin/core/compat.gradle

+        commandLine gw, 'clean', 'build'
+    }
+
+    def copyClassesTask = taskContainer.create(name: "copy${moduleName.capitalize()}Classes", type: Copy) {


Could we not also just change the buildDir of each module?

plugin/compat/common.gradle

plugin/core/build.gradle

…77/task/introduce_compat_module_4_1

…radle 4.1+

mr-archano added 7 commits October 28, 2018 10:19

Add module using Gradle 4.1 API

afc979f

Add placeholder implementation of SoftwareComponentInternal compatibl…

0c2e885

…e with Gradle 4.1

Rename settings file in compat module

b180d9f

Add missing Gradle Wrapper for Gradle 4.1 in compat module

9b0fad7

Copy implementation of existing AndroidSoftwareComponent to compat mo…

bad11d7

…dule

Add setup to automatically compile and add compat classes to core mod…

8766ca0

…ule classpath and jar

Use reflection to initialise compat implementation of SoftwareCompone…

2a4342d

…nt compiled with different version of Gradle

mr-archano assigned tobiasheine, devisnik and ouchadam Oct 28, 2018

mr-archano commented Oct 28, 2018

View reviewed changes

mr-archano added the ❌ do not merge label Oct 28, 2018

tobiasheine approved these changes Oct 30, 2018

View reviewed changes

Mecharyry reviewed Oct 30, 2018

View reviewed changes

plugin/compat/common.gradle Outdated Show resolved Hide resolved

Mecharyry reviewed Oct 30, 2018

View reviewed changes

plugin/core/build.gradle Show resolved Hide resolved

Mecharyry approved these changes Oct 30, 2018

View reviewed changes

mr-archano added 3 commits October 30, 2018 22:29

Merge branch 'feature/177/support_new_gradle_versions' into feature/1…

0e329a2

…77/task/introduce_compat_module_4_1

Add javadoc to document original reference for implementation

3950c39

Use implementation configuration instead of compile as we are using G…

b7d5838

…radle 4.1+

mr-archano removed the ❌ do not merge label Oct 30, 2018

mr-archano merged commit a875fbd into feature/177/support_new_gradle_versions Nov 3, 2018

mr-archano deleted the feature/177/task/introduce_compat_module_4_1 branch November 3, 2018 09:46

This was referenced Nov 5, 2018

Feature/177/task: Support Gradle 4.5 #246

Merged

Feature/177/task: Support Gradle 4.8 #248

Merged

This was referenced Nov 10, 2018

Feature/177: Support new Gradle versions #249

Merged

Get rid of Gradle internal APIs #179

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/177/task: Introduce compat modules #244

Feature/177/task: Introduce compat modules #244

mr-archano commented Oct 28, 2018 •

edited

Loading

mr-archano Oct 28, 2018

JLLeitschuh Oct 30, 2018

mr-archano Oct 30, 2018

mr-archano Oct 28, 2018

tobiasheine Oct 30, 2018

mr-archano Oct 30, 2018

mr-archano Oct 28, 2018

tobiasheine Oct 30, 2018

mr-archano Oct 30, 2018

mr-archano Oct 28, 2018

tobiasheine Oct 30, 2018

mr-archano Oct 30, 2018

mr-archano Oct 28, 2018 •

edited

Loading

mr-archano Oct 28, 2018

mr-archano Oct 28, 2018 •

edited

Loading

tobiasheine left a comment

tobiasheine Oct 30, 2018

tobiasheine Oct 30, 2018

tobiasheine Oct 30, 2018


		def moduleName = moduleDir.name

		def cleanModuleTask = taskContainer.create(name: "clean${moduleName.capitalize()}", type: Delete) {

Feature/177/task: Introduce compat modules #244

Feature/177/task: Introduce compat modules #244

Conversation

mr-archano commented Oct 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mr-archano Oct 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mr-archano Oct 28, 2018 • edited Loading

Choose a reason for hiding this comment

tobiasheine left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mr-archano commented Oct 28, 2018 •

edited

Loading

mr-archano Oct 28, 2018 •

edited

Loading

mr-archano Oct 28, 2018 •

edited

Loading