[androidsdk] Fix SDK extraction incrementality

[simonrozsival]

Summary

This fixes src/androidsdk/androidsdk.csproj being slow on repeated builds by making the SDK extraction targets genuinely incremental.

The important change is that after extracting each Android SDK/NDK archive, we now refresh the extracted source.properties file that MSBuild uses as the target output. That lets later builds see the extraction output as newer than the cached archive input and skip the expensive unzip/delete/recreate work.

What was happening before

_DownloadAndroidSdkPackages was not the main problem: the archives were already cached under $(AndroidToolchainCacheDirectory)/~/android-archives, and the shared DownloadFileWithRetry.targets stamp files meant the download/hash-validation work was normally skipped.

The slow part was _ExtractAndroidSdkPackages. It is batched per package and declares:

Inputs="$(AndroidToolchainCacheDirectory)\%(_AndroidSdkPackage.Identity)"
Outputs="%(_AndroidSdkPackage.Destination)\source.properties"

That looks correct at first: cached zip in, extracted source.properties out. The problem is that unzip preserves timestamps from the zip entries. Many Android SDK zips contain very old file mtimes, for example source.properties dated 2008, 2010, 2024, or 2025, while the cached zip file in ~/android-archives has the time it was downloaded/restored.

So after a successful extraction, MSBuild still saw:

cached zip input          newer
extracted source.properties output  older

That made _ExtractAndroidSdkPackages out-of-date on every build. Each repeated build deleted the existing SDK/NDK destination, recreated a staging directory, and unzipped/moved the package contents again. On my machine this showed up as _ExtractAndroidSdkPackages taking about 205s on an otherwise repeated build.

Why this works now

After extraction completes, the target verifies that the expected source.properties exists and then touches it:

<Touch Files="%(_AndroidSdkPackage.Destination)\source.properties" />

That does not modify package contents semantically; it only updates the incremental-build sentinel that this target already chose as its output. On the next build, the output timestamp is newer than the cached zip input, so MSBuild can correctly skip the batch instead of re-extracting the package.

Making extraction actually skippable exposed two related incremental-build issues, both fixed here:

1. On Unix, the target enumerated the temporary .staging directory while MSBuild was doing skipped-target output inference. Once extraction is skipped, that staging directory correctly does not exist, so the target now guards the staging directory enumeration and staged-file glob.
2. _GenerateAndroidPackageXmls declared package.xml outputs for every SDK package, but only packages with GeneratePackageXml=true synthesize one. For packages that never produce package.xml, the target was permanently missing outputs and ran every build. It now builds a filtered _GeneratedPackageXml item list and makes the incremental Inputs/Outputs apply only to those packages.

Validation

Ran repeated builds of the standalone project:

dotnet build src/androidsdk/androidsdk.csproj -c Debug -nologo -v:diag

After the one expected re-extraction to refresh sentinels, the steady-state diagnostic build shows:

Skipping target "_ExtractAndroidSdkPackages" because all output files are up-to-date with respect to the input files.
Skipping target "_GenerateAndroidPackageXmls" because all output files are up-to-date with respect to the input files.
Skipping target "_AcceptAndroidSdkLicenses" because all output files are up-to-date with respect to the input files.

_ExtractAndroidSdkPackages      4 ms / 11 calls
_GenerateAndroidPackageXmls     1 ms / 1 call
Build succeeded.
Time Elapsed 00:00:01.41

[Copilot]

Pull request overview

This PR improves incremental behavior in the androidsdk build targets so repeated builds correctly skip SDK extraction and related steps when nothing changed, avoiding unnecessary work and fixing cases where preserved mtimes prevented MSBuild’s Inputs/Outputs logic from considering packages up-to-date.

Changes:

• Refreshes extracted source.properties timestamps after unzip to ensure MSBuild incremental outputs behave correctly on subsequent builds.
• Adds guards to Unix staging directory inference so item globbing doesn’t misbehave when extraction is skipped/doesn’t produce expected staging content.
• Makes package.xml generation incremental only for SDK packages that actually synthesize package.xml (via GeneratePackageXml=true metadata).

Show a summary per file

File	Description
src/androidsdk/androidsdk.targets	Fixes _ExtractAndroidSdkPackages sentinel freshness, hardens Unix staging inference, and scopes incremental package.xml generation to only the packages that generate it.

Copilot's findings

• Files reviewed: 1/1 changed files
• Comments generated: 0

[jonathanpeppers]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

🤖 Android PR Reviewer

Verdict: ✅ Looks good — a well-reasoned, well-documented incremental-build fix. Two optional 💡 nits inline; nothing blocking.

What the PR does

• Touches the extracted source.properties after each package extraction so MSBuild's Outputs sentinel is newer than the cached-zip Inputs. unzip preserves old archive mtimes, which previously left the output older than the input and re-extracted every package on every build.
• Adds a fail-fast <Error> if extraction didn't produce source.properties.
• Guards the .staging enumeration (Exists(...)) and the staged-file glob (_GlobRoot != '') so output inference of the now-skippable target can't enumerate a missing directory or glob from filesystem root.
• Splits package.xml item filtering into _AddGeneratedPackageXmls, so _GenerateAndroidPackageXmls declares Inputs/Outputs only for packages that actually emit a package.xml — fixing a target that ran every build.

Verification notes

• ✅ Traced the <Touch> against both downstream consumers of source.properties mtime — _GenerateAndroidPackageXmls and _CopyNdkRedistributables (Inputs="$(AndroidNdkDirectory)\source.properties"). Both re-run once after the initial re-extraction, then settle to skipped. No re-trigger loop.
• ✅ The new <Error> only hardens an assumption the existing Outputs already made (every package emits source.properties), so it won't introduce spurious failures for the current package set; it converts a silent "always rebuild" into a loud failure if that ever breaks.
• ✅ The '$(_GlobRoot)' != '' guard correctly prevents the catastrophic Include="\**\*" root-glob when _StagedTopDir is empty during output inference.
• ✅ _GeneratedPackageXml is always non-empty (the active emulator variant has GeneratePackageXml=true), so the refactored Inputs/Outputs stay well-formed.

Suggestions (optional)

1. 💡 Reuse the already-computed $(_DestDir) in the new <Touch>/<Error> for consistency with the rest of the body.
2. 💡 _AddGeneratedPackageXmls could be a DependsOnTargets of its single consumer rather than a BeforeTargets, matching the repo convention.

CI

The Azure DevOps build (Linux / Windows / Mac) was still in progress at review time — please confirm it goes green before merge.

Generated by Android PR Reviewer for issue #11756 · 628.2 AIC · ⌖ 46.1 AIC · ⊞ 40K
_{Comment /review to run again}

[jonathanpeppers]

Addressed both suggestions in 7f44b6c:

1. Reused $(_DestDir) in the new <Error> and <Touch> instead of %(_AndroidSdkPackage.Destination), matching the rest of the target body.
2. Switched _AddGeneratedPackageXmls to a DependsOnTargets of _GenerateAndroidPackageXmls instead of BeforeTargets, matching repo convention.