[TIMOB-18082] Android: Enable --multi-dex for android builds #8095
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Unless the support library is included, it should have a tiapp check to be sure the min target is android 5 or higher |
|
For lesser than Android 5.0 https://developer.android.com/topic/libraries/support-library/features.html#multidex But for now, just setting the min to API 21 if multi-dex is chosen should be a good step. |
|
For testing, I think adding loads and loads of modules would make it hit the 64k limit fast. |
|
to be able to actually compile titanium, I have to add android-support-multidex.jar to our libraries anyways. I assume that'll get baked in like the android-support-v4/android-support-v7 do. |
|
To easily test this, I've created a Module compiled against 5.3.0.GA (you can easily recompile it against what you need) https://github.com/ashcoding/ReachTheLimit/releases/tag/1.0.0 This module is for Android. Simply add it to a project and add the reference to it in the project You should be able to reach the limit and get the error:- |
|
Awesome, thanks @ashcoding 👍 I was able to re-compile the module for 6.0.0 and confirmed that I hit the error. I then built the SDK with the changes from this PR and re-ran and it ran fine. Looks like the PR works. |
|
OK, I think this PR is ready for review/merge. |
|
For testing purposes, the module can be downloaded here https://github.com/ashcoding/ReachTheLimit/releases/tag/2.0.0 This supports 6.0.0. |
|
Tried this on an Emulator running Android 4.0.4 (API 15). Crashes as follows:- Tried on a SM-G900F, Samsung Galaxy S5, Android 4.4.2 (API 19). Crashes as follows:- It works fine on Nexus 6 running Android 6.0 (API 23). |
|
Nexus 6 running Android 6.0 (API 23) is also able to crash. Crash as follows:- |
|
The test code I'm using is var Reachthelimit = require('com.appcelerator.reachthelimit');
var limitBreaker = Reachthelimit.createExample({message: "hello world"});
var win = Ti.UI.createWindow();
win.open();
limitBreaker.hitTheLimits();With the module https://github.com/ashcoding/ReachTheLimit/releases/tag/2.0.0 |
|
@ashcoding So the first issue looks like it may have some stack trace missing. It looks similar to when I was missing the android-support-multidex.jar library (so it couldn't load the MultiDex class). I thought this commit fixed that: df2cf69 The second issue appears to be related to the V8 update. Looks like cleaning up native modules triggers some bad behavior/crash there. I'll have to try and reproduce and see... |
|
@ashcoding I just tried against a 4.4.2 (19) x86 emulator and it seemed to work fine for me. Maybe you tested against a version without that last commit? Can you retry please? |
|
Retried. |
|
OH. Do I need to recompile my module? Let me try that. |
|
Ok. That works on Android 6.0, Nexus 6. No issues. |
|
It's Failing on SM-G900F, Galaxy S5 running Android 4.4.2. It uses armeabi-v7a. |
|
@sgtcoolguy I'm able to reproduce the same errors as Ash on the devices :( |
|
Tried it again. The only device that's working is Nexus 6, Android 6.0. The other devices we have are failing. I tried it on the latest https://github.com/ashcoding/ReachTheLimit/releases/tag/2.0.2 which was recompiled with the latest master at the time this was written. |
|
Well I am not originally an Android dev, so I do not know how to properly fix this one. On older APIs, the support library needs to be packaged into the app. How do we do that? |
|
Perhaps, this is explaining what is happening. http://stackoverflow.com/a/32780470/6207774 From what I can see from the test we did, it is working on Android 6.0 and failing in Android 4.4.2. On Pre-L:
|
|
So it seems like what we need is the classes used to start the app to be in the main dex and everything else can be in the secondary dex. This would include ensuring the classes called in here https://github.com/appcelerator/titanium_mobile/blob/master/android/titanium/src/java/org/appcelerator/titanium/TiApplication.java#L361-L382 to be included in the main dex to ensure that the app is started correctly. And any other classes that we need to identify that is required. Looks like there's a few more things that's needed to be done if this is to be merged and functional for it to support for Android Lollipop and below. Related readings:
Otherwise, if we are only targeting Android L and above only, perhaps we can use this. Meaning, if detected that a person is compiling for Android L and above, to allow for this code to be used. |
|
@ashcoding I think you're jumping too far ahead. The likely cause is because multidex support was in a separate library until it got folded directly into Android core. So my understanding is that on API level 21 and lower we need to make sure the multidex support library gets added to the class path/into the app. I thought I had done so with my changes above. Apparently that's not enough. How do we force a 3rd-party JAR into the app? I thought this would do so: https://github.com/sgtcoolguy/titanium_mobile/blob/7175ae0f54785adb8df024e01d7801c2afe5412b/android/dependency.json#L29 Do we not have other cases of support libraries we're bundling into the user's app for them? Again, I haven't typically does Android SDK dev so I'm not as familiar, but surely we've done this before... |
|
This is a PR I did some time back to include cardview |
|
I believe you might be right. You could be missing these:
|
|
I'm still facing the issue. 😞 |
… the build-tools mainDexClasses script (because it's a bash script, and because it doesn't handle args with spaces correctly). So we need to use proguard and MainDexListBuilder class in dx.jar ourselves - since it looks liek gradle now does it for you?
|
@ashcoding @cheekiatng OK, I pushed another update. I was unable to reproduce the same error locally, but went on Ash's hunch that we need to inform dx about what files to keep in the first/main dex file. I was seeing some odd stuff where it complained about not being able to find Anyhow, after a lot of Googling and frustration my solution is based on this: http://blog.osom.info/2014/10/generating-main-dex-list-file.html Android has a
I think we should probably guard doing all this extra work based on the target API level of the build? It appears it's not necessary on API 21+? Also disconcerting is that if you google more about this, you run into some source about the process/gradle here: https://android.googlesource.com/platform/tools/base/+/9fba95348ef762fd9a7c82ac332d119684a74bcb/build-system/gradle/src/main/groovy/com/android/build/gradle/internal/tasks/multidex/CreateMainDexList.groovy Which shows that it actually uses a different class: |
|
I think you solved it! I'm not having any serious crashes on the SM-G900F, Galaxy S5 running Android 4.4.2. I'm getting this though Which I believe is not the fault of the PR. |
|
I believe it's the fault of my module and the way I've written it that causes it to overflow. I'll try to figure out another way to cause the multidex issue and try it with this. But it does seem it's reaching there. 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JIRA: https://jira.appcelerator.org/browse/TIMOB-18082
Description:
This calls dexer with the --multi-dex argument, modifies our CLI to copy *.dex (not just classes.dex), and modifies TiApplication to override attachBaseContext to call MultiDex.install (to support loading multiple dex files). We also include the android-support-multidex.jar as a library dependency.
This PR's functionality was confirmed with the module @ashcoding provided.