-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
refactor: source and tests #4071
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
Conversation
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.
Pull request overview
This PR performs a comprehensive refactoring of the Apktool codebase focused on improving maintainability and code organization. The changes include merging AbstractDirectory into Directory, refactoring ApkBuilder and ApkDecoder with improved resource handling logic, migrating test utilities from TestUtils into BaseTest, and simplifying exception handling across all tests.
Changes:
- Refactored directory hierarchy by merging
AbstractDirectoryintoDirectorybase class - Improved
ApkBuilderwith clearer order of operations for resource building and proper file lock handling - Changed
ApkDecoder.decode()return type fromApkInfoto void with separate getter method - Migrated
getIncludeFiles()fromApkBuildertoAaptInvokerand removed unused aapt2 parameters - Merged
TestUtilsintoBaseTestand simplified test exception declarations tothrows Exception - Renamed
XmlUtils.loadDocumentContent()toparseDocument()with performance improvement usingStringReader
Reviewed changes
Copilot reviewed 68 out of 68 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| XmlUtils.java | Renamed method and optimized XML parsing with StringReader |
| Directory.java, AbstractDirectory.java | Merged abstract class into base class to simplify hierarchy |
| FileDirectory.java, ZipRODirectory.java | Refactored to extend Directory directly with cleaned implementations |
| ExtFile.java | Changed exception type from IOException to DirectoryException |
| ApkBuilder.java | Major refactoring with improved resource building logic and file handling |
| ApkDecoder.java | Changed API to void return with separate getter for ApkInfo |
| AaptInvoker.java | Added getIncludeFiles() method and removed unused parameters |
| BaseTest.java | Merged TestUtils functionality and simplified helper methods |
| Test files | Simplified exception declarations and removed unused imports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
brut.apktool/apktool-lib/src/test/java/brut/androlib/BaseTest.java
Outdated
Show resolved
Hide resolved
brut.apktool/apktool-lib/src/test/java/brut/androlib/BaseTest.java
Outdated
Show resolved
Hide resolved
|
I wish I properly organized all my samples that broke arsc parsing since trying to follow if these string pool changes are semantically the same. |
I pretty much just merged |
ApkBuilder: RefactoredbuildResourceswith well-documented order of operation.ApkBuilder: Determine whetherAndroidManifest.xmlis raw usingResChunkHeaderinbuildResources.ApkBuilder: MergedbackupManifestFileintobuildManifestOnlyandbuildResourcesFullywhere editing happens.ApkBuilder: Made surebuildcloses the APK file even if an exception occurred to release file lock.ApkBuilder,ApkDecoder: Normalized comments and logging.AaptInvoker: Removed unused parameters (aapt2:-Aand-Roptions are neither used nor needed).AaptInvoker: MigratedgetIncludeFilesfromApkBuilder.BinaryXmlResourceParser: Initial usage ofResChunkPullParser(the rest is WIP for later).AbstractDirectoryintoDirectory.FileDirectoryandZipRODirectory.FileDirectory/ZipRODirectorywhenever possible,ExtFileis ambiguous and hides possible file lock.XmlUtils: RenamedloadDocumentContenttoparseDocumentand usedStringReaderfor performance.TestUtilsintoBaseTest.The reasoning behind
ExtFileis being reevaluated but kept unchanged for now.Maintenance PR: No changes in usage or output.