Skip to content

Conversation

bryceatmoderne
Copy link
Contributor

@bryceatmoderne bryceatmoderne commented Aug 22, 2024

What's changed?

  • Updated rewrite-gradle-plugin to read android project source sets using the Android Gradle Plugin.
  • in order to avoid ClassCastExceptions when loading AGP classes using RewriteClassLoader the class loader of the AGP plugin instance is now passed in the RewriteClassLoader constructor and all com.android... classes are loaded using it
  • all logic to read android source sets is encapsulated in AndroidProjectParser which is lazily allocated to ensure rewrite-gradle-plugin tasks do not attempt to load AGP classes for non-android projects
  • some reflection was required to accommodate different types across AGP versions; for example see AndroidProjectCompileOptions
  • these changes have been tested using AGP versions 8.6.0 to 3.3.0
  • the compileOnly dependency of AGP v 7.0.4 was choses as it was the last version to support JDK 8 which rewrite-gradle-plugin targets
  • note that despite a new AGP Variant API, the majority of classes and methods required to read android source sets have remained constant between versions 3.3.0 through 8.6.0 despite some being deprecated (and their documented removed from reference guides). Once google decides to remove these classes in a future AGP release, a new approach will need to be considered to support android source sets

What's your motivation?

These changes will allow rewrite-gradle-plugin to run recipes on android project repositories.

Anything in particular you'd like reviewers to focus on?

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

I tried several approaches include adding AGP jars to RewriteClassLoader but could not avoid ClassCastExceptionss when assigned gradle decorated AGP classes to ones loaded by RewriteClassLoader

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@bryceatmoderne bryceatmoderne self-assigned this Aug 22, 2024
@bryceatmoderne bryceatmoderne marked this pull request as draft August 22, 2024 00:48
return sourceFileStream;
}

private SourceFileStream parseAndroidProjectSourceSets(Project subproject,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

callout: Future problem will almost certainly be needing an Android specific marker to carry various information such as the min and max SDK versions, etc.

@bryceatmoderne bryceatmoderne added the enhancement New feature or request label Aug 31, 2024
@bryceatmoderne bryceatmoderne marked this pull request as ready for review September 4, 2024 15:55
@sambsnyd sambsnyd merged commit 8139f0e into main Sep 4, 2024
1 check passed
@sambsnyd sambsnyd deleted the android-sourceset-support branch September 4, 2024 17:34
@shanman190
Copy link
Collaborator

For the OSS build plugins, the plugin classloader part wouldn't be necessary. However, when testing via the tooling API or loading the plugin via the initscript, it results in a different location as to where to find plugins. To illustrate this a bit more, consider the typical classloader hierarchy below for a Gradle project:

JVM
  Gradle
    Initscript
      Settings
        Root Project
          Subproject A
            Subprojects A.1
              ...
            Subprojects A.2
              ...
          Subproject B
            Subproject B.1
              ...
            Subproject B.2
              ...
          ...

For a moment, let's just focus in on where we would find RewriteClassLoader and Android Gradle Plugin's BaseAppModuleExtension to make things simplier.

When running via the OSS build plugins applied to the root project, then both classes would appear in the "Root Project" classloader above. When they are both in "Subproject A", they would both appear within the "Subproject A" classloader. Now if the OSS build plugin is at the root, but AGP is in "Subproject A", the plugin classloader logic here is necessary because the class is in a child classloader rather than a parent classloader. When the OSS build plugin is applied via an Initscript, it's now a 4th generational parent from the AGP class.

As a result of the placement of the various jars into each of their classloaders for the necessary classpath isolation, this is what is resulting in the differing behavior and ultimately the necessity of loading the AGP classes from a particular classloader. This actually compounds with each additional plugin and is probably one case where Kotlin Multiplatform may not be working fully as an example.

@bryceatmoderne
Copy link
Contributor Author

For the OSS build plugins, the plugin classloader part wouldn't be necessary. However, when testing via the tooling API or loading the plugin via the initscript, it results in a different location as to where to find plugins. To illustrate this a bit more, consider the typical classloader hierarchy below for a Gradle project:

JVM
  Gradle
    Initscript
      Settings
        Root Project
          Subproject A
            Subprojects A.1
              ...
            Subprojects A.2
              ...
          Subproject B
            Subproject B.1
              ...
            Subproject B.2
              ...
          ...

For a moment, let's just focus in on where we would find RewriteClassLoader and Android Gradle Plugin's BaseAppModuleExtension to make things simplier.

When running via the OSS build plugins applied to the root project, then both classes would appear in the "Root Project" classloader above. When they are both in "Subproject A", they would both appear within the "Subproject A" classloader. Now if the OSS build plugin is at the root, but AGP is in "Subproject A", the plugin classloader logic here is necessary because the class is in a child classloader rather than a parent classloader. When the OSS build plugin is applied via an Initscript, it's now a 4th generational parent from the AGP class.

As a result of the placement of the various jars into each of their classloaders for the necessary classpath isolation, this is what is resulting in the differing behavior and ultimately the necessity of loading the AGP classes from a particular classloader. This actually compounds with each additional plugin and is probably one case where Kotlin Multiplatform may not be working fully as an example.

Thank you for these insights @shanman190. Much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants