-
-
Notifications
You must be signed in to change notification settings - Fork 5
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.
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
base: dev
Are you sure you want to change the base?
Changes from all commits
ea5affe
bd48b21
fe15805
3b4db80
03a6326
3aeaf4e
b0c8a4a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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 | ||||||||||||||||
|
@@ -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) | ||||||||||||||||
|
@@ -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. | ||||||||||||||||
*/ | ||||||||||||||||
|
@@ -115,12 +125,13 @@ 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"]) | ||||||||||||||||
// Should also execute all the tests like with normal `gradlew build` | ||||||||||||||||
task.dependsOn(tasks["build"]) | ||||||||||||||||
|
||||||||||||||||
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() | ||||||||||||||||
|
@@ -229,6 +240,11 @@ abstract class PatchesPlugin : Plugin<Project> { | |||||||||||||||
private fun Project.configureJarTask(patchesExtension: PatchesExtension) { | ||||||||||||||||
tasks.withType(Jar::class.java).configureEach { | ||||||||||||||||
it.archiveExtension.set("rvp") | ||||||||||||||||
|
||||||||||||||||
if (it.archiveClassifier.orNull.isNullOrEmpty()) { | ||||||||||||||||
it.archiveClassifier.set("thin"); | ||||||||||||||||
} | ||||||||||||||||
|
||||||||||||||||
it.manifest.apply { | ||||||||||||||||
attributes["Name"] = patchesExtension.about.name | ||||||||||||||||
attributes["Description"] = patchesExtension.about.description | ||||||||||||||||
|
@@ -241,4 +257,23 @@ 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please explain the purpose of these lines There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would "shade" be needed in runtime or compiletime? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it wouldn't be in the compileClasspath:
You can ask the same question with any other dependency used. Answer: So Java/Kotlin knows it exists and can compile it. For the runtimeClasspath: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I couldn't find the explanation below. Why did you create a separate configuration? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
maybe better? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the shade variable is still used on line 266 Also I assume the "+}" on line 264 was a mistake? |
||||||||||||||||
|
||||||||||||||||
tasks.withType(ShadowJar::class.java).configureEach { | ||||||||||||||||
it.configurations = listOf(shade) | ||||||||||||||||
it.archiveClassifier.set("") | ||||||||||||||||
|
||||||||||||||||
it.minimize() | ||||||||||||||||
it.isEnableRelocation = true | ||||||||||||||||
it.relocationPrefix = "app.revanced.patches" | ||||||||||||||||
} | ||||||||||||||||
tasks.named("assemble") { | ||||||||||||||||
it.dependsOn(tasks.named("shadowJar")) | ||||||||||||||||
} | ||||||||||||||||
tasks.named("jar") { | ||||||||||||||||
it.enabled = false | ||||||||||||||||
} | ||||||||||||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think overwriting the jar is fine, (i dont remember what we discussed regarding this)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See commit message here 3aeaf4e
The task (the one without dependencies shaded in) is already disabled and never runs, though it causes an error if it has the same classifier as the other task on running a maven publish.