search

wizmer / syncorg

An implementation of MobileOrg for the Android platform
GNU General Public License v3.0
174 stars 23 forks source link

The number of method references in a .dex file cannot exceed 64K #49

Closed knezi closed 7 years ago

knezi commented 7 years ago

Hi,

the source code reached the famous problem of insufficiently big dex file and therefore it can't be build. As I see it, there are the following possibilities:

None of this is ideal. For me personally the easiest way would be just drop the support for older Android, but I believe there are still many users with Android below 21.

What should we do about it?

wizmer commented 7 years ago

Yes, I have seen it also. It came when I added the Google Drive API, so I think we won't go back under the limit by just a bit of clean up (I tried though). Using proGuard won't hurt I guess, it is worth trying. We can' t really drop support like that, that would not be cool for current users. Finally I guess we'll end up using multi-index...

On Fri, Mar 3, 2017 at 5:15 PM Jan Knížek notifications@github.com wrote:

Hi,

the source code reached the famous problem of insufficiently big dex file and therefore it can't be build. As I see it, there are the following possibilities:

  • drop support for older Android (the oldest supported version would be 5 (API 21)
  • use proGuard and hope it will shrink the code enough; that also mean update the build API from 24 to 25. It is probably just a temporary solution
  • use multiindex which results in longer starting time
  • clean up the code from unneeded methods

None of this is ideal. For me personally the easiest way would be just drop the support for older Android, but I believe there are still many users with Android below 21.

What should we do about it?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/wizmer/syncorg/issues/49, or mute the thread https://github.com/notifications/unsubscribe-auth/ABm2VrWATbeok5W6hsfW2GB-pN8LnP5hks5riDy2gaJpZM4MScmV .

aspiers commented 7 years ago

I believe there are still many users with Android below 21.

https://en.wikipedia.org/wiki/Android_version_history#/media/File:Android_version_distribution_(January_2017).png suggests you're right.

What should we do about it?

Of course cleaning up the code is always good, but I appreciate that takes time and effort.

Another possibility would be to introduce a new branch in the codebase which supports API < 21. Then the master branch would be free to drop the need to support that, but it would still be possible to provide the older version to those who need it. Anyone with free time and motivation would be free to switch that branch to multidex and also backport fixes/features where possible.

knezi commented 7 years ago

Ok, I've tried to add multidex support (https://github.com/knezi/syncorg/commit/881ce315734d23438ca85406e634a72447f5cbb4).

I've also increased the heap size, because it was not big enough.

The problem is that adding multidex support made lint throw over 400 issues (30 are errors, such as function not found) and Unit tests do not pass either. I'll look at it and try to resolve at least some of them.

But since it hasn't slowed down the loading time (on my Nexus 4 still pretty fast) I think we can use it after resolving the errors.

wizmer commented 7 years ago

Good job. I have merged your change in my local. I'll do a commit when it is all setup. Hope to do it tonight.

knezi commented 7 years ago

You can also merge the next one - removing JUnits tests, there were no check done anyway. Don't merge the last one though, it does not work.

wizmer commented 7 years ago

Lol, Android Studio just hinted me to use the dependency: com.google.android.gms:play-services-drive:10.2.0' instead of: compile 'com.google.android.gms:play-services:10.2.0' in order not to avoid the 64K .dex file problem.

And it worked...

So I reverted multiDexEnabled to false but we'll keep the line for the day the problem comes back for real.