[Xamarin.Android.Build.Tasks] Replace GetExtraPackages with XmlPeek by simonrozsival · Pull Request #13486 · dotnet/android

simonrozsival · 2026-11-11T11:39:56Z

Context: #12988

The GetExtraPackages task only reads /manifest/@package from each library's AndroidManifest.xml under $(IntermediateOutputPath)__library_projects__. MSBuild can do this itself, so this PR replaces the task with a _GetAaptExtraPackages target that uses XmlPeek, adds it to the DependsOnTargets of _PrepareUpdateAndroidResgen and _PrepareCreateBaseApk, and deletes GetExtraPackages.cs and its <UsingTask/>.

The target behaves like the task in these ways:

It searches each library directory in enumeration order: first $(_LibraryProjectImportsDirectoryName), then library_project_imports.
It prefers bin/AndroidManifest.xml over AndroidManifest.xml.
It removes duplicates case-sensitively with DistinctWithCase(), keeps the original order, and joins the results with :.

Behavior differences, for inputs that are invalid or not expected in practice:

A manifest that has no package attribute, or whose root is not <manifest>, is skipped instead of raising XAGEP7009.
Malformed XML reports MSB3733 from XmlPeek instead of XAGEP7009.
package="" reports MSB4028.
DTD default attributes and DTD entities are not resolved.

This PR deletes GetExtraPackages.cs, which #12988 also edits, so whichever PR merges second will need a rebase.

Net change: +24/−83.

Testing

I ran a differential harness that runs the deleted task and the new target on the same fixtures, using a private .NET 12 SDK. Both produce identical results for:
- the custom and library_project_imports locations
- bin/ precedence
- per-library and directory ordering
- case-sensitive duplicates
- paths with spaces, ;, %, ' and []
- missing manifests or library directories
- package values containing whitespace, ;, % and XML entities
The only mismatches were the invalid-input cases listed above.
I did not run the NUnit build tests locally because this worktree has no local SDK build. Every app build that references a library project runs this target, so CI provides coverage.

Context: #12988 The `GetExtraPackages` task enumerates `$(IntermediateOutputPath)__library_projects__/*`, looks in each library's `$(_LibraryProjectImportsDirectoryName)` and `library_project_imports` directories for `bin/AndroidManifest.xml` (preferred) or `AndroidManifest.xml`, reads `/manifest/@package`, and returns the distinct values joined with `:` as `$(AaptExtraPackages)` for aapt2. All of that can be expressed with built-in MSBuild: `System.IO.Directory.GetDirectories()` for enumeration, `Exists()` conditions for manifest selection, `<XmlPeek/>` for the package attribute, and `DistinctWithCase()` to keep the original ordinal (case-sensitive) de-duplication and ordering. Add a `_GetAaptExtraPackages` target, run it via `DependsOnTargets` from `_PrepareUpdateAndroidResgen` and `_PrepareCreateBaseApk`, and delete `GetExtraPackages.cs`. Behavioral differences, all on inputs that aapt2 would not accept anyway: * A manifest without a `package` attribute is skipped instead of failing the build. * Malformed XML reports `<XmlPeek/>`'s MSB3733 instead of a wrapped `XmlException`. * `package=""` causes MSB4028 instead of producing an empty `:`-separated entry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Context: f363a8d f363a8d declared the `Manifest` metadata inside the same `<_AndroidLibraryImportsDir/>` element whose `Include` used item transforms, so `%(Identity)` did not refer to each import directory and no manifests were found; `$(AaptExtraPackages)` was always empty. Set the metadata in a separate item-update element, and batch over `%(_AndroidLibraryProjectDir.Identity)` so each library's custom and `library_project_imports` directories are visited together, matching the previous `GetExtraPackages` ordering. A differential harness comparing the deleted task with the new target now produces identical results for both import locations, `bin/AndroidManifest.xml` precedence, directory order, case-sensitive de-duplication, and escaped paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Normal app-build CI generally exercises only empty results, leaving nonempty-manifest behavior without the requested automated target-level regression coverage.

1 open issue

Add automated target tests for nonempty resource manifests · New

What changed in this PR

Replaces the custom GetExtraPackages task with MSBuild’s XmlPeek to simplify library-manifest package collection in the Android resource build pipeline.

Changes:

Adds a shared _GetAaptExtraPackages target for resource generation and base APK preparation.
Preserves manifest precedence, package ordering, and case-sensitive deduplication, with documented differences for invalid inputs.
Removes the obsolete task, its registration, and its diagnostic-prefix documentation.

File	Description
src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets	Adds the shared XML-reading target and replaces both task invocations.
src/Xamarin.Android.Build.Tasks/Tasks/GetExtraPackages.cs	Deletes the superseded custom task.
Documentation/docs-mobile/messages/index.md	Removes the retired `GEP` task-prefix entry.

_{Review effort: Balanced}

+    </_AndroidLibraryImportsDir>
+    <_AndroidLibraryManifest Include="%(_AndroidLibraryImportsDir.Manifest)" Condition="Exists('%(_AndroidLibraryImportsDir.Manifest)')" />
+  </ItemGroup>
+  <XmlPeek XmlInputPath="%(_AndroidLibraryManifest.Identity)" Query="/manifest/@package" Condition="'@(_AndroidLibraryManifest)' != ''">


simonrozsival · 2026-11-11T14:48:59Z

/review

github-actions · 2026-11-11T14:49:59Z

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #13486

github-actions

✅ LGTM

No blocking correctness issues found. 0 errors · 0 warnings · 1 suggestion (inline: preserve focused regression coverage).

The replacement retains the two import-directory locations, bin/AndroidManifest.xml precedence, and case-sensitive package deduplication. Both previous callers depend on the shared target, and the task registration and diagnostic-prefix documentation are removed consistently. The invalid-input behavior differences are explicitly documented in the PR description.

CI: All 41 checks returned for the PR head are completed successfully, including the overall Azure DevOps dotnet-android check for build 1675204. Independent Azure build/timeline inspection was unavailable because az devops invoke required login; this CI assessment is based on the published GitHub checks. No local SDK build or test run was performed for this review.

Generated by Android PR Reviewer for #13486 · copilot · gpt61 · 37.1 AIC · ⌖ 7.43 AIC · ⊞ 19.5K
_{Comment /review to run again}

github-actions · 2026-11-11T14:55:13Z

 </PropertyGroup>

+<!-- Collects the `package` of each library's bin/AndroidManifest.xml or AndroidManifest.xml for aapt2 extra packages. -->
+<Target Name="_GetAaptExtraPackages">


🤖 💡 Testing — Could we check in a focused test that invokes this target against synthetic __library_projects__ fixtures and asserts the exact AaptExtraPackages value? Normal app builds use $(_AndroidLibrayProjectIntermediatePath) (default $(IntermediateOutputPath)lp\), and ResolveLibraryProjectImports.Extract() deletes the old __library_projects__ directory when upgrading. Consequently, those builds normally exercise the empty-directory path, not the new XmlPeek batching. Preserving the differential harness cases for both import-directory names, bin/ precedence, and case-sensitive deduplication would make the equivalence checks repeatable in CI.

{Rule: Test edge cases}

simonrozsival and others added 2 commits November 11, 2026 12:37

simonrozsival requested a review from jonathanpeppers as a code owner November 11, 2026 11:39

Copilot AI balanced review requested due to automatic review settings November 11, 2026 11:39

Copilot started reviewing on behalf of simonrozsival November 11, 2026 11:40

View session

Copilot AI reviewed Nov 11, 2026

View reviewed changes

simonrozsival mentioned this pull request Nov 11, 2026

[Xamarin.Android.Build.Tasks] Replace CreateResgenManifest with WriteLinesToFile #13496

Open

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

github-actions Bot reviewed Nov 11, 2026

View reviewed changes

Navigation Menu

Repository navigation

[Xamarin.Android.Build.Tasks] Replace GetExtraPackages with XmlPeek - #13486

[Xamarin.Android.Build.Tasks] Replace GetExtraPackages with XmlPeek#13486
simonrozsival wants to merge 2 commits into
mainfrom
simonrozsival-extra-packages-to-xml

simonrozsival commented Nov 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

simonrozsival commented Nov 11, 2026

Uh oh!

github-actions Bot commented Nov 11, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Nov 11, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

simonrozsival commented Nov 11, 2026

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

simonrozsival commented Nov 11, 2026

Uh oh!

github-actions Bot commented Nov 11, 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

✅ LGTM

Uh oh!

github-actions Bot Nov 11, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Nov 11, 2026 •

edited

Loading