Skip to content

Conversation

validcube
Copy link
Member

Use build tool 35.0.1 or above to stop build tool from generating AIDL file with unicode escape when the first character of the directory contain a "u"

See: https://issuetracker.google.com/issues/354735915

fixes #2672

@validcube validcube requested a review from Axelen123 July 22, 2025 13:45
@validcube validcube changed the base branch from main to compose-dev July 22, 2025 13:45
@validcube
Copy link
Member Author

This PR should be merged immediately when approved.

@validcube validcube requested a review from brosssh July 22, 2025 13:48
@oSumAtrIX
Copy link
Member

What is the default one

@validcube
Copy link
Member Author

validcube commented Jul 22, 2025

What is the default one

The default was build tool 35.0.0

(The default value is grabbed from the AGP 8.9.1, however as of right now the value is still the same with the latest version)

@oSumAtrIX
Copy link
Member

I don't think we should hardcode. AGP should update to it

Copy link
Member

@Axelen123 Axelen123 left a comment

Choose a reason for hiding this comment

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

I just realized something. Is there a reason why this is not done for app as well?

@validcube
Copy link
Member Author

I just realized something. Is there a reason why this is not done for app as well?

That's because :app already have specified build tools

@validcube
Copy link
Member Author

I don't think we should hardcode. AGP should update to it

AGP default (likely well tested in this case) hold the same weight as AGP minimum, so it should be fine to bump it.

https://developer.android.com/build/releases/gradle-plugin#compatibility

@Axelen123
Copy link
Member

I just realized something. Is there a reason why this is not done for app as well?

That's because :app already have specified build tools

Didn't realize that. I probably would have checked if I had access to a computer.

Copy link
Member

@Axelen123 Axelen123 left a comment

Choose a reason for hiding this comment

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

Approved unless the issue can be solved by bumping AGP.

@brosssh
Copy link
Member

brosssh commented Jul 23, 2025

Approved unless the issue can be solved by bumping AGP.

Latest AGP still has 35.0.0 as default:
image

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.

This PR should not be merged. Instead you should wait for AGP to update to the fixed build tools version, because this PR adds unnecessary maintenance burden (updating build tools manually) and doesn't even use the version catalog. This applies to the :app module

@validcube
Copy link
Member Author

validcube commented Jul 23, 2025

This PR should not be merged. Instead you should wait for AGP to update to the fixed build tools version, because this PR adds unnecessary maintenance burden (updating build tools manually) and doesn't even use the version catalog. This applies to the :app module

I've checked this, the AGP source does not set or dictate the specific dependency version needs to be used. If developers didn't specify which version to use, then AGP will get its default value, which by no means is the recommended version because there is none.

However if they do the AGP will let us know by crashing the right away or put it in documentation: see: https://developer.android.com/build/releases/gradle-plugin#updating-gradle

@oSumAtrIX
Copy link
Member

Why does agp not default to the latest version

@validcube
Copy link
Member Author

validcube commented Jul 24, 2025

Why does agp not default to the latest version

Library or dependency don't need to automatically update its default dependency or grab whatever latest from the server. That just wouldn't make sense in general.

@oSumAtrIX
Copy link
Member

But Everytime agp is updated I'd suppose they update build tools too?

@brosssh
Copy link
Member

brosssh commented Jul 24, 2025

They don't, 35.0.0 has been the default value since AGP 8.8.0. The latest AGP is 8.11, still version 35.0.0 is the default.

@oSumAtrIX
Copy link
Member

That is odd. Why wouldn't they bump?

@validcube
Copy link
Member Author

validcube commented Jul 25, 2025

That is odd. Why wouldn't they bump?

That's the minimum version (which happens to also be the default/fallback version), AGP support build tools 35.0.0 and above, the minimum is updated on an ad-hoc basis.

Version on build tools doesn't really matter much, because building a newer version of Android like Android 16 (36) works fine on older build tools like 35.0.0

@Axelen123
Copy link
Member

The app module already sets the build tools version and updating them isn't very important, so I don't think this PR adds a significant extra maintenance burden.

@oSumAtrIX
Copy link
Member

What I am saying applies to the app module as well. However as insignificant as it may look, it's an additional tech debt that you need to avoid if possible, and not just when it "looks significant". Setting the build tools version would require everyone to have that specific version too. Is that the case when unset?

@validcube
Copy link
Member Author

What I am saying applies to the app module as well. However as insignificant as it may look, it's an additional tech debt that you need to avoid if possible, and not just when it "looks significant". Setting the build tools version would require everyone to have that specific version too. Is that the case when unset?

I don't see why would bumping build tools would be considered as technical debt to the project? This PR bump the default of 35.0.0 to 35.0.1, this isn't done randomly but to fix an issue, so we actually reduce a tech debt by not having to wait for AGP to bump the minimum/default version to 35.0.1 or make workaround for user whose the first character of the directory is a "u".

But maybe when AGP finally does update the default/minimum version to 36.0.01, we can remove it from the build config.

Footnotes

  1. (36.0.0 is not a mistake, they always update the major component, not patch unless critical)

@oSumAtrIX
Copy link
Member

I don't see why would bumping build tools would be considered as technical debt to the project?

Because now you have to bump it from time to time manually.

so we actually reduce a tech debt by not having to wait for AGP to bump the minimum/default version to 35.0.1

Not doing something and waiting for something to happen is not a debt.

or make workaround for user whose the first character of the directory is a "u".

I personally would apply the ostrich algorithm and let time heal the wounds instead of casting an indefinite technical debt upon us. Is there a bump cycle? If it is not too long, then that's definitely the better course of action.

@validcube
Copy link
Member Author

validcube commented Aug 9, 2025

I personally would apply the ostrich algorithm and let time heal the wounds instead of casting an indefinite technical debt upon us.

Using ostrich to ignore critical problem should not be the reason why this PR can't be merged when the fix is just one line and does not violate any android development rule.

Is there a bump cycle? If it is not too long, then that's definitely the better course of action.

There is no specific bump cycle for default/minimum version of build tools. If we were to have educated guesses from last time the minimum was bump, it would take around 14 months for AGP to bump it (AGP 8.2.0 Nov 2023 -> 8.8.0 Jan 2025) (additionally I have checked the latest alphas and they're still on 35.0.0)

Not doing something and waiting for something to happen is not a debt.

It is a debt because we didn't fix the issue that arises with using the default configuration.

Because now you have to bump it from time to time manually.

I don't see why this would be a problem as this is equivalent to updating a dependency.

@oSumAtrIX
Copy link
Member

Using ostrich to ignore critical problem should not be the reason why this PR can't be merged when the fix is just one line and does not violate any android development rule.

In my opinion its a very niche problem and not critical that doesn't justify increasing tech debt.

It is a debt because we didn't fix the issue that arises with using the default configuration.

Its not our problem to fix. AGP updates automatically which would fix the issue without any intervention. If you hardcode now, you cast tech debt upon yourself for a one time niche problem (which isn't even our problem)

I don't see why this would be a problem as this is equivalent to updating a dependency.

Because its yet another hardcoded value in a random location you'll forget in two weeks.

@validcube validcube added the ReVanced Manager Compose Regarding the Compose rewrite of ReVanced Manager label Aug 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReVanced Manager Compose Regarding the Compose rewrite of ReVanced Manager
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(Compose): Illegal unicode escape during the build
4 participants