Skip to content

fix: Add shadow plugin #6

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from
Open

fix: Add shadow plugin #6

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ea5affe
Support shadow plugin
MarkusTieger Jul 5, 2025
bd48b21
Improved a comment
MarkusTieger Jul 7, 2025
fe15805
Fix jar task wasn't given thin classifier
MarkusTieger Jul 8, 2025
3b4db80
Apply suggestions from code review
oSumAtrIX Jul 8, 2025
03a6326
Disable jar task instead of overwriting the assemble task dependencies
MarkusTieger Jul 13, 2025
3aeaf4e
Readd thin classifier, as maven publishications fails otherwise, desp…
MarkusTieger Jul 13, 2025
b0c8a4a
Use app.revanced.patches as relocation prefix
MarkusTieger Sep 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ hs_err_pid*

# Generated files
.idea/**/contentModel.xml
.idea/**/misc.xml
.idea/**/vcs.xml
.idea/**/kotlinc.xml
.idea/**/.gitignore

# Sensitive or high-churn files
.idea/**/dataSources/
Expand Down
2 changes: 2 additions & 0 deletions build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ plugins {
group = "app.revanced"

repositories {
gradlePluginPortal()
mavenCentral()
google()
}
Expand All @@ -21,6 +22,7 @@ dependencies {
implementation(libs.guava)
implementation(libs.kotlin)
implementation(libs.kotlin.android)
implementation(libs.shadow)

implementation(gradleApi())
implementation(gradleKotlinDsl())
Expand Down
2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ binary-compatibility-validator = "0.15.1"
#noinspection GradleDependency
agp = "8.2.2" # 8.3.0 causes Java verifier error: https://github.com/ReVanced/revanced-patches/issues/2818
guava = "33.2.1-jre"
shadow = "8.1.1"

[libraries]
binary-compatibility-validator = { module = "org.jetbrains.kotlinx.binary-compatibility-validator:org.jetbrains.kotlinx.binary-compatibility-validator.gradle.plugin", version.ref = "binary-compatibility-validator" }
kotlin = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "kotlin" }
kotlin-android = { module = "org.jetbrains.kotlin.android:org.jetbrains.kotlin.android.gradle.plugin", version.ref = "kotlin" }
android-application = { module = "com.android.application:com.android.application.gradle.plugin", version.ref = "agp" }
guava = { module = "com.google.guava:guava", version.ref = "guava" }
shadow = { group = "com.github.johnrengelman", name = "shadow", version.ref = "shadow" }

[plugins]
kotlin = { id = "org.jetbrains.kotlin.jvm", version.ref = "kotlin"}
Expand Down
42 changes: 40 additions & 2 deletions src/main/kotlin/app/revanced/patches/gradle/PatchesPlugin.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import com.android.tools.r8.D8
import com.android.tools.r8.D8Command
import com.android.tools.r8.OutputMode
import com.android.tools.r8.utils.ArchiveResourceProvider
import com.github.jengelman.gradle.plugins.shadow.ShadowPlugin
import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar
import kotlinx.validation.BinaryCompatibilityValidatorPlugin
import org.gradle.api.JavaVersion
import org.gradle.api.Plugin
Expand All @@ -32,6 +34,7 @@ abstract class PatchesPlugin : Plugin<Project> {
project.configureDependencies()
project.configureKotlin()
project.configureJava()
project.configureShadow()
project.configureBinaryCompatibilityValidator()
project.configureConsumeExtensions(extension)
project.configureJarTask(extension)
Expand Down Expand Up @@ -85,6 +88,13 @@ abstract class PatchesPlugin : Plugin<Project> {
}
}

/**
* Configures the shadow plugin
*/
private fun Project.configureShadow() {
pluginManager.apply(ShadowPlugin::class.java)
}

/**
* Applies the binary compatibility validator plugin to the project, because patches have a public API.
*/
Expand Down Expand Up @@ -115,12 +125,16 @@ abstract class PatchesPlugin : Plugin<Project> {
task.description = "Builds the project for Android by compiling to DEX and adding it to the patches file."
task.group = "build"

task.dependsOn(tasks["jar"])
task.dependsOn(tasks["shadowJar"])
Copy link
Member

@oSumAtrIX oSumAtrIX Jul 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

How about keeping everything the same around here and just making jar depend on shadow jar task? Looks simpler to me, as JAR already handles everything. Only the classifier and extension needs to be set and signing or publication configured

Copy link
Author

Choose a reason for hiding this comment

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

The thin (not shaded) jar would be additionally built then.

Copy link
Member

Choose a reason for hiding this comment

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

When I run shadowJar in revanced-cli it already builds the thin jar. So what is different?

Copy link
Author

Choose a reason for hiding this comment

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

Idk. Here it doesn't.

To be honest, I don't know myself why it isn't here the case here, but in revanced-cli it is.

Feel free to test it yourself.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe because of the "application" plugin:

explorer_AIRg4slSFh.mp4


// Add these manually, as the task does not depend on the "jar" task (no one needs a thin jar).
task.dependsOn(tasks.named("javadocJar"))
task.dependsOn(tasks.named("sourcesJar"))

task.doLast {
val workingDirectory = layout.buildDirectory.dir("revanced").get().asFile.also(File::mkdirs)

val patchesFile = tasks["jar"].outputs.files.first()
val patchesFile = tasks["shadowJar"].outputs.files.first()
val classesZipFile = workingDirectory.resolve("classes.zip")

D8Command.builder()
Expand Down Expand Up @@ -229,6 +243,13 @@ abstract class PatchesPlugin : Plugin<Project> {
private fun Project.configureJarTask(patchesExtension: PatchesExtension) {
tasks.withType(Jar::class.java).configureEach {
it.archiveExtension.set("rvp")

// JarTask includes the javadoc and sourcesjar. Be sure to only add the "-thin" filename suffix, if there isn't already one set
if (it.archiveClassifier.orNull.isNullOrEmpty()) {
it.archiveClassifier.set("thin")
Copy link
Member

Choose a reason for hiding this comment

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

What is the purpose of this

Copy link
Author

@MarkusTieger MarkusTieger Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The shaded jar has the classifier set to "null" (see below). Therefore it has the filename "patches-5.30.0.rvp". This could conflict with the jar task. I therefore set the archiveClassifier to "thin", which results in the filename "patches-5.30.0-thin.rvp" if somebody runs "./gradlew jar" (the non-shaded jar, it is not run by default). I first check if it is null, if not it is the "javadocJar" (patches-5.30.0-javadoc.rvp) and "sourcesJar" (patches-5.3.0.0-sources.rvp) which should not be affected by this change.

Copy link
Member

Choose a reason for hiding this comment

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

shadowJar already has an classifier:

image

It does not conflict with the JAR task.
Personally I'd like it to not have the -all classifier though. It should produce the artifact name same as JAR and overwrite it, but I don't know if this is good Gradle convention, or if that even works.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it has the "all" classifier by default. I changed that and removed it, so "revanced-patches-5.30.0.rvp" is the shadowJar and "revanced-patches-5.30.0-thin.rvp" is the normal jar (which is not expected to be built by default).

Copy link
Member

Choose a reason for hiding this comment

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

I think the JAR task and shadowJar task should produce the same artifact name, even if overwriting each other. It is the "expected" output. "thin" would be an arbitrary change from our end. As long as the build task does not run the jar task it should be fine. If the user wants they can run the jar task and it would overwrite the shadow jar

Copy link
Author

@MarkusTieger MarkusTieger Jul 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think the JAR task and shadowJar task should produce the same artifact name

I don't think it has an advantage. It doesn't hurt that there is a clear difference between the two files. It could create confusion if it has the same filename (and maybe even more issues / bug reports etc., although I think that shouldn't be noticable)

Copy link
Member

@oSumAtrIX oSumAtrIX Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

shadowJar should IMO just be a configuration of jar e.g. jar.shadow = true.
Not producing the same artifact causes issues like having to configure the API regex, naming decisions and confusion in artifacts generated, their differences and the likes. If you generate a shadow you don't need the thin jar or vice versa. Overriding doesn't change anything else in other systems and spares said issues on its way.

}

// ShadowJar inherits the manifest from the JarTask, so changing it here will also change the fat jar
it.manifest.apply {
attributes["Name"] = patchesExtension.about.name
attributes["Description"] = patchesExtension.about.description
Expand All @@ -241,4 +262,21 @@ private fun Project.configureJarTask(patchesExtension: PatchesExtension) {
attributes["License"] = patchesExtension.about.license
}
}

val shade = configurations.create("shade")
configurations.getByName("compileClasspath").extendsFrom(shade)
configurations.getByName("runtimeClasspath").extendsFrom(shade)
Comment on lines +262 to +263
Copy link
Member

Choose a reason for hiding this comment

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

Please explain the purpose of these lines

Copy link
Author

@MarkusTieger MarkusTieger Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I created the "shade" configuration (I explained why below). By default, it does nothing.

Meaning, it isn't present in the compileClasspath (present at compile time, self explanatory I guess), and not present in runtimeClasspath (present at runtime, for a javaExec task for example).
By adding it to the compileClasspath and runtimeClasspath, "shade" acts the same way as "implementation"

Copy link
Member

Choose a reason for hiding this comment

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

Why would "shade" be needed in runtime or compiletime?

Copy link
Author

Choose a reason for hiding this comment

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

If it wouldn't be in the compileClasspath:

e: patches/all/misc/spoof/SignatureSpoofPatch.kt:6:20 Unresolved reference 'apksig'.
e: patches/all/misc/spoof/SignatureSpoofPatch.kt:7:20 Unresolved reference 'apksig'.
e: patches/all/misc/spoof/SignatureSpoofPatch.kt:76:24 Unresolved reference 'ApkVerifier'.
e: patches/all/misc/spoof/SignatureSpoofPatch.kt:81:71 Unresolved reference 'certificate'.
e: patches/all/misc/spoof/SignatureSpoofPatch.kt:83:71 Unresolved reference 'certificate'.
e: patches/all/misc/spoof/SignatureSpoofPatch.kt:85:71 Unresolved reference 'certificate'.
e: patches/all/misc/spoof/SignatureSpoofPatch.kt:91:17 Unresolved reference 'ApkFormatException'.

You can ask the same question with any other dependency used.
For example, why is "revanced-patcher" inside the compileClasspath (via "implementation).

Answer: So Java/Kotlin knows it exists and can compile it.

For the runtimeClasspath:
This is technically only relevant if "JavaExec" is used. Here are all dependencies that are expected to be present at runtime (= when the patches are executed).

Copy link
Member

Choose a reason for hiding this comment

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

I created the "shade" configuration (I explained why below)

I couldn't find the explanation below. Why did you create a separate configuration?
The plugin already adds a "shade()" API as seen here: https://gradleup.com/shadow/configuration/

Copy link
Author

Choose a reason for hiding this comment

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

"Dependencies added to the shadow configuration are not bundled into the output JAR." Thats blacklisting.

The "shade" configuration contains the ones that are getting bundled.

But I could also just use the "shadow" configuration to blacklist "revanced-patcher" and "smali" and everything else with "implementation" would get shaded in? If you prefer this way, let me know

Copy link
Member

Choose a reason for hiding this comment

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

I think using the default APIs is better. As the plugin configures the patches project, the plugin can use the shadow APIs to exclude the APIs such as patcher, library or co.

Copy link
Author

@MarkusTieger MarkusTieger Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

there is also "guava" inside the revanced-patches build.gradle.kts? Is there any reason for it being there and not hardcoded in the plugin? Would also need to be excluded

Copy link
Author

@MarkusTieger MarkusTieger Jul 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Kotlin plugin automatically adds dependencies, which shouldn't be shaded in, which makes excluding harder.

Copy link
Member

Choose a reason for hiding this comment

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

After coming back to this, i cant follow this review, can you explain why a new configuration is added again?

Comment on lines +261 to +263
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val shade = configurations.create("shade")
configurations.getByName("compileClasspath").extendsFrom(shade)
configurations.getByName("runtimeClasspath").extendsFrom(shade)
configurations.create("shade")
.also(configurations.getByName("compileClasspath")::extendsFrom)
.also(configurations.getByName("runtimeClasspath")::extendsFrom)
}

maybe better?

Copy link
Author

Choose a reason for hiding this comment

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

the shade variable is still used on line 266 it.configurations = listOf(shade)

Also I assume the "+}" on line 264 was a mistake?


tasks.withType(ShadowJar::class.java).configureEach {
it.configurations = listOf(shade)
it.archiveExtension.set("rvp")
it.archiveClassifier.set(null as String?)

it.minimize()
it.isEnableRelocation = true
it.relocationPrefix = "app.revanced.libs"
}
tasks.named("assemble") {
it.setDependsOn(listOf(tasks.named("shadowJar"), tasks.named("javadocJar"), tasks.named("sourcesJar")))
}
Copy link
Member

Choose a reason for hiding this comment

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

Why do we set configurations, archiveextnsion and archiveClassifier?

What does "minimize", and relocation do
"app.revanced.libs" seems arbitrary.

Why does the assemble task depend on shadowJar. I thought shadowJar depends on assemble. What about javadoc and sources? Those should not be ran when assemble is ran

Copy link
Author

@MarkusTieger MarkusTieger Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why does the assemble task depend on shadowJar. I thought shadowJar depends on assemble

So when you run ./gradlew build, it runs the build task. The build task depends on "assemble" and "test".

The assemble task itself doesn't do anything, it just depends on the required tasks iirc. By default with the java plugin on "jar", but I replaced that with shadowJar (although I could just added it instead of replaced, but there is no use for the unshaded patches jar/rvp )

What about javadoc and sources? Those should not be ran when assemble is ran

Its the default behaviour of gradle. The jar task depends on the javadoc and sourcesjar task (if java.withSourcesJar() and java.withJavadocJar() is called, which is the case). Because the jar task isn't run anymore (by default), we have to add the tasks manually to not lose the behaviour.

What does "minimize", and relocation do
"app.revanced.libs" seems arbitrary.

relocation is that the libraries don't conflict. The revanced-cli for example, uses apksig without obfuscation. If they use different versions if could get into a conflict. What relocation does it, it "relocates" the dependencies with a package prefix. For example "com.android.apksig" becomes "app.revanced.libs.com.android.apksig". Shadow automatically updates the imports in the shaded jar.
Minimize just means, it removes everything that isn't used. For example, I only use apksig to obtain signatures using verification. I do not sign apk files. Shadow should automatically remove any dead code.

Why do we set configurations, archiveextnsion and archiveClassifier?

By default, the output jar would have this filename "patches-5.30.0-all.jar".
I set the archiveextension to "rvp" so it is "patches-5.30.0-all.rvp".
Then I set the archiveClassifier to null, to remove the "-all", so its "patches-5.30.0.rvp".

The configurations are set, because by default the runtimeClasspath is used by shadow. In the runtimeClasspath is everything that should be present at runtime. Everything from "runtimeOnly" or "implementation" or "api" etc. is inside the runtimeClasspath. This includes things like "revanced-patcher" which is present in runtime, which shouldn't be shaded in. I explicitly add a new configuration called "shade", which is the exact same as "implementation", but with the difference that shadow shades every dependency from it into the shaded jar.

Copy link
Member

Choose a reason for hiding this comment

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

The assemble task itself doesn't do anything, it just depends on the required tasks iirc. By default with the java plugin on "jar", but I replaced that with shadowJar (although I could just added it instead of replaced, but there is no use for the unshaded patches jar/rvp )

I can't understand this. Please explain why the assemble task depends on shadowJar. The project should be built merely from running "gradlew build". Currently buildAndroid exists to build a .dex file but that's not ideal for the reason of it to work from running the build task.

Because the jar task isn't run anymore (by default), we have to add the tasks manually to not lose the behaviour.

shadowJar depends on the JAR task.

Shadow automatically updates the imports in the shaded jar.

This could break reflection. I don't think relocation should be enabled by default. Patches projects should individually decide on that. Without relocation and obfuscation, would it cause problems when CLI and patches have apkzlib in classpath?

Then I set the archiveClassifier to null, to remove the "-all", so its "patches-5.30.0.rvp".

You can set it to an empty string "" to avoid the cast.
If you set the extension to rvp and the classifier to "", would it override the thin JAR? I would like this behaviour. Also, does this work with the signing plugin for Gradle? WIll the shadowJar artifact be signed correctly?

Copy link
Author

Choose a reason for hiding this comment

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

This could break reflection.

No, shadow also replaces strings of classes.

Without relocation and obfuscation, would it cause problems when CLI and patches have apkzlib in classpath?

It could. Because of the patches, there is a seperate classloader. If the version does mismatch, for example:

  • revanced-patcher uses apksig 8.11.0

  • revanced-patches uses apksig 8.5.2 (via revanced-library)

  • revanced-patcher loads class ApkSigner and its dependent shared classes (because the revanced-patcher code lys in the AppClassLoader, it will try to load the 8.5.2 class of it)

  • revanced-patches loads class ApkVerifier (because revanced-patches has its own classloader, it will attempt to load the 8.11.0 version of it)

  • Now there is ApkSigner and shared classes (with version 8.5.2) in memory and ApkVerifier (with version 8.11.0) in memory. Although there are multiple versions of it, there can only be one classname loaded at a time (doesn't matter the classloader, jvm limitation).

  • If there is an internal api breakage, ApkVerifier could throw an exception, because it uses some classes also used by ApkSigner, which is an older version. For example, it could try to access a method that doesn't exist anymore.

You can set it to an empty string "" to avoid the cast.
If you set the extension to rvp and the classifier to "", would it override the thin JAR? I would like this behaviour. Also, does this work with the signing plugin for Gradle? WIll the shadowJar artifact be signed correctly?

I will look into this (although I don't like the behaviour of overwriting a jar). Like I already wrote once, it could confuse people (same filename, different files).

I can't understand this. Please explain why the assemble task depends on shadowJar. The project should be built merely from running "gradlew build". Currently buildAndroid exists to build a .dex file but that's not ideal for the reason of it to work from running the build task.

The shadowJar task isn't run by default. "./gradlew build" runs the jar task by default, not shadowJar. So I overwrote the dependency of "jar" in the assemble task with "shadowJar".
Also in my tests shadow did not depend on "jar", steps to reproduce:

build.gradle.kts

plugins {
  id("java")
  id("com.github.johnrengelman.shadow") version "8.1.1"
}

tasks.named("assemble") {
   setDependsOn(listOf(tasks.named("shadowJar")))
}

gradle build --dry-run or gradle assemble --dry-run
Notice: no "jar" task there

Copy link
Member

@oSumAtrIX oSumAtrIX Jul 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

No, shadow also replaces strings of classes.

That assumes the strings contain the full class. Something like "cla" + "ss" + "name" wont work.

It could. Because of the patches, there is a seperate classloader. If the version does mismatch, for example:

Different API versions can not really be solved however, wont a duplicate class be thrown at runtime?

The shadowJar task isn't run by default. "./gradlew build" runs the jar task by default, not shadowJar. So I overwrote the dependency of "jar" in the assemble task with "shadowJar".

You should make jar dependon shadowJar. This way we don't have to change anything anywhere and jar "turns" into "shadowJar". Perhaps it is possible to cancel jar task so it doesn't run unnecessarily but things like javadoc continue to work. Otherwise the shadowJar task should overwrite the jar just fine.

Copy link
Author

@MarkusTieger MarkusTieger Jul 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

That assumes the strings contain the full class. Something like "cla" + "ss" + "name" wont work.

Bad example, java is smart enough to optimize the "cla" + "ss" + "name" statement into "classname".
See

public class Test {

  public static void main(String[] args) {
    System.out.println("cla" + "ss" + "name");
  }

}

$ javac Test.java
$ javap -c Test.class (Notice the string "classname" being compiled in)

Same with:

public class Test {

  private static final String PREFIX = "class";

  public static void main(String[] args) {
    System.out.println(PREFIX + "name");
  }

}

But, yes, something like String.valueOf(new char[] {'c', 'l', 'a', 's', 's', 'n', 'a', 'm', 'e'}) obviously won't work, but why should any java library do something like this?

Different API versions can not really be solved however, wont a duplicate class be thrown at runtime?

Yes, the issue is though that different classes with different versions of the same library could be loaded at the same time. If you wish, I could write you a quick example in java (with a git repo).

You should make jar dependon shadowJar. This way we don't have to change anything anywhere and jar "turns" into "shadowJar". Perhaps it is possible to cancel jar task so it doesn't run unnecessarily but things like javadoc continue to work. Otherwise the shadowJar task should overwrite the jar just fine.

Yes, I can do that (with a few changes).

}