Repository navigation
[xabt] Recover missing AAPT2 keep rules from incremental caches - #12969
Conversation
Invalidate packaged resources only when a non-design-time application requires generated ProGuard rules and its merged AAPT2 rules are missing. Preserve the existing up-to-date rule registration and avoid repeated resource linking after repair. Cover deletion-only and Java-only R8 recovery, AAPT2-rooted Java classes and a layout constructor, no-op resource/Java/dex builds, and disabled-link/library/design-time exclusions. Co-authored-by: Copilot App <[email protected]>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The recovery logic and comprehensive regression coverage are sound; only a minor formatting suggestion remains.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Repairs missing AAPT2 keep rules in damaged incremental build caches by forcing resource relinking.
Changes:
- Invalidates packaged resources when expected keep rules are missing.
- Adds regression coverage for recovery and excluded configurations.
| File | Description |
|---|---|
Xamarin.Android.Common.targets |
Triggers AAPT2 relinking when keep rules are absent. |
IncrementalBuildTest.cs |
Tests rule regeneration, R8 retention, and subsequent incrementality. |
Include the upstream trimmable typemap legacy-scan fix from #12976 while preserving AAPT2 damaged-cache recovery and incremental keep-rule registration. Co-authored-by: Copilot App <[email protected]>
Address the review convention without changing the recovery condition or packaged-resource path. Co-authored-by: Copilot App <[email protected]>
|
@dalexsoto review |
dalexsoto
left a comment
dalexsoto
left a comment
There was a problem hiding this comment.
The recovery invalidates only packaged resources when expected AAPT2 rules are missing, then uses the existing link/configuration path to restore them. Although _CreateBaseApkInputs always runs, the !Exists('$(_Aapt2ProguardRules)') guard prevents deletion once repair has restored the rules, preserving settled-build incrementality and the existing surviving-rule registration. No current blocking issue remains.
Part of #12940.
Summary
Repair already-damaged incremental
objtrees whereaapt_rules.txtis missing but packaged resources are still up to date._CreateBaseApkInputsinvalidates only_PackagedResourceswhen a non-design-time Android application expects AAPT2-generated ProGuard rules and those rules are absent. The existing preparation and AAPT2-link targets then regenerate them.This is distinct from #12950, which preserves and re-registers surviving rules during incremental builds. Its late
_Aapt2ProguardRulescalculation and skipped-targetProguardConfiguration/FileWritesregistration remain intact. No extra collection target, new property, or broadobjcleanup is introduced.Regression coverage
The new Release/CoreCLR/R8/trimmable fixture embeds an unbound Java JAR, so generated keeps for managed peers or app-authored Java cannot accidentally root the tested classes.
_CreateBaseApkInputswith shrinking disabled,AndroidApplication=false, andDesignTimeBuild=true; packaged resources must remain untouched and rules must not be generated.On unchanged main
976e552, both damaged-cache cases fail because the rules remain missing. The Java-only case runs R8 without AAPT2 rules and strips both JAR classes while_CreateBaseApkremains skipped. The existing #12950 regression passes on main.Local validation
3 passed, 0 failed, 0 skipped in one focused invocation with the recovery enabled. Shipped task/target outputs and host test/configuration assemblies were rebuilt from this main-based branch using a private SDK/toolchain. No device was used.
Scope is limited to
Xamarin.Android.Common.targetsandIncrementalBuildTest.cs; no LLVM, native, binutils, bootstrap, or remapping changes or dependencies on other open PRs.