Repository navigation
[Xamarin.Android.Build.Tasks] Extract .aar files with MSBuild targets - #13510
simonrozsival wants to merge 5 commits into
Conversation
Context: classic Xamarin.Android class libraries embedded Android
content in assemblies as `__AndroidLibraryProjects__.zip`,
`__AndroidNativeLibraries__.zip`, and `*.jar` resources. .NET for
Android libraries ship `.aar` files instead, but both formats were
handled by `<ResolveLibraryProjectImports/>`.
Move `.aar` extraction into the new `_AndroidExtractAarLibraries`
target, using `<GetFileHash/>`, `<Unzip/>`, and per-`.aar` partial
builds:
* Each `.aar` is extracted to `lp/[hash of path]/jl/`, skipping
`.jar` files other than `classes.jar` and `libs/*.jar`.
* `lp/[id].stamp` holds the `.aar` SHA256 and is only rewritten when
the content changes, so touching an unchanged `.aar` no longer
invalidates downstream targets.
* `lp/aar.map` maps `lp/[id]` to the `.aar` path for tool error
messages, and invalidates `_UpdateAndroidResgen` and
`_BuildLibraryImportsCache` when `.aar` files are added or removed.
* Items are computed from the extracted directories on every build,
which also fixes `.net/env/` files being dropped when an `.aar`
was not re-extracted.
`<ResolveLibraryProjectImports/>` now only handles classic assemblies.
Support for them is deprecated: emit XA0150 when such content is found.
Also remove the `__library_projects__` directory migration and an
unreachable `__AndroidEnvironment__` cleanup branch.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The AAR changes can leave stale resource IDs, mark failed extractions as complete, and pass macOS metadata files to d8/R8 as invalid jars.
3 open issues
1 file not reviewed
src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Generated file
What changed in this PR
This PR separates modern AAR extraction from legacy Xamarin.Android embedded-resource handling, moving AAR processing into incremental MSBuild targets to reduce unnecessary build work.
Changes:
- Add per-AAR extraction, content hashes, and path mapping for diagnostics and cache invalidation.
- Retain legacy extraction while introducing the documented XA0150 deprecation warning.
- Update tests and helpers for the hashed directory layout and incremental behavior.
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.EmbeddedResource.targets | Adds incremental AAR extraction and derives library items from extracted content. |
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets | Adds the AAR map to resource-generation inputs. |
| src/Xamarin.Android.Build.Tasks/Utilities/AssemblyIdentityMap.cs | Resolves hashed AAR directories and corrects index bounds checking. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/BuildOutput.cs | Adds an AAR directory lookup helper. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/ResolveLibraryProjectImportsTests.cs | Tests legacy warnings, extraction, and AAR map lookup. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs | Updates cache and incremental-build assertions. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs | Checks AAR extraction and unchanged-content stamp preservation. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BindingBuildTest.cs | Updates binding tests for hashed paths and hidden-file handling. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs | Uses AAR map lookup in custom-view resource tests. |
| src/Xamarin.Android.Build.Tasks/Tasks/ResolveLibraryProjectImports.cs | Removes AAR processing and warns about legacy embedded content. |
| src/Xamarin.Android.Build.Tasks/Tasks/JavaToolTask.cs | Recognizes hexadecimal import directories in tool diagnostics. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.resx | Adds XA0150 warning messages. |
| src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs | Adds accessors for the new warning messages. |
| Documentation/docs-mobile/TOC.yml | Adds XA0150 to documentation navigation. |
| Documentation/docs-mobile/messages/xa1004.md | Clarifies legacy archive errors versus AAR extraction errors. |
| Documentation/docs-mobile/messages/xa0150.md | Documents legacy-format deprecation and migration guidance. |
| Documentation/docs-mobile/messages/index.md | Adds XA0150 to the warning index. |
Review effort: Balanced
* Exclude `__MACOSX` entries from the `.jar` extraction pass, so
macOS metadata such as `libs/__MACOSX/._foo.jar` is not passed to
d8/R8.
* Extract changed `.aar` files in the batched
`_AndroidExtractChangedAarLibrary` target and only write the
`[id].stamp` and `extracted.stamp` of an `.aar` when all of its
steps succeeded. Errors are warnings in design-time builds; a
failed `.aar` is now extracted again by the next build.
* Add the content stamp of every `.aar` to
`@(_LibraryResourceDirectoryStamps)`, so removing all resources
from an `.aar` still invalidates `_UpdateAndroidResgen`.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use native directory separators consistently for extracted AAR contents. On Windows, mixed separators prevented custom-view resources from matching their recompilation inputs and made consumer ProGuard configuration paths inconsistent. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
⚠️ Needs Changes
2 inline findings: 1 ❌ error, 1
The extraction split is clear, and separating content hashes from completion timestamps supports the intended touch-only optimization. Deriving AAR items on every build also addresses the cached environment/ProGuard paths, and the resource-removal regression test is valuable. The previously reviewed macOS-entry filtering and resource-stamp fixes are present.
The remaining findings concern stale stamps after a failed AAR replacement and a false XA0150 warning after an assembly's legacy content is removed. Details and concrete regression scenarios are attached inline.
CI: All 40 GitHub checks report success, including the aggregate dotnet-android check for Azure build 1678826. Independent Azure build/timeline verification was blocked because az devops invoke required authentication.
Local verification: An isolated reproduction importing the production extraction targets was attempted, but .NET first-run initialization failed before MSBuild executed. The findings are based on source analysis, not a successfully executed reproduction. No repository files were changed.
Generated by Android PR Reviewer for #13510 · copilot · gpt61 · 107.8 AIC · ⌖ 4.9 AIC · ⊞ 19.5K
Comment /review to run again
* `_AndroidExtractChangedAarLibrary` deletes the `[id].stamp` and
`extracted.stamp` of an `.aar` before modifying its extraction
directory. Previously, extracting A, failing to extract a corrupt
B, then restoring A matched the stale hash of A and skipped
re-extraction, leaving the directory deleted.
* `<ResolveLibraryProjectImports/>` deletes the extraction directory
and stamp of an assembly that no longer contains legacy content,
instead of logging XA0150 for leftover directories on later builds.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
There was a problem hiding this comment.
⚠️ Needs Changes
Findings: 0 errors, 1 warning, 0 suggestions. The new resource-presence checks discard a boolean result rather than asserting it; the single inline comment covers both affected tests. No additional product-code defect was confirmed.
The extraction split is clear, the per-AAR stamp invalidation addresses failed-replacement recovery, and retaining content stamps for resource-free AARs addresses stale resource generation. XA0150 also has localized messages, documentation, and coverage for legacy-to-modern transitions.
CI and validation: All 40 reported GitHub checks succeeded, including the dotnet-android checks for Azure build 1679311. Direct Azure build/timeline verification was blocked by the CLI requiring authentication, so that status was not independently confirmed. An attempted isolated MSBuild check was blocked by execution approval; no local tests ran. Review findings are based on the diff, full changed-file context, and related callers/helpers.
Generated by Android PR Reviewer for #13510 · copilot · gpt61 · 106.5 AIC · ⌖ 3.88 AIC · ⊞ 20.2K
Comment /review to run again
`StringAssertEx.ContainsText()` returns a `bool` and never asserts, so `AarResourcesRemovedRegeneratesResources` and `AarIsExtractedAgainAfterFailedExtraction` did not verify that `R.txt` contained the `.aar` resource. Assert the result. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully!
|
|
Looking back, this change does not seem beneficial enough to justify the added complexity. I'm closing this to reduce distractions. We can revisit it later. |
There was a problem hiding this comment.
⚠️ Needs Changes
Findings: 1 ❌ error, 0
The extraction split is well scoped, and the per-AAR stamp invalidation plus regression tests address failed-extraction recovery and removal of all resources. One incremental correctness gap remains: cleaning up a native-only legacy DLL can leave the downstream imports cache referencing deleted native libraries. The inline finding describes the input sequence and cache-invalidation chain. I also suggested automated coverage for the environment-file cache-hit fix.
CI: All 40 GitHub checks report success, including the aggregate dotnet-android check for Azure build 1679764. Direct Azure build/timeline verification was blocked because az devops invoke requires authentication in this environment. Local reproduction was not completed because execution permissions denied fixture generation; the correctness finding is based on tracing the task outputs, SaveIfChanged, target inputs, and downstream cache consumers.
Generated by Android PR Reviewer for #13510 · copilot · gpt61 · 128.7 AIC · ⌖ 5.02 AIC · ⊞ 21K
Comment /review to run again
| // Left over from a previous version of this assembly; keeping it would resolve stale content | ||
| // and log XA0150 when the hash matches the stamp on the next build. | ||
| Log.LogDebugMessage ($"Deleting '{outDirForDll}': '{assemblyPath}' no longer contains legacy Android library content"); | ||
| Directory.Delete (outDirForDll, recursive: true); |
There was a problem hiding this comment.
🤖 ❌ MSBuild tasks — Invalidate the imported-library cache when deleting this directory. Consider a direct DLL reference containing only __AndroidNativeLibraries__.zip, then replace that DLL at the same path with one containing no Android content, without changing the project or AAR inputs. The JAR/resource/asset outputs are empty both times, and extractedDirectories still contains the identical entry added at line 168. Consequently libraryprojectimports.cache is unchanged and SaveIfChanged preserves its timestamp. _BuildLibraryImportsCache can then skip, leaving libraryimports.cache with NativeLibraries paths pointing into the directory just deleted here; _GetLibraryImports reintroduces those nonexistent files as AndroidNativeLibrary items.
Ensure this cleanup changes the cached extraction state or explicitly invalidates _BuildLibraryImportsCache. Please cover the native-only → no-content transition through both cache tasks/the incremental build pipeline; the current transition test includes JARs/resources, which change the first cache and hide this case.
Rule: Incremental-build correctness; invalidate downstream caches when removing outputs.
| </ItemGroup> | ||
| <ItemGroup Condition=" '@(_AndroidAarLibraryImport->Count())' != '0' "> | ||
| <LibraryEnvironments | ||
| Include="%(_AndroidAarLibraryImport._AndroidImportsDirectory).net/env/**" |
There was a problem hiding this comment.
🤖 💡 Testing — Could we add an automated regression for the .net/env/ cache-hit fix? The existing library-content test checks the environment value after the initial build, while the new incremental fixtures contain no environment entry. Extend that coverage with a timestamp-only AAR change that runs _AndroidExtractAarLibraries but skips re-extraction, then assert that the library environment value still reaches the generated runtime configuration. This would specifically catch the dropped-environment behavior removed from the old cache-hit branch.
Rule: Bug fixes need regression tests covering the affected incremental path.
Context: fcd7cf8 (#5131)
Android libraries can carry Android content (resources, assets, Java libraries, native libraries, ProGuard rules, environment files) in two formats:
__AndroidLibraryProjects__.zip,__AndroidNativeLibraries__.zip, and*.jarmanaged resources..aarfile next to the assembly instead.<ResolveLibraryProjectImports/>handled both formats in one ~520 line task. Every time the target ran, it hashed every referenced assembly and.aar. The.aarcache-hit path also duplicated the extraction path and drifted from it:.net/env/environment files were dropped whenever an.aarwas not re-extracted.Nothing in .NET for Android produces the classic format anymore, so split the two formats.
.aarextraction in MSBuildThe new
_AndroidExtractAarLibrariesand_AndroidExtractChangedAarLibrarytargets replace the C#.aarhandling with<GetFileHash/>,<Unzip/>, and per-.aarpartial builds:.aaris extracted tolp/[id]/jl/, where[id]is a hash of the.aarpath. Previously it waslp/[N], with[N]coming frommap.cache. As before,.jarfiles other thanclasses.jarandlibs/**/*.jarare not extracted, and neither are__MACOSXor.DS_Storeentries.lp/[id].stampholds the SHA256 of the.aarand is only rewritten when the content changes. Touching an unchanged.aarrehashes only that file, and no downstream targets run..aarfiles are extracted by a target batched per.aar. Each batch deletes that.aar's stamps first and only writes them back once<RemoveDir/>, both<Unzip/>passes and<Touch/>have succeeded. A failed extraction (an error, or a warning in design-time builds) is therefore retried by the next build. Without this, extracting A, failing to extract a corrupt replacement, and then restoring A would match A's stale hash and skip re-extraction..aarentries often carry fixed timestamps (1980).lp/aar.map([id]=[path]lines) letsJavaToolTaskkeep mappinglp/[id]paths in javac/d8/r8 errors back to their.aar. It is also an input of_UpdateAndroidResgenand_BuildLibraryImportsCache, so adding or removing an.aarregenerates resources..net/env/files..aaris part of@(_LibraryResourceDirectoryStamps). As a result, an update that removes all of an.aar's resources still runs_UpdateAndroidResgen, instead of leaving stale IDs inR.txt.NormalizeDirectory(), so they use native separators. On Windows, mixed separators prevented custom-view resources from matching their recompilation inputs.Classic assemblies: deprecated, still supported
<ResolveLibraryProjectImports/>now only handles classic assemblies. It stays enabled by default and emits the new warning XA0150 when it finds embedded Android content:When an updated assembly no longer contains classic content, its leftover
lp/[N]directory and stamp are deleted. Otherwise later builds would keep logging XA0150 for it and resolve stale jars and resources.This change also removes:
__library_projects__→lpdirectory migration;__AndroidEnvironment__cleanup branch;Behavior changes
.aarnow fails with MSB3933 from<Unzip/>instead of XA1004.xa1004.mdis updated accordingly.internal_impl-*.jarrename (only seen in old support-library.aarfiles) is gone..aaris removed, its assets remain in the.apkuntil_CreateBaseApkruns for another reason. This also happens onmain.Testing
Run on macOS against a locally built SDK (
make prepare && make leeroy):Xamarin.AndroidX.AppCompat(52.aarfiles) was built with and without this change. Both produced identical@(LibraryResourceDirectories)(same order), assets, ProGuard rules, jars and manifests. The.apkwas byte-identical: every entry CRC matched, includingresources.arsc..aarcontainingres/,assets/,proguard.txt,.net/env/andlint.jar, I exercised a no-op build, a touch-only change, a content change and removing the.aar. I also ran a design-time build with a corrupt.aar, which logged a warning and wrote no stamps; the next build retried the.aarand failed.ResolveLibraryProjectImportsTestscovers XA0150 on fresh and cached runs, the NuGet and plain-assembly variants, no warning for modern assemblies or for a legacy → modern transition, and theaar.maplookup.IncrementalBuildTest.AarResourcesRemovedRegeneratesResourcesandIncrementalBuildTest.AarIsExtractedAgainAfterFailedExtractioncover the incremental fixes../dotnet-local.sh test bin/TestDebug/net11.0/Xamarin.Android.Build.Tests.dll --filter …). This includesAarContentExtraction,DotNetBuildLibrary,BindingCheckHiddenFiles,IncrementalBuildTest.ResolveLibraryProjectImportsandAppProjectTargetsDoNotBreak.AarContentExtractionpassed.BuildReleaseArm64(apkdiffis not installed) and one wasBindLibraryWithMultipleGradleVersions(Gradle 8.7 does not run on JDK 23).