Skip to content
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

build: exclude aapt1 binary, minimize revanced-library #340

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

Conversation

kitadai31
Copy link

First, exclude aapt1 binaries.
aapt1 is never used in ReVanced CLI, but accounts for 10% of the jar size.

Second, specify app.revanced:revanced-patcher explicitly in the minimize block, because revanced-library does not need to be excluded from minimization.
Since ReVanced Library was born, exclude(dependency("app.revanced:.*")) has been excluding revanced-library from minimization unintentionally.
(This line exists before ReVanced Library was created.)

I tested patching, list patches, list versions, mount/unmount, and install/uninstall. It worked as expected.

Finally, remove exclude(dependency("org.jetbrains.kotlin:.*")) as it has no effect. Removing or adding this line made no difference.

This PR reduces the jar size as follows:
Removing aapt1: 49.9 MB -> 44.8 MB
Optimize minimization: 44.8 MB -> 40.3 MB

@oSumAtrIX
Copy link
Member

Shouldn't this be done in ReVanced Patcher?

@kitadai31
Copy link
Author

kitadai31 commented Sep 2, 2024

I don't think so.

In my understanding, the ReVanced Patcher jar doesn't includes these aapt binaries that comes from apktool-lib dependency, so patcher has no control over whether aapt resources gets included in the final fat jar of CLI.

In addition, ReVanced Manager also excludes aapt in its build.gradle.
https://github.com/ReVanced/revanced-manager/blob/d201bdc422fcceb09040398c87edbaf2e97c7296/app/build.gradle.kts#L63

@oSumAtrIX
Copy link
Member

But now every frontend would have to remove this library. This is duplication that should've been handled by the library itself, because the library itself does not use aapt1. Please check if it is possible to move this to ReVanced Patcher, I would rather not want to do this here.

@kitadai31
Copy link
Author

I have researched for a while and it seems impossible to move this change to ReVanced Patcher.

Another option is to remove aapt1 from ReVanced/Apktool fork.
If this fork is only used by the ReVanced Patcher, this is an option.

@oSumAtrIX
Copy link
Member

Why is it impossible?

@kitadai31
Copy link
Author

Gradle specification.

I first analyzed the revenged-patcher artifacts.
The artifact of ReVanced Patcher have these three files:

  • revanced-patcher-20.0.0.jar
  • revanced-patcher-20.0.0.pom
  • revanced-patcher-20.0.0.module

Among these, the jar contains only the classes of revanced-patches.
Dependency information is contained in the pom and module.

In the pom file, dependencies are written like this:

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  ...
  <dependencies>
    <dependency>
      <groupId>org.jetbrains.kotlin</groupId>
      <artifactId>kotlin-stdlib</artifactId>
      <version>2.0.0</version>
      <scope>compile</scope>
    </dependency>
    <dependency>
      <groupId>app.revanced</groupId>
      <artifactId>apktool-lib</artifactId>
      <version>2.9.3</version>
      <scope>runtime</scope>
    </dependency>
    ...
  </dependencies>
</project>

If a <dependency> element can be configured to exclude certain files from the jar file, then aapt1 can be excluded on the revanced-patcher side.
However, according to the POM descriptors reference, there is no such feature.
https://maven.apache.org/ref/3.9.9/maven-model/maven.html#class_dependency

The module file is for Gradle but same as pom.

Therefore, I concluded that this is impossible.

Also, this ChatGPT's answer is helpful to understand.
https://chatgpt.com/share/5a610a60-6992-4568-bde3-b32dc0b8f727

build.gradle.kts Outdated
Comment on lines 62 to 66
exclude("/prebuilt/linux/aapt")
exclude("/prebuilt/linux/aapt_64")
exclude("/prebuilt/macosx/aapt_64")
exclude("/prebuilt/windows/aapt.exe")
exclude("/prebuilt/windows/aapt_64.exe")
Copy link
Member

Choose a reason for hiding this comment

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

Can't a wildcard be used to reduce the amount of exclude statements?

Copy link
Author

Choose a reason for hiding this comment

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

I tried my best but it's not possible.
If I use a wildcard, it will always match aapt2.
Example: exclude("/prebuilt/**/aapt*") matches /prebuilt/linux/aapt2, /prebuilt/linux/aapt2_64

Apache Ant style wildcard is supported here, but it has only three wildcards, ?, *, and **
If I put aapt*, it also matches aapt2* always.
So there is no way to avoid including aapt2 binaries other than to list the five aapt1.

Copy link
Member

@Ushie Ushie Sep 11, 2024

Choose a reason for hiding this comment

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

What about something like this?

exclude("/prebuilt/*/aapt") exclude("/prebuilt/*/aapt_*") exclude("/prebuilt/*/aapt.*") though at that point, listing all might aswell be better

Copy link
Author

Choose a reason for hiding this comment

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

Sorry I was dumb
Your wildcards work, thank you

I made a few adjustments.
Remove wildcards from Linux aapt and Windows aapt.exe, and put 3 arguments into single exclude() call.
I think this is compact and visually easy to see what is being excluded.

Suggested change
exclude("/prebuilt/linux/aapt")
exclude("/prebuilt/linux/aapt_64")
exclude("/prebuilt/macosx/aapt_64")
exclude("/prebuilt/windows/aapt.exe")
exclude("/prebuilt/windows/aapt_64.exe")
exclude("/prebuilt/linux/aapt", "/prebuilt/windows/aapt.exe", "/prebuilt/*/aapt_*")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants