[Build] Move aapt2 download from xaprepare to MSBuild target#11399
Conversation
Remove Step_Get_Android_BuildTools from xaprepare and move the aapt2 download/extract logic into src/aapt2/aapt2.targets as a proper MSBuild target. The new _DownloadBuildTools target: - Downloads Google's repository2-3.xml manifest - Dynamically reads SHA-1 checksums via XmlPeek (no hardcoded hashes) - Downloads the 3 platform build-tools zips - Validates checksums and errors on mismatch - Uses Inputs/Outputs for incremental build skip This removes one more step from xaprepare, moving the logic to a standard MSBuild target that is easier to understand and maintain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refactor _DownloadBuildTools to use target batching via Outputs="...%(_BuildTools.Identity)" so the target body runs once per item. This replaces repeated DownloadFile/GetFileHash/Delete/Error calls with single instances that operate on the batched item. Also separate manifest download into its own target with a Condition to skip when all zips already exist. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
These prefix properties were only used by Step_Get_Android_BuildTools which has been deleted. The property was always empty and served no purpose for current build-tools versions. Removed from: - KnownProperties.cs - Properties.Defaults.cs.in - xaprepare.targets - Configuration.props - AndroidToolchain.cs (inlined empty prefix away) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2b8296c to
3a13b23
Compare
Remove the Condition on _DownloadAndroidManifest that checked whether zip files exist. When the zips are cached but _DownloadBuildTools Inputs/Outputs determines they are stale, the manifest target was being skipped (Exists=true) while _DownloadBuildTools still ran, causing XmlPeek to fail on the missing manifest file. The Inputs/Outputs on _DownloadBuildTools already handles the fully- cached case: if all zips are up-to-date, neither target runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use Inputs/Outputs on _DownloadAndroidManifest so it skips entirely when the manifest file exists and is newer than Configuration.props. This avoids any network requests on incremental builds, unlike SkipUnchangedFiles which still makes an HTTP round-trip. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MSBuild GetFileHash only supports SHA-256 and above, but Google SDK manifest (repository2-3.xml) only provides SHA-1. Switch to hardcoded SHA-256 hashes in Configuration.props, following the same pattern used by bundletool (XABundleToolHash). This simplifies the targets by removing the manifest download target and XmlPeek logic entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
Pull request overview
This PR removes the xaprepare step responsible for downloading Android build-tools archives and shifts the aapt2 acquisition flow into the MSBuild-driven src/aapt2/aapt2.targets, aligning more tool bootstrapping with standard MSBuild targets rather than custom xaprepare logic.
Changes:
- Add an MSBuild
_DownloadBuildToolstarget inaapt2.targetsto download build-tools zips and validate them via SHA-256 before extraction. - Remove the xaprepare
Step_Get_Android_BuildToolsstep and associated wiring/properties/prefix handling. - Add build-tools SHA-256 hash properties to
Configuration.propsfor download validation.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/aapt2/aapt2.targets | Adds MSBuild download + SHA-256 validation target and makes extraction depend on it. |
| Configuration.props | Removes unused build-tools URL prefix properties; adds per-OS build-tools SHA-256 hashes. |
| build-tools/xaprepare/xaprepare/xaprepare.targets | Removes generation-time substitutions for removed build-tools prefix properties. |
| build-tools/xaprepare/xaprepare/Steps/Step_Get_Android_BuildTools.cs | Deletes the xaprepare download step implementation. |
| build-tools/xaprepare/xaprepare/Scenarios/Scenario_Standard.cs | Removes the build-tools download step from the standard scenario. |
| build-tools/xaprepare/xaprepare/ConfigAndData/Dependencies/AndroidToolchain.cs | Removes use of the removed build-tools URL prefix property when defining the build-tools component. |
| build-tools/xaprepare/xaprepare/Application/Properties.Defaults.cs.in | Removes default property initialization for deleted build-tools prefix properties. |
| build-tools/xaprepare/xaprepare/Application/KnownProperties.cs | Removes known-property constants for deleted build-tools prefix properties. |
| build-tools/xaprepare/xaprepare/Application/Context.cs | Removes the unused BuildToolsArchiveDownloaded context property. |
There was a problem hiding this comment.
✅ LGTM — Clean migration from xaprepare to MSBuild
1 warning · 1 suggestion · 0 errors
Summary
This is a well-structured migration that replaces the custom Step_Get_Android_BuildTools xaprepare step with a standard MSBuild _DownloadBuildTools target. The new target follows established repo patterns (mirrors _DownloadBinutils in binutils.targets) and is an improvement over the old code — it adds SHA-256 hash verification that the xaprepare step lacked entirely.
The removal is thorough — all references to the deleted step and the now-unused XABuildToolsPackagePrefix* properties have been cleaned up across KnownProperties.cs, Properties.Defaults.cs.in, Context.cs, AndroidToolchain.cs, Scenario_Standard.cs, and xaprepare.targets.
Positive callouts
- 🔒 The delete-then-error pattern on hash mismatch prevents use of corrupt/tampered downloads
- ♻️ Proper
Inputs/Outputson_DownloadBuildToolsenables incremental skip when files are already cached - 🧹 Complete cleanup of vestigial
XABuildToolsPackagePrefix*properties (they were all empty strings)
Note on PR description
The description mentions "Dynamically reads SHA-1 checksums via XmlPeek (no hardcoded hashes)" — this appears to describe an earlier iteration. The actual implementation hardcodes SHA-256 hashes in Configuration.props, which is correctly documented in the code comment. Consider updating the description to match.
CI
dotnet-android ✅ passed. Xamarin.Android-PR not yet visible — may still be starting.
Generated by Android PR Reviewer for issue #11399 · ● 3.2M
The target lacked Inputs/Outputs so UnzipDirectoryChildren ran on every build. Use the cached zip as Input and the extracted aapt2 binary as Output so extraction is skipped when already up-to-date. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Remove
Step_Get_Android_BuildToolsfrom xaprepare and move the aapt2 download/extract logic intosrc/aapt2/aapt2.targetsas a proper MSBuild target.Changes
The new
_DownloadBuildToolstarget inaapt2.targets:Configuration.props(Google's manifest only provides SHA-1, which MSBuild'sGetFileHashdoesn't support)Outputs="...%(_BuildTools.Identity)") so each platform is processed independentlyInputs/Outputson both_DownloadBuildToolsand_ExtractAapt2for incremental build skipRemoved from xaprepare:
Step_Get_Android_BuildTools.csScenario_Standard.csBuildToolsArchiveDownloadedproperty fromContext.csCleaned up unused properties:
XABuildToolsPackagePrefixand per-platform variants (MacOS/Windows/Linux) fromConfiguration.props,KnownProperties.cs,Properties.Defaults.cs.in,xaprepare.targets, andAndroidToolchain.csContext
Part of the effort to eliminate xaprepare steps in favor of standard MSBuild targets. This makes the build logic easier to understand, debug, and maintain — no custom C# download infrastructure needed.