Skip to content
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

Add support for app bundle #26

Closed
wants to merge 3 commits into from

Conversation

nesterov-n
Copy link
Contributor

PR is related to #19

Problem
SoLoader cannot load .solibraries when an application is installed through App Bundle with config:

bundle {
    abi {
        enableSplit = true
    }
}

The reason is that app bundle installation consists of multiple apks. For instance:

  • base.apk
  • split_config.arm64_v8a.apk
  • split_config.xxxhdpi.apk

SoLoader class creates ApkSoSource pointing only to base.apk. But *.so files exist only in split_config.arm64_v8a.apk

Solution
Create separate ApkSoSource for each apk in the application folder. I couldn't figure out how to filter out apk without lib folder.

Testing
I haven't tried this with dynamic feature modules yet. So I can't say that it fully fixes #19. But at least it fixes facebook/fresco#2253
Any help with dynamic modules testing would be much appreciated.

P.S.
Due to some reasons, I can't wait for the new release of SoLoader lib, so I have published a temporary patched version to bintray in my account
with different grouipId com.avito.android:patched-soloader:0.1.0
If you are against publishing your lib in a different account, please let me know. I'll remove my version immediately.

@nesterov-n
Copy link
Contributor Author

Previous versions didn't work on pre-lollipop devices. I've fixed it.

@passy
Copy link
Member

passy commented Jan 15, 2019

Thanks for the PR. This looks good to me, but I'm not the best person to review this. I'll try to find someone internally. :)

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@passy has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
Log.d(TAG, "adding backup sources from split apks");
int splitIndex = 0;
for (String splitApkDir : context.getApplicationInfo().splitSourceDirs) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

splitSourceDirs may be null in some cases so I suggest to add a necessary check here.
So does aosp in its sources. e.g https://github.com/aosp-mirror/platform_frameworks_base/blob/master/core/java/android/app/LoadedApk.java#L955

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Fixed

@facebook-github-bot
Copy link

@nesterov-n has updated the pull request. Re-import the pull request

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@passy has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@mannodermaus
Copy link

mannodermaus commented Jan 21, 2019

Is there an approximate ETA on when this fix will be included in SoLoader? We're facing a problem with React Native because of this same root cause.

In the meantime, @nesterov-n, would you be so kind as to publish another version of your patched module, which includes 1a044bd? Thank you in advance, and sorry for the trouble 🙇
Edit: I realize that splitSourceDirs will never be null in case of an AAB-originated app anyway, so it's not an urgent requirement to add this to the patched SoLoader.

@passy
Copy link
Member

passy commented Jan 21, 2019

@mannodermaus Sorry, I can't give you any estimates. I would recommend using a patched version in the meantime or @nesterov-n's published artifact.

@beatrizz
Copy link

For whatever it's worth, we're also impacted by this. We use Fresco, which uses SoLoader, and we can't use AABs until this is solved or we build our own version of Fresco that uses @nesterov-n 's version of SoLoader.
An ETA would help us make a decision but I understand that it's not always possible to provide one.

@nesterov-n
Copy link
Contributor Author

nesterov-n commented Jan 21, 2019

@mannodermaus I've published the new version of patched-soloader that includes 1a044bd. You can check it with com.avito.android:patched-soloader:0.1.1
P.S. I accidentally published two files with same version number 0.1.1 so you might have to run build with --refresh-dependencies flag to update your local build cache.

@mannodermaus
Copy link

@beatrizz, while waiting for the official update to SoLoader, will @nesterov-n's workaround work for you? You exclude Fresco's transitive dependency on SoLoader, then add the patched artifact instead.

@YuriDenison
Copy link

@nesterov-n, thank you for the fix.

I accidentally published two files with same version number 0.1.1

Could you please publish 0.1.2 then, cuz refreshing dependecies on all our CI build agents and all developer's laptops is not a good idea. Thank you!

@nesterov-n
Copy link
Contributor Author

@YuriDenison Done. You can check it com.avito.android:patched-soloader:0.1.2

@nesterov-n
Copy link
Contributor Author

@beatrizz

We can't use AABs until this is solved or we build our own version of Fresco that uses @nesterov-n 's version of SoLoader.

You don't have to build your own version of Fresco. All you need to make Fresco works with app bundle is adding these lines in your build.gradle

repositories {
    maven {
        url  "https://dl.bintray.com/nnesterov/maven" 
    }
}

compile('com.facebook.fresco:fresco:1.10.0') {
    // remove original soloader from fresco transitive dependency
    exclude group: 'com.facebook.soloader', module: 'soloader'  
}
// add dependency from patched version
compile("com.avito.android:patched-soloader:0.1.2")

That's all. We have already tested it on production. Works well.

@beatrizz
Copy link

Thanks @mannodermaus and @nesterov-n ! I didn't know you could just change the library like that.
It seems a bit fragile (e.g. we'd need to be careful if we updated to a new Fresco version that relied on something that was only in a new SoLoader version) but it'll work as a temporary fix. Hopefully the fix will be in SoLoader and Fresco soon.

@efrohnhoefer
Copy link

Only noticed this issue when upgrading to Android Gradle Plugin (AGP) 3.3. We are currently working around the issue by building bundle with AGP 3.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants