facebook / buck

A fast build system that encourages the creation of small, reusable modules over a variety of platforms and languages.
https://buck.build
Apache License 2.0
8.56k stars 1.16k forks source link

Android App Bundles - Add Play feature delivery support for dex and native features in Buck #2592

Closed uKetki closed 3 years ago

uKetki commented 3 years ago

The complete implementation is divided into 5 commits with detailed description are as follows:

[1/n Dynamic Feature Support] Create Android Optimiser layer and segregate responsibility of AndroidBinaryBuildable and AndroidBinaryOptimizer

Looking at the buck code for generating apk and aab, it is evident that AndroidBinaryBuildable is bloated with a lot of responsibilities ranging from apk/aab generation, signing to optimization.

The overview of current buck architecture:

Existing Binary generation logic

The suggestion is to refactor existing code and create two separate layers: Optimizers and Buildable which can re-use the common logic for binary generation and offload the specific implementation to the respective layers: BuildableOptimizer

This approach will later help to add changes needed to support dynamic feature delivery.

[2/n Dynamic Feature Support] Update documentation of android_bundle rule to include changes required for dynamic features

[3/n Dynamic Feature Support] Add new configuration flag useDynamicFeature to support dynamic features related attributes

[4/n Dynamic Feature Support] Define new configuration application_modules_with_manifest to decouple the manifest behavior from the resources

[5/n Dynamic Feature Support] Enable buck to support dynamic features with native libraries

Resulted App Bundle overview (before changes in buck):

Screenshot 2021-02-17 at 11 06 39 AM

Resulted App Bundle overview (after changes in buck):

Screenshot 2021-02-17 at 11 23 29 AM

Closes #2586 | _Tested feature by including custom buck pex file in PlayFeatureDelivery sample._

uKetki commented 3 years ago

macos_test_integration test failed with the below reason which seems unrelated to the changes:

FAILURE com.facebook.buck.testrunner.RunWithDefaultTimeoutIntegrationTest testRunWithHonorsDefaultTimeoutOnTestThatRunsLong: Should fail due to exceeding timeout. Expected exit code 32 but was 0.
java.lang.AssertionError: Should fail due to exceeding timeout. Expected exit code 32 but was 0.
rajyengi commented 3 years ago

Looks like all of the automated CI has passed, I haven't fully had a chance to review this yet but don't want to hold this up too long. This is ready to go @uKetki?

uKetki commented 3 years ago

Thanks, @rajyengi. This PR is completed from my side, I am planning to create a separate PR with the testdata and related Buck.fixture files.

rajyengi commented 3 years ago

Sounds good, since CI looks fine for now and the feature is behind useDynamicFeature flag, I'll go ahead and merge this. Looking forward to seeing the test data PR though.