refactor: nested dex + replace --only-main-classes with -a/--all-src#4069
refactor: nested dex + replace --only-main-classes with -a/--all-src#4069IgorEisberg merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the handling of nested/dynamic dex files and replaces the --only-main-classes CLI option with -a/--all-src. The default decoding mode changes from decoding all sources to only decoding standard classes*.dex files, which is more typical for normal usage. Dynamic dex files (not matching the classes*.dex pattern) are now handled differently: with -a they're decoded, without it they're copied to the "unknown" folder. The PR also fixes nested dex file decoding by using @ as a path separator placeholder in smali folder names.
Changes:
- Default decode mode changed from FULL to ONLY_MAIN_CLASSES; CLI option inverted from
--only-main-classesto-a/--all-src - Nested dex files supported by replacing path separators with
@in folder names during decode, reversed during build - SmaliDecoder and SmaliBuilder refactored to be reusable across multiple dex files
Reviewed changes
Copilot reviewed 23 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| brut.j.dir/.../Directory.java | Added separatorChar constant alongside separator String for performance |
| brut.j.dir/.../AbstractDirectory.java | Uses separatorChar for indexOf operations |
| brut.j.dir/.../ZipRODirectory.java | Uses separatorChar for indexOf operations |
| brut.apktool/.../Config.java | Changed default mDecodeSources from FULL to ONLY_MAIN_CLASSES |
| brut.apktool/.../ApkInfo.java | Added CLASSES_FILES_PATTERN, renamed constants for clarity |
| brut.apktool/.../SmaliDecoder.java | Made reusable with thread-safe mDexFiles tracking |
| brut.apktool/.../SmaliBuilder.java | Made reusable by accepting smaliDir parameter instead of storing it |
| brut.apktool/.../ResDecoder.java | Renamed from ResourcesDecoder for brevity |
| brut.apktool/.../ResFileDecoder.java | Renamed resFileMapping to resFileMap |
| brut.apktool/.../AaptInvoker.java | Removed conditional check for resources.zip existence |
| brut.apktool/.../ApkDecoder.java | Refactored dex handling with @ separator for nested paths |
| brut.apktool/.../ApkBuilder.java | Reverse @ separator back to path separator during build |
| brut.apktool/.../Main.java | Replaced --only-main-classes with -a/--all-src CLI option |
| brut.apktool/.../TestUtils.java | Optimized replace operations |
| brut.apktool/.../test/*.java | Renamed test resources, removed setForced(true), use separate output dirs |
| test resources/*.apk | Renamed test APK files with underscores for consistency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
brut.apktool/apktool-lib/src/main/java/brut/androlib/res/AaptInvoker.java
Show resolved
Hide resolved
iBotPeaches
left a comment
There was a problem hiding this comment.
Can you hit PR for the doc changes?
Done! |
* Initial plan * fix: tighten span tag parsing (iBotPeaches#4061) * fix: avoid assumptions about res dir (iBotPeaches#4067) * refactor: nested dex + replace --only-main-classes with -a/--all-src (iBotPeaches#4069) * refactor: source and tests (iBotPeaches#4071) * chore: promote -a/--all-src to a common option (iBotPeaches#4072) * build(deps): bump com.android.tools:r8 from 8.13.17 to 8.13.19 (iBotPeaches#4064) Bumps com.android.tools:r8 from 8.13.17 to 8.13.19. --- updated-dependencies: - dependency-name: com.android.tools:r8 dependency-version: 8.13.19 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * minor: TextUtils hotfix (iBotPeaches#4079) * refactor: new BinaryXmlResourceParser, ResXmlSerializer and more (iBotPeaches#4077) * build: migrate to modern maven publish (iBotPeaches#4073) * build: migrate to modern maven publish * build: move to v0.36 * build: use v33 for Java 11 min * build: skip maven on unsupported Java versions * chore: configure codeql manually * build: defer maven publishing till jdk11 * chore: move publising to own file * build(deps): bump gradle/actions from 5.0.0 to 5.0.1 (iBotPeaches#4080) Bumps [gradle/actions](https://github.com/gradle/actions) from 5.0.0 to 5.0.1. - [Release notes](https://github.com/gradle/actions/releases) - [Commits](gradle/actions@v5...v5.0.1) --- updated-dependencies: - dependency-name: gradle/actions dependency-version: 5.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * fix: ensure proper locale in ResPrimitive (iBotPeaches#4084) Signed-off-by: Salvo Giangreco <giangrecosalvo9@gmail.com> * feat: support for API 36.1 (Baklava) (iBotPeaches#4082) * feat: staged aliases support (iBotPeaches#4085) * fix: framework parse performed twice (iBotPeaches#4086) * fix: framework parse performed twice * fix up dynamic references with calling pkgId * Fix merge conflict markers in SmaliDecoder.java * Fix compilation errors in SmaliDecoder and AaptInvoker --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: Salvo Giangreco <giangrecosalvo9@gmail.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Igor Eisberg <8811086+IgorEisberg@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Connor Tumbleson <iBotPeaches@users.noreply.github.com> Co-authored-by: Salvo Giangreco <giangrecosalvo9@gmail.com>
Fixes: #4068
mDecodeSourcesfromFULLtoONLY_MAIN_CLASSES, which is more common in normal usage.--only-main-classeswith-a/--all-srcas it's less common in normal usage, and complements-s/--no-src.CLASSES_FILES_PATTERNto easily separate standard dex files from unknown dex files (usually loaded dynamically by the app).-a/--all-src, unknown dex files are (not matchingCLASSES_FILES_PATTERN) decoded and not copied to the working folder.-a/--all-src, unknown dex files are copied to theunknownfolder to be repacked as-is.@as a path separator placeholder insmali_folder name.DuplicateDexTesttoDynamicDexTestas "duplicate" dex are now impossible (unless these dex file have@in their name, for some reason). Now it tests for success rather than validating an old bug that caused failure.Misc optimizations:
ResourcesDecodertoResDecoderfor simplicity and to match variable names.SmaliDecoderonce and make it reusable, likeResDecoder.SmaliDecoderrecord handledmDexFilesand the minimalmInferredApiLevelto be later used byApkDecoder.AndroidManifest.xml, notresources.arsc.SmaliBuilderonce and make it reusable.AaptInvokeronce for consistency withResDecoder.Directoryby usingseparator(String) andseparatorChar(char), likeFileclass, to avoid concatenation of different data types by an implicitStringBuilder.RAW_DIRS,*_FILES_PATTERN,resFileMapandresZip.sConfig.setForced(true)in tests, use separate folders instead for easier analysis in case of failure.