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

Ignore split configs when bundle config moves shared libraries to base.apk #8987

Merged
merged 6 commits into from
Jun 17, 2024

Conversation

grendello
Copy link
Contributor

@grendello grendello commented May 27, 2024

Fixes: #8979

When bundle configuration uses standard settings for split configs, the per-ABI library
directory (which contains all of our DSOs/assemblies/blobs etc) will be placed in a per-ABI
split config file named split_config.{ARCH}.apk and we use the fact to optimize startup
time.

However, if a custom build config file with the following settings is found, Android bundletool
doesn't create the per-ABI split config file, and so we need to search all the files in order
to find shared libraries, assemblies/blobs etc:

{
  "optimizations": {
    "splitsConfig": {
      "splitDimension": [
        {
          "value": "ABI",
          "negate": true
        }
      ],
    }
  }
}

The presence or absence of split config files is checked in our Java startup code which will
notice that split configs are present, but will not check (for performance reasons, to avoid
string comparisons) whether the per-ABI split config is present. We, therefore, need to let
our native runtime know in some inexpensive way that the split configs should be ignored and
that the DSOs/assemblies/blobs should be searched for in the usual, non-split config, way.

Since we know at build time whether this is the case, it's best to record the fact then and
let the native runtime merely check a boolean flag instead of dynamic detection at each app
startup.

EnvironmentHelper.ApplicationConfig app_config = EnvironmentHelper.ReadApplicationConfig (envFiles);

Assert.That (app_config, Is.Not.Null, "application_config must be present in the environment files");
Assert.AreEqual (app_config.ignore_split_configs, true, $"App config should indicate that split configs must be ignored");
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably add additional code like

Assert.IsTrue (appBuilder.Install (app), $"{app.ProjectName} should install");
				RunProjectAndAssert (app, appBuilder);
				Assert.True (WaitForActivityToStart (app.PackageName, "MainActivity",
					Path.Combine (Root, appBuilder.ProjectDirectory, "logcat.log"), 30), "Activity should have started.");

To make sure the app actually installs and runs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put the run code in a separate test InstallAndRun

Copy link
Contributor

Choose a reason for hiding this comment

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

the AdbStartActivity method does not check if the app started. It just does a launch,
You are better off using

RunProjectAndAssert (app, appBuilder);
Assert.True (WaitForActivityToStart (app.PackageName, "MainActivity",
		Path.Combine (Root, appBuilder.ProjectDirectory, "logcat.log"), 30), "Activity should have started.");

@grendello grendello marked this pull request as ready for review May 28, 2024 18:12
grendello added 6 commits June 3, 2024 11:45
* main: (22 commits)
  Bump to dotnet/android-tools@1c09dcc (#9026)
  Bump to dotnet/java-interop@ccafbe6 (#9025)
  [Mono.Android-Tests] Fix repo URL in redirect tests (#9035)
  [ci] Update checkout path for nightly build (#9028)
  [ci] Fix android source path for MAUI test job (#9030)
  Link Code of Conduct (#9034)
  [ci] Update sdk-insertions trigger to manual only (#9029)
  Update java-interop and android-tools submodule mentions (#9023)
  LEGO: Merge pull request 9022
  [Xamarin.Android.Build.Tasks] fastdev works with aab files (#8990)
  Use new binutils URL (#9019)
  Localized file check-in by OneLocBuild Task: Build definition ID 17928: Build ID 9686669 (#9011)
  LEGO: Merge pull request 9015
  [api-merge] Update "constant" values to mirror latest API levels (#9004)
  [Mono.Android] Fix wrong value for `ApplicationExitInfoReason.Other` (#9003)
  [Mono.Android] Fix omitted Gl* constants. (#9009)
  [manifest-attribute-codegen] Generate custom attribute declarations (#8781)
  [tests] Reduce default build output verbosity (#9002)
  [templates] Update Wear OS en template string (#9005)
  [build] Do not provision JDK 8 (#8999)
  ...
@grendello grendello force-pushed the dev/grendel/bundle-and-blobs branch from f1f4018 to 5b87b56 Compare June 17, 2024 10:30
@dellis1972 dellis1972 merged commit a954a33 into main Jun 17, 2024
58 checks passed
@dellis1972 dellis1972 deleted the dev/grendel/bundle-and-blobs branch June 17, 2024 12:28
grendello added a commit that referenced this pull request Jun 21, 2024
* main: (26 commits)
  Make APK and shared library alignment configurable (#9046)
  [r8] update proguard rule to keep .NET runtime classes (#9044)
  Explicitly align to 4k (#9041)
  [trimming] preserve custom views and `$(AndroidHttpClientHandlerType)` (#8954)
  Ignore split configs when bundle config moves shared libraries to base.apk (#8987)
  Bump to dotnet/android-tools@1c09dcc (#9026)
  Bump to dotnet/java-interop@ccafbe6 (#9025)
  [Mono.Android-Tests] Fix repo URL in redirect tests (#9035)
  [ci] Update checkout path for nightly build (#9028)
  [ci] Fix android source path for MAUI test job (#9030)
  Link Code of Conduct (#9034)
  [ci] Update sdk-insertions trigger to manual only (#9029)
  Update java-interop and android-tools submodule mentions (#9023)
  LEGO: Merge pull request 9022
  [Xamarin.Android.Build.Tasks] fastdev works with aab files (#8990)
  Use new binutils URL (#9019)
  Localized file check-in by OneLocBuild Task: Build definition ID 17928: Build ID 9686669 (#9011)
  LEGO: Merge pull request 9015
  [api-merge] Update "constant" values to mirror latest API levels (#9004)
  [Mono.Android] Fix wrong value for `ApplicationExitInfoReason.Other` (#9003)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Jul 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App Crashes when using bundle.config with abi splits set to negate.
2 participants