Repository navigation
[Microsoft.Android.Build.Tasks] Add typemap rule generators - #12822
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The generator mishandles some failure diagnostics and does not consistently follow the task error-state contract.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Adds shared typemap key utilities and ProGuard generator tasks for later runtime-specific adapters.
Changes:
- Generates sorted class-only and member-preserving ProGuard rules.
- Adds canonical key helpers and localized XA4327/XA4328 diagnostics.
- Adds production-task references, tests, and documentation.
| File | Description |
|---|---|
src/Microsoft.Android.Build.Tasks/Tasks/GenerateTypeMapProguardConfiguration.cs |
Core rule generator |
src/Microsoft.Android.Build.Tasks/Tasks/GenerateTypeMapMemberProguardConfiguration.cs |
Member rule variant |
src/Microsoft.Android.Build.Tasks/Utilities/TypeMapClassName.cs |
JNI class-name extraction |
src/Microsoft.Android.Build.Tasks/Utilities/TypeMapKey.cs |
Alias normalization |
src/Xamarin.Android.Build.Tasks/Properties/Resources.resx |
New diagnostics |
src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs |
Generated resource accessors |
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/TypeMapTaskBuildEngine.cs |
Minimal test build engine |
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj |
Production task reference |
tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Generator/TypeMapProguardTests.cs |
Generator tests |
src/Microsoft.Android.Build.Tasks/Tests/Microsoft.Android.Build.Tasks.Tests/GenerateTypeMapProguardConfigurationTests.cs |
NUnit coverage |
Documentation/docs-mobile/messages/xa4327.md |
XA4327 documentation |
Documentation/docs-mobile/messages/xa4328.md |
XA4328 documentation |
Documentation/docs-mobile/messages/index.md |
Message index entries |
Documentation/docs-mobile/TOC.yml |
Documentation navigation |
Files not reviewed (1)
- src/Xamarin.Android.Build.Tasks/Properties/Resources.Designer.cs: Generated file
simonrozsival
added this pull request to stack #12830
September 18, 2026 19:14
Establish the shared retained-key contract before adding the runtime extractors and R8 pipeline wiring. Layer 1 of 6 splitting #12821 uses the frozen source at 87bd3f8. Add canonical JNI key helpers and deterministic UTF-8/LF generators for class-only keep rules and all-member rules scoped to retained classes. Reject invalid records before overwriting existing output. Add shared XA4327/XA4328 diagnostics without retiring the active legacy diagnostics, plus isolated production-task unit coverage. No build pipeline behavior changes; activation comes in later layers. Co-authored-by: Copilot App <[email protected]>
Give each parameterized case an isolated directory so sanitized test names do not collide on Windows. Co-authored-by: Copilot App <[email protected]>
Emit interface roots and interface member rules alongside class rules so R8 preserves retained managed interface metadata and proxy selection. Co-authored-by: Copilot App <[email protected]>
simonrozsival
force-pushed
the
simonrozsival-typemap-proguard-foundation
branch
from
September 21, 2026 15:42
b9f98b3 to
39eed16
Compare
Member
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Handle invalid paths through XA4328, include the output path when inputs are missing, and consistently return the AndroidTask error state. Keep parallel invalid-record test data under the fixture cleanup directory and add regression coverage. Co-authored-by: Copilot App <[email protected]>
Give each invalid-record case a stable unique NUnit name so BaseTest setup and teardown use the same isolated directory during parallel execution. Remove the GUID child directory that was still deleted through a shared sanitized parent. Co-authored-by: Copilot App <[email protected]>
Member
Author
|
/review |
Contributor
|
✅ Android PR Reviewer completed successfully!
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
⚠️ Needs Changes
Findings: 0 errors · 0 warnings · 1 suggestion
The generator is deterministic, validates all inputs before replacing the output, uses localized XA4328 diagnostics, and covers invalid input/path cases well. I left one non-blocking testing suggestion: validate the emitted class/interface directives with R8 rather than only comparing generated strings.
CI: Azure DevOps build 1606094 is still in progress. At review time, all 4 completed checks had succeeded (including license/cla), 4 checks were running, and the aggregate dotnet-android check was queued; no failures were reported. The review remains pending until required CI completes.
Generated by Android PR Reviewer for #12822 · copilot · gpt56 · 103.5 AIC · ⌖ 9.74 AIC · ⊞ 17.6K
Comment /review to run again
|
|
||
| protected virtual void WriteClassRule (TextWriter writer, string name) | ||
| { | ||
| writer.WriteLine ($"-keep class {name}"); |
Contributor
There was a problem hiding this comment.
🤖 💡 Testing — Please add a focused consumer-level test that feeds the generated class and interface rules to R8 (or point to the later layer that does so). The current tests only compare strings produced by this method, so they cannot catch a directive that is well-formed text but rejected by R8 or has different keep semantics—especially the newly added interface forms.
Rule: Verify generated configuration with the real consumer.
Resolve diagnostic resource and documentation conflicts by retaining main's removal of XA4325/XA4326 and preserving the XA4327/XA4328 typemap generator diagnostics. Co-authored-by: Copilot App <[email protected]>
simonrozsival
added
the
ready-to-review
jonathanpeppers
approved these changes
Sep 22, 2026
simonrozsival
added a commit
that referenced
this pull request
Sep 24, 2026
## Why Layer 1 of 4 in the remaining #12821 stack, based on main. The foundation #12822 has merged; the LLVM adapter #12823 is closed and omitted. Typemap-derived ProGuard rules need the Java class keys that survive ILLink, rather than pre-link registration metadata or stale alias arrays. ## Changes Add `ExtractTypeMapKeysFromAssemblies` in the modern `Microsoft.Android.Build.Tasks` assembly. It reads surviving assembly-level generic `TypeMapAttribute` entries, unions and sorts their keys, normalizes alias suffixes and object-array descriptors, and writes canonical UTF-8 output. Empty linked stubs are valid; absent inputs, malformed assemblies/attributes/keys, and output failures report XA4327. Source-link the existing metadata helpers and enable unsafe blocks in the modern task project. Add the pinned ILLink package and its path metadata to the standalone test project, retaining its production-task `ProjectReference`. This layer does not activate the task in MSBuild or change R8 policy. Native-object extraction, orchestration, and legacy cleanup remain in later layers. The copied extractor and fixture exactly match frozen source `87bd3f8a8b512859339e5446c1f86709a6c28c3b`; the test-project native-object metadata is intentionally deferred to layer 4. ## Validation On .NET SDK `11.0.100-preview.7.26381.103`, the following focused standalone run passed **89/89**, with no skips, using the actual modern production task DLL. This includes `RealILLinkRetainsOnlyLiveTypeMapAttributes` with pinned `Microsoft.NET.ILLink.Tasks` `11.0.0-rc.2.26461.115`. ```sh dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj -v minimal --filter 'FullyQualifiedName~ExtractTypeMapKeysFromAssembliesTests|FullyQualifiedName~ExtractTypeMapKeysFromLlvmIrArrayTests|FullyQualifiedName~TypeMapProguardTests' ``` Base: `main`. Bottom of native stack #12893.
This was referenced Oct 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Layer 1 of 6 splitting #12821; build activation comes in later layers.
Establish a shared retained-Java-key contract so the runtime-specific extractors can feed the same ProGuard generators without duplicating rule generation. This is a focused slice of the validated source at
87bd3f8a8b512859339e5446c1f86709a6c28c3b, based onmainatd63fe4c9f8cf8def6049c9415bb0abf4269ee0f5.Scope and contract
<GenerateTypeMapProguardConfiguration/>, which unions canonical keys and emits only-keep classrules. Reject invalid names/rule injection and missing inputs before overwriting output; valid empty inputs produce empty output.<GenerateTypeMapMemberProguardConfiguration/>, which keeps all members of retained classes. This is not precise-method inference.No extractors, target imports,
UsingTaskactivation, R8 policy changes, runtime changes, or DGML removal are included. The standaloneTypeMapProguardTestsfixture is intentionally a generator-only subset; later layers add its adapter/target/configuration coverage.Validation
dotnet test tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests/Microsoft.Android.Sdk.TrimmableTypeMap.Tests.csproj -v minimal --filter 'FullyQualifiedName~TypeMapProguardTests'15 passed, 0 failed, 0 skipped, compiling the actual modern production task project and its source-linked shared resources.
The focused NUnit command was also attempted. Its existing legacy
Xamarin.Android.Build.Tasksproject reference requires the missingbin/BuildDebug/net10.0/xa-prep-tasks.dll(GitBlame/GitCommitHash, MSB4062) in this clean worktree. The unchanged frozen-source fixture is retained; no full SDK/bootstrap build or test-project infrastructure changes were made.git diff --checkpasses. Complete extracted files match the frozen source byte-for-byte; retained standalone test/helper methods match the frozen source. The diff is additive only: 14 files, 546 insertions.