-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Pass minimum api level to smali library #1313
Pass minimum api level to smali library #1313
Conversation
Thanks for the PR. I have a lot of local work in progress for Android Nougat stuff, but the instant I get that settled I'll get this merged. Thanks again |
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.
Code looks good, just some minor code style things before I can merge.
@@ -278,6 +279,9 @@ public void build(ExtFile appDir, File outFile) | |||
mAndRes.setVersionInfo(meta.versionInfo); | |||
mAndRes.setSharedLibrary(meta.sharedLibrary); | |||
|
|||
if (meta.sdkInfo != null && meta.sdkInfo.get("minSdkVersion") != null) |
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.
Can you use braces for added logic? I try to stay away from single statement control flow as its just prone to errors.
} | ||
|
||
private void build() throws AndrolibException { | ||
try { | ||
DexBuilder dexBuilder = DexBuilder.makeDexBuilder(); | ||
DexBuilder dexBuilder; | ||
if (mApiLevel > 0) |
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.
Same as previous comment, use braces here instead of single statements.
When apktool creates a DexBuilder class, it creates it without specifying a minimum api level. This causes the DexBuilder class to assume api level 20 by default. This is not ideal since in some cases, the concrete minimum api level is required. One such case is in smali's DexWriter class which implements a workaround for a bug in Dalvik that was fixed in Android 4.2 (https://code.google.com/p/android/issues/detail?id=35304) that causes apps that call the Method.getParameterAnnotations api to crash in some cases (see bug report for more details). The workaround that smali implements is only triggered if the minimum api level is below 17 (Android 4.2). But since apktool effectively sets the minimum api level to 20, this code is never triggered causing apktool to create apk files that crash on devices below Android 4.2. This change passes the minimum api level to the smali library.
b0cf6b7
to
db35f54
Compare
When apktool creates a DexBuilder class, it creates it without specifying a minimum api level. This causes the DexBuilder class to assume api level 20 by default. This is not ideal since in some cases, the concrete minimum api level is required.
One such case is in smali's DexWriter class which implements a workaround for a bug in Dalvik that was fixed in Android 4.2 (https://code.google.com/p/android/issues/detail?id=35304) that causes apps that call the Method.getParameterAnnotations api to crash in some cases (see bug report for more details). The workaround that smali implements is only triggered if the minimum api level is below 17 (Android 4.2). But since apktool effectively sets the minimum api level to 20, this code is never triggered causing apktool to create apk files that crash on devices below Android 4.2.
This change passes the minimum api level to the smali library.