[TrimmableTypeMap] Merge library .aar manifests on the legacy merger path by simonrozsival · Pull Request #12000 · dotnet/android

simonrozsival · 2026-07-07T09:12:00Z

Summary

On the trimmable typemap path, the app AndroidManifest.xml was missing content declared by referenced library .aar manifests (permissions, activities, providers, meta-data) when AndroidManifestMerger=legacy. The manifestmerger.jar path was unaffected because it merges library manifests downstream in _ManifestMerger.

The trimmable ManifestGenerator already contained the merge logic (MergeLibraryManifests + FixupNameElements + ${applicationId}/placeholder substitution + duplicate-element removal), but it was never fed the extracted library manifests, so on the legacy path nothing was merged.

Fix

Wire the extracted library manifests through to the generator:

@(ExtractedManifestDocuments) → GenerateTrimmableTypeMap.MergedManifestDocuments → ManifestConfig.LibraryManifests → ManifestGenerator.LibraryManifests.

_GenerateTrimmableTypeMap now depends on _GetLibraryImports (which populates @(ExtractedManifestDocuments)) and includes those documents in its Inputs for incrementality.
The library manifests are only passed on the legacy merger path (AndroidManifestMerger == 'legacy').
Merge failures now surface as an XA4302 warning via a new LogLibraryManifestMergeWarning logger method, matching the legacy ManifestDocument behavior.

Tests

Adds two ManifestGeneratorTests:

LibraryManifests_MergedWithApplicationIdAndRelativeNamesResolved — verifies ${applicationId} resolves to the app package, relative .Type component names are qualified with the library's own package, and duplicate meta-data collapses to one element (mirrors ManifestTest.MergeLibraryManifest).
LibraryManifests_MissingFileIgnored — a non-existent library manifest path is skipped without throwing.

This fixes ManifestTest.MergeLibraryManifest on the trimmable/NativeAOT path.

…path The trimmable manifest generator (ManifestGenerator) already had the library manifest merge logic (MergeLibraryManifests + FixupNameElements + placeholder substitution), but it was never fed the extracted library (.aar) AndroidManifest.xml files, so on the legacy manifest-merger path the app manifest was missing library-declared permissions/activities/providers. The manifestmerger.jar path was unaffected (it merges downstream in _ManifestMerger). Wire @(ExtractedManifestDocuments) through: _GenerateTrimmableTypeMap (depends on _GetLibraryImports; adds the docs to Inputs) -> GenerateTrimmableTypeMap.MergedManifestDocuments -> ManifestConfig.LibraryManifests -> ManifestGenerator.LibraryManifests. Also surface merge failures via a new XA4302 logger warning (LogLibraryManifestMergeWarning), matching the legacy ManifestDocument path. Fixes ManifestTest.MergeLibraryManifest on the trimmable/NativeAOT path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR fixes missing .aar-provided manifest content (permissions/activities/providers/meta-data, etc.) when using the trimmable typemap path together with AndroidManifestMerger=legacy, by ensuring extracted library manifests are passed into the trimmable manifest generator and merged there (matching the behavior that manifestmerger.jar already provides downstream).

Changes:

Wire @(ExtractedManifestDocuments) into GenerateTrimmableTypeMap and down into ManifestGenerator.LibraryManifests (legacy merger path only).
Add XA4302-based warning plumbing for library-manifest merge failures via ITrimmableTypeMapLogger.LogLibraryManifestMergeWarning.
Add unit tests validating ${applicationId} substitution, relative name qualification, duplicate element de-dupe, and missing file handling.

Show a summary per file

File	Description
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TrimmableTypeMapGeneratorTests.cs	Extend test logger interface implementation for the new merge-warning callback.
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/ManifestGeneratorTests.cs	Add coverage for library manifest merging behaviors (placeholders, relative names, de-dupe, missing file).
src/Xamarin.Android.Build.Tasks/Tasks/GenerateTrimmableTypeMap.cs	Add task parameter for merged library manifests and pass through into `ManifestConfig`.
src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets	Ensure `_GenerateTrimmableTypeMap` runs with library imports available and passes extracted manifests on legacy path.
src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapTypes.cs	Extend `ManifestConfig` to carry library manifest paths.
src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs	Feed library manifests + warning handler into `ManifestGenerator`.
src/Microsoft.Android.Sdk.TrimmableTypeMap/ITrimmableTypeMapLogger.cs	Add logger API for surfacing library manifest merge warnings.

Copilot's findings

Files reviewed: 7/7 changed files
Comments generated: 1

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

simonrozsival · 2026-07-07T16:02:51Z

/review

github-actions · 2026-07-07T16:03:43Z

✅ Android PR Reviewer completed successfully!

github-actions

🤖 Android PR Review — ✅ LGTM (2 minor suggestions)

Solid, well-scoped fix. On the trimmable typemap path with AndroidManifestMerger=legacy, referenced-library .aar manifest content (permissions/activities/providers/meta-data) was dropped because the generator's already-present merge logic (MergeLibraryManifests + FixupNameElements + placeholder substitution + dedup) was never fed the extracted manifests. The PR wires @(ExtractedManifestDocuments) → MergedManifestDocuments → ManifestConfig.LibraryManifests → ManifestGenerator.LibraryManifests, matching how the LlvmIr/GenerateJavaStubs sibling already handles it.

What I verified

✅ Merge ordering in Generate is correct and documented: merge → dedup → strip node="remove" → placeholder substitution, so ${applicationId} resolves in merged content and duplicate meta-data collapses. Mirrors ManifestDocument.
✅ @(ExtractedManifestDocuments) added to Inputs + DependsOnTargets="_GetLibraryImports" — incrementality is correct (it over-includes on the non-legacy path, which is safe: over-inclusion never yields stale output).
✅ _MergedManifestDocuments is gated to AndroidManifestMerger == 'legacy', so there is no double-merge with manifestmerger.jar.
✅ XA4302 is reused (not a new code) and matches the legacy ManifestDocument.cs usage; the resource string and docs/index already exist.
✅ MergedManifestDocuments is correctly non-[Required]/nullable per MSBuild task conventions; no ! operators; Mono formatting preserved.
✅ Coverage: new ManifestGenerator unit tests plus the NativeAOT-parameterized ManifestTest.MergeLibraryManifest integration test exercise the end-to-end wiring.
✅ CI: all 42 dotnet-android checks are green. (mergeable_state: blocked is the pending required review, not CI.)

Suggestions (non-blocking, posted inline)

💡 The Action<int, string> warning-callback design vs. the existing WarnInvalidPlaceholder pattern (+ the silently-dropped code-0 warning).
💡 A missing test for the XA4302 merge-failure path — the branch the Action<int,string> refactor exists to serve.

Nice work — mirroring ManifestDocument semantics and reaching parity with the LlvmIr path are exactly right.

Severity tally: 0 ❌ · 0 ⚠️ · 2 💡

Generated by Android PR Reviewer for #12000 · 290.8 AIC · ⌖ 27.2 AIC · ⊞ 6.8K
_{Comment /review to run again}

github-actions · 2026-07-07T16:15:48Z

 			ForceExtractNativeLibs = forceDebuggable,
 			ManifestPlaceholders = config.ManifestPlaceholders,
 			ApplicationJavaClass = config.ApplicationJavaClass,
+			Warn = (code, message) => {


🤖 💡 Patterns — Warn was widened to Action<int, string>? and now multiplexes two unrelated diagnostics through magic codes (0 = ignore, 4302 = surface as XA4302). ManifestGenerator already models exactly this case with a dedicated Action<string>? WarnInvalidPlaceholder; a parallel Action<string>? WarnLibraryManifestMerge would drop the 0 sentinel and this if (code == ...) routing, and keep the two warnings independently wireable.

Separately: the code == 0 path (the AssemblyLevelElementBuilder "Could not resolve ... type" warning) stays silently swallowed on the trimmable path. It was already dropped before this PR, so this is not a regression — but if that suppression is deliberate, a one-line note on why would save the next reader a spelunk.

(Consistency with the existing WarnInvalidPlaceholder callback)

github-actions · 2026-07-07T16:15:48Z

+	}
+
+	[Fact]
+	public void LibraryManifests_MissingFileIgnored ()


🤖 💡 Testing — Good coverage of the successful merge and the missing-file skip. The remaining gap is the merge-failure branch: a malformed library manifest that makes XDocument.Load throw and routes through Warn (LibraryManifestMergeWarningCode, ...) → XA4302. Surfacing that warning is the entire reason for the Action<int, string> refactor in this PR, so a test that points LibraryManifests at an invalid .xml, sets gen.Warn to capture the callback, and asserts the merge-failure message fires would lock in the new routing.

(Rule: Test error paths, not just the happy path)

Copilot AI review requested due to automatic review settings July 7, 2026 09:12

simonrozsival requested a review from jonathanpeppers as a code owner July 7, 2026 09:12

Copilot started reviewing on behalf of simonrozsival July 7, 2026 09:12 View session

simonrozsival added copilot `copilot-cli` or other AIs were used to author this trimmable-type-map labels Jul 7, 2026

Copilot AI reviewed Jul 7, 2026

View reviewed changes

Comment thread src/Microsoft.Android.Sdk.TrimmableTypeMap/TrimmableTypeMapGenerator.cs

Copilot started work on behalf of simonrozsival July 7, 2026 09:23 View session

Change ManifestGenerator.Warn to Action<int, string> with warning code

7c25603

Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>

Copilot finished work on behalf of simonrozsival July 7, 2026 09:55

simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Jul 7, 2026

simonrozsival enabled auto-merge (squash) July 7, 2026 16:03

github-actions Bot reviewed Jul 7, 2026

View reviewed changes

jonathanpeppers approved these changes Jul 7, 2026

View reviewed changes

simonrozsival merged commit 977fa65 into main Jul 7, 2026
42 checks passed

simonrozsival deleted the dev/simonrozsival/trimmable-merge-library-manifests branch July 7, 2026 21:05

simonrozsival mentioned this pull request Jul 7, 2026

[NativeAOT] Make the trimmable typemap the default #11822

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[TrimmableTypeMap] Merge library .aar manifests on the legacy merger path#12000

[TrimmableTypeMap] Merge library .aar manifests on the legacy merger path#12000
simonrozsival merged 2 commits into
mainfrom
dev/simonrozsival/trimmable-merge-library-manifests

simonrozsival commented Jul 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

simonrozsival commented Jul 7, 2026

Uh oh!

github-actions Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Jul 7, 2026

Uh oh!

github-actions Bot Jul 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

simonrozsival commented Jul 7, 2026

Summary

Fix

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

simonrozsival commented Jul 7, 2026

Uh oh!

github-actions Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 Android PR Review — ✅ LGTM (2 minor suggestions)

What I verified

Suggestions (non-blocking, posted inline)

Uh oh!

github-actions Bot Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Jul 7, 2026 •

edited

Loading