Skip to content

Bump to build-tools 35.0.2 #16

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Bump to build-tools 35.0.2 #16

wants to merge 3 commits into from

Conversation

Taknok
Copy link

@Taknok Taknok commented Apr 19, 2025

Also include a CI with binary attestation and auto release.

@Taknok Taknok changed the title Bump to build-tools 35 Bump to build-tools 35.0.2 Apr 19, 2025
Copy link
Member

@oSumAtrIX oSumAtrIX left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. However I have a couple of strong gripes that need to be addressed.

  • Why are we bumping to 35.0.2 specifically? Is there no way to target the latest Android has?
  • The README uses "I". You should communicate to us via GitHub, not the checked in readme.
  • The README, workflows and code style and license does not follow the previous one. Changing them is not part of the PRs aim.
  • The PR now makes this repo base on some other repo with different goals. The repos purpose is to build AAPT2 not other build tools.
  • Part of what belongs to the build system is put in CI, or part what belongs to GIT as the source manager is put in the build system. Separation of concern is important
  • The current readme should be kept as is or synced with our other repos readmes and their style and speech of figure.

@Taknok
Copy link
Author

Taknok commented Apr 21, 2025

Hi,
Thank for the very detailed review.
As a general approach, this repo is based on the work of lzhiyong at https://github.com/lzhiyong/android-sdk-tools. I did the less possible modification from the original repo in order to lesser the merge conflict that may comes in the futur. Here is the diff between my modification from lzhiyong https://github.com/Taknok/aapt2/compare/lzhiyong-master..main

Here are the diff made by ibotpeaches for the aapt2 used for apktools : aosp-mirror/platform_frameworks_base@main...iBotPeaches:platform_frameworks_base:apktool_14.0.0

Why are we bumping to 35.0.2 specifically? Is there no way to target the latest Android has?

Because build-tools were stop in 2023 to v34 https://developer.android.com/tools/releases/build-tools
Aapt2 modifications are tracking in platform-tools now, latest tag for platform-tools is the v35.0.2 https://android.googlesource.com/platform/manifest/+refs

The README uses "I". You should communicate to us via GitHub, not the checked in readme.

I agree, I only add the 1st line in the Readme as I did not wanted to modify it to avoid rebase/merge from future modifications in the lzhiyong repo. But if it is wanted to modify it, it is a totally understandable choice. Tell me if you want me to recreate/rephrase it.

The README, workflows and code style and license does not follow the previous one. Changing them is not part of the PRs aim.

I was in a quite tricky situation, as lzhiyong put his code under the apache v2 and you code is under the GPLv3. Here the GPLv3 can be applied to the repo and all created files/code, but the code from lzhiyong should remain under apache v2

The PR now makes this repo base on some other repo with different goals. The repos purpose is to build AAPT2 not other build tools.

I do not totally agree. I used the work of lzhiyong that aim to provide build-tools and platform-tools binaries for android but I striped all the repo to keep only what is used for aapt and aapt2. I add the modification of IbotPeaches in order to keep the best compatibility of aapt2 with the apktools libs.

Part of what belongs to the build system is put in CI, or part what belongs to GIT as the source manager is put in the build system. Separation of concern is important

I did not design the build system, and (as said previously I did not want to modify too much the repo to lesser merge/rebase conflict). We can choose to go back to submodule with a cleaner build.py or build.sh as before. I let you decide.

The current readme should be kept as is or synced with our other repos readmes and their style and speech of figure.

Cf response to point 2

As a general approach, it seems that you prefer to increase the diff with the lzhiyong repo in order to clean and standardize across revanced repo even if future rebase/merge will be a bit harder ?

@Taknok
Copy link
Author

Taknok commented Apr 25, 2025

What i propose:

  1. Source repo are submodules pinned to 35.0.2
  2. Patch is in a separate bash file as patch should be done once
  3. Build is in another bash and it targets one arch passed as argument because build could be done many times
  4. CI as closed as possible of revanced (ndk installation, protoc build etc differs)

Are those points ok ?

@oSumAtrIX
Copy link
Member

I do not totally agree. I used the work of lzhiyong that aim to provide build-tools and platform-tools binaries for android but I striped all the repo to keep only what is used for aapt and aapt2. I add the modification of IbotPeaches in order to keep the best compatibility of aapt2 with the apktools libs.

I suppose your PR has the same goal (build aapt2), but not izhiyongs repo. However, as you have modified their repo substantially, it may be easier to base of our repo instead of theirs, as our repo already has the same goal as your PR, while theirs follows another.

As a general approach, it seems that you prefer to increase the diff with the lzhiyong repo in order to clean and standardize across revanced repo even if future rebase/merge will be a bit harder ?

You understand the general change I would like to see in this PR. Basically, this repo was made from "scratch" with every line intended as is for the purpose of building AAPT2. I am not familiar with many lines from lzhiyong and reviewing that seems quite difficult, because I come across lines I either question or need changes a lot. I am wondering, how easy it is to work up to v35 from our repo. What prevents this? From my understanding, we need to bump the submodules to whatever v35 uses. Some compilation errors will surely occur that would be needed to look into, but then again lzhiyong's repo would come in clutch for reference in this regard. Once we can compile upstream AAPT2 v35 with our build system, we can look into the patches that make it work on Android and for ReVanced's purposes, which should be ibotpeaches patches (and ours such as the one that fix AAPT2 for arm32).

What i propose:

  1. Source repo are submodules pinned to 35.0.2
  2. Patch is in a separate bash file as patch should be done once
  3. Build is in another bash and it targets one arch passed as argument because build could be done many times
  4. CI as closed as possible of revanced (ndk installation, protoc build etc differs)

Are those points ok ?

Yep! Sounds perfect. Regarding 4, please let me know what changes you have in mind. I would like to understand why changes are needed in the first place.

@Taknok Taknok requested a review from oSumAtrIX May 1, 2025 21:01
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.

2 participants