#32: Remove apktool dependency

victorrattis commented 5 years ago

Remove apktool dependency and use baksmali to decode an APK file.

Remove apktoolkit dependency;
Use baksmali lib to decode the APK;
Add and config Ivy Apache to download the dependencies;
Implement smali decoder to decode an APK in smali code;

alexzaitsev commented 5 years ago

Wow! Amazing work. Give me some time to play around with your changes.

victorrattis commented 5 years ago

Thanks, take your time...

I analyzed better my submitted code and there is an improvement to do:

In build.xml: I included all the dependencies hardcoded:

<include name="${lib.dir}/baksmali-2.2.5.jar" />
<include name="${lib.dir}/dexlib2-2.2.5.jar" />
<include name="${lib.dir}/guava-18.0.jar" />
<include name="${lib.dir}/util-2.2.5.jar" />
<include name="${lib.dir}/jcommander-1.64.jar" />

And the above code can be improvement for the following one: <include name="${lib.dir}/**/*.jar" />

I am going submit another commit with this fix.

victorrattis commented 5 years ago

Done, I sent a new commit with all the changes suggested by you. For this new commit, I squashed the changes in just one commit... I think it's better, but what do you think about in joining two commits into one commit only? or is it better to keep two or more commits for each change request in code review?

alexzaitsev commented 5 years ago

@victorrattis I think that squashing in most cases is the preferable way for PRs. Thanks for the quick changes! I'm going to approve and merge it to the master. How do you feel about this PR - have you tested it well? Should something in readme be changed as well?

victorrattis commented 5 years ago

Hi @alexzaitsev, I analyzed and tested better this patch... I stopped for a while to analyse each code checking if everything is okay.

My dev analysis:

I found two points that caught my attention:

1. Android API version: To decode the APK using baksmali jar is necessary to pass as parameter the API version. Using the value 28 as default because in my analysis it is the latest version. Theoretically and in my tests it decodes well the dex file of the last and previous versions without any problem. The baksmali uses this info to make the mapping of API that will be used to decode each dex code (for more information look this at link).

private static final int DEFAULT_ANDROID_VERSION = 28;

About this point, I have tested APK between the 15 and 28 versions and it works well but I am going to add a // TODO comment there explaining the next step to change the default version to the current version of the APK so it can be more dynamic.

2. Get the dex files inside the APK: I implemented a solution to filter and return all files that contains "classes" in the file name but I think it is better to change it to filter all the files with the extension ".dex" so it checks if contains "classes".

return ZipFileUtils.filterByContainsName(apkFilePath, "classes");

The current patch is working well but I am going to submit a new commit with these improvements. I have tested in a few Android versions and projects. So then I can integrate it =)

victorrattis commented 5 years ago

Should something in readme be changed as well?

I think that so because in current README says about apktool, but do you think better do that in another issue? or in this pull request?

victorrattis commented 5 years ago

Ready, I submitted the last version for this Pull Request, I believe it is last. I tested it using APKs with or without multi-dex, few Android SDK versions and Android App Bundle and it is working well in my tests. I didn't test using an instant app. Now you can integrate it =)

alexzaitsev commented 5 years ago

Ready, I submitted the last version for this Pull Request, I believe it is last. I tested it using APKs with or without multi-dex, few Android SDK versions and Android App Bundle and it is working well in my tests. I didn't test using an instant app. Now you can integrate it =)

Thank you for your efforts! You've done a huge piece of work here.

alexzaitsev commented 5 years ago

Should something in readme be changed as well?

I think that so because in current README says about apktool, but do you think better do that in another issue? or in this pull request?

I think that it's ok to have readme changes here as well because they reflect code changes. Thanks!

victorrattis commented 5 years ago

Done, I submitted as changes in the README.md file, I think it is okay... I removed the following sections:

"Long way" - This section says to run apktool and so run the apk-dependency-graph jar and I think it is not more necessary;
"Troubleshooting" - I tested an instant app APK and in my tests, it is working well. Due to that, I removed this section.

alexzaitsev commented 5 years ago

Hooray! Thank you @victorrattis ! Amazing work!

alexzaitsev / apk-dependency-graph

#32: Remove apktool dependency #33

My dev analysis: