[NativeAOT] Add GenerateNativeAotBootstrapSources task#11255
[NativeAOT] Add GenerateNativeAotBootstrapSources task#11255jonathanpeppers merged 22 commits intomainfrom
Conversation
…pe map Parameterize Build_WithTrimmableTypeMap_Succeeds and Build_WithTrimmableTypeMap_IncrementalBuild to run with both CoreCLR and NativeAOT runtimes. NativeAOT uses Release configuration (required). CoreCLR + Release is skipped pending upstream fixes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Redesign _AndroidComputeIlcCompileInputs to populate IlcCompileInput and IlcReference from @(ResolvedFileToPublish) instead of hardcoding the $(IntermediateLinkDir) path. This matches the SDK contract in Microsoft.NETCore.Native.Publish.targets and works regardless of whether ILLink ran. When _AndroidTypeMapImplementation is 'trimmable': - Remove PrepareForILLink and ILLink from IlcCompileDependsOn - Skip _PrepareLinking dependency in _AndroidBeforeIlcCompile - Skip ILLink-specific settings (RunILLink, SuppressTrimAnalysisWarnings) - Skip _PreTrimmingFixLegacyDesignerUpdateItems in _AndroidRunNativeCompile Fixes #11182 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
_OriginalSuppressTrimAnalysisWarnings is only set in the ILLink-specific PropertyGroup (non-trimmable path). Without this condition, the restore in _AndroidComputeIlcCompileInputs would clobber SuppressTrimAnalysisWarnings with an empty value in the trimmable path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ILC input wiring change results in a 470 KB size reduction in the native binary (10.2%) because the SDK's _ComputeIlcCompileInputs provides a cleaner set of references for ILC trimming. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removing the blanket TrimMode=All rooting for ILC inputs means ILC trims more unused code and produces fewer AOT analysis warnings (120 -> 92). Use Assert.LessOrEqual so the test does not break if warnings decrease further. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The trimmable path skips _PrepareLinking (ILLink-specific), but _PrepareLinking indirectly triggered _CreatePropertiesCache which resolves AndroidBinUtilsDirectory via _ResolveSdks. Without this, CompileNativeAssembly fails in the inner per-RID build. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
NativeAOT + trimmable builds fail at Java compilation because JavaInteropRuntime.java and NativeAotEnvironmentVars.java are not generated by the trimmable typemap path. Tracked in #11210. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit b5eee26.
This reverts commit 71665b0.
The ILLink path needs TrimmerRootAssembly TrimMode=All for all ILLink-trimmed assemblies so ILC doesn't trim further. Without it, ILC removes JNI callback types that are invoked from Java at runtime, causing MemberAccessException crashes. The trimmable path doesn't need this because it doesn't run ILLink and ILC handles all trimming directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR #11091 fixed CoreCLR+Release trimmable typemap builds, so the Assert.Ignore is no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
NativeAOT + trimmable builds don't fully succeed yet (#11210), so the test parameterization is premature. Restore to main's version. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The SDK's ComputeIlcCompileInputs sets IlcCompileInput from @(IntermediateAssembly), which is the untransformed assembly. When Android runs ILLink before ILC, the ILLink output in $(IntermediateLinkDir) has the rewritten assemblies (e.g. FixLegacyResourceDesigner transformations). Override IlcCompileInput to use the ILLink output for the non-trimmable path. The trimmable path doesn't need this because ILLink doesn't run — ILC gets the untransformed assemblies directly from the SDK. Workaround for dotnet/runtime#127577 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract NativeAOT bootstrap file generation (JavaInteropRuntime.java, NativeAotEnvironmentVars.java) into GenerateNativeAotBootstrapFiles on GenerateAdditionalProviderSources. Both the legacy ILLink path and the new GenerateNativeAotBootstrapSources task call the shared method. Wire GenerateNativeAotBootstrapSources into the trimmable _GenerateJavaStubs target conditioned on NativeAOT runtime. Parameterize Build_WithTrimmableTypeMap_Succeeds to cover NativeAOT. Fixes #11210 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a dedicated MSBuild task to generate NativeAOT bootstrap Java sources in the trimmable typemap build path, reusing the existing generation logic so both the legacy ILLink path and the new trimmable path produce the required JavaInteropRuntime.java and NativeAotEnvironmentVars.java.
Changes:
- Introduces
GenerateNativeAotBootstrapSourcestask and wires it into the trimmable_GenerateJavaStubspath for NativeAOT. - Extracts NativeAOT bootstrap Java generation into a shared helper (
GenerateAdditionalProviderSources.GenerateNativeAotBootstrapFiles). - Expands
Build_WithTrimmableTypeMap_Succeedsto cover both Release/Debug and CoreCLR/NativeAOT configurations.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/TrimmableTypeMapBuildTests.cs | Parameterizes the trimmable typemap build test across runtime + configuration. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateNativeAotBootstrapSources.cs | New task that delegates bootstrap source generation to shared logic. |
| src/Xamarin.Android.Build.Tasks/Tasks/GenerateAdditionalProviderSources.cs | Refactors bootstrap generation into a shared static helper and makes OutputDirectory required. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.Trimmable.targets | Hooks bootstrap generation into the trimmable _GenerateJavaStubs target for NativeAOT. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets | Adjusts NativeAOT pipeline dependencies for trimmable vs non-trimmable paths. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 4
| <!-- Generate NativeAOT bootstrap Java sources (JavaInteropRuntime.java, NativeAotEnvironmentVars.java) --> | ||
| <GenerateNativeAotBootstrapSources | ||
| Condition=" '$(_AndroidRuntime)' == 'NativeAOT' " | ||
| OutputDirectory="$(IntermediateOutputPath)android" | ||
| TargetName="$(TargetName)" | ||
| Environments="@(_EnvironmentFiles)" | ||
| HttpClientHandlerType="$(AndroidHttpClientHandlerType)" | ||
| EnableSGenConcurrent="$(AndroidEnableSGenConcurrent)" /> |
There was a problem hiding this comment.
The new NativeAOT bootstrap generation is invoked from _GenerateJavaStubs, but this target’s Inputs/Outputs are currently based only on the TypeMap DLL. Changes to @(_EnvironmentFiles) / AndroidHttpClientHandlerType / AndroidEnableSGenConcurrent can leave JavaInteropRuntime.java / NativeAotEnvironmentVars.java stale because _GenerateJavaStubs will be skipped on incremental builds; consider adding those items/properties to the target Inputs (or moving this work into a dedicated incremental target with its own Inputs/Outputs).
There was a problem hiding this comment.
Added _EnvironmentFiles to inputs, but not the other properties because that's an existing gap and would require changes to more than just the trimmable typemap targets.
NativeAOT incremental variant is skipped — the NativeAOT inner build causes _GenerateJavaStubs to re-run on incremental builds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…nerate() The AppendEnvVarEntry and GenerateJavaSource local functions became unused after extracting NativeAOT bootstrap generation into the shared static GenerateNativeAotBootstrapFiles() method. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Match the existing instance-level GetResource pattern instead of inlining GetManifestResourceStream directly in GenerateJavaSource. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ilds The NativeAOT bootstrap sources (JavaInteropRuntime.java, NativeAotEnvironmentVars.java) depend on @(_EnvironmentFiles). Adding them to the target Inputs ensures the target re-runs when environment files change. Also fix missing Microsoft.Build.Framework using in test file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/review |
|
✅ Android PR Reviewer completed successfully! |
There was a problem hiding this comment.
🤖 Review Summary — ✅ LGTM (with minor suggestions)
Clean, well-structured refactoring that extracts NativeAOT bootstrap file generation into a reusable shared method and wires it into the trimmable build path via a new GenerateNativeAotBootstrapSources task.
What the PR does well
- Good separation of concerns: The trimmable vs. non-trimmable conditional split in
NativeAOT.targetsis clean and well-commented, with clear explanations of why ILLink is skipped in the trimmable path. - Correct incremental build handling: Adding
@(_EnvironmentFiles)to_GenerateJavaStubsInputs ensures environment file changes correctly invalidate the NativeAOT bootstrap sources. - Uses
Files.CopyIfStringChanged: The shared method preserves the existing pattern of avoiding unnecessary writes, which is critical for incremental build correctness. - Test expansion: Parameterizing the existing tests to cover NativeAOT + trimmable is a good addition, with proper use of
IgnoreUnsupportedConfigurationfor invalid combos. - Surgical
IlcReferenceupdate: Replacing the heavy-handed "remove all, add back" pattern with the targetedRemove="@(PrivateSdkAssemblies->'...')"is a nice improvement.
Issues found
| Severity | Count |
|---|---|
| 💡 Suggestion | 3 |
All three findings are minor style/consistency suggestions — no bugs, safety, or performance issues found. The targets logic, conditional splits, and the shared static method implementation are all correct.
Generated by Android PR Reviewer for issue #11255 · ● 6.6M
| /// Delegates to <see cref="GenerateAdditionalProviderSources.GenerateNativeAotBootstrapFiles"/> | ||
| /// for the actual generation logic. | ||
| /// </summary> | ||
| public class GenerateNativeAotBootstrapSources : AndroidTask |
There was a problem hiding this comment.
🤖 💡 Consistency — This PR seals GenerateAdditionalProviderSources (line 13 of that file), but the new GenerateNativeAotBootstrapSources is left unsealed. Consider adding sealed for consistency within this PR.
| public class GenerateNativeAotBootstrapSources : AndroidTask | |
| public sealed class GenerateNativeAotBootstrapSources : AndroidTask |
Rule: Code organization
| /// Generates JavaInteropRuntime.java and NativeAotEnvironmentVars.java for NativeAOT apps. | ||
| /// Shared between the legacy (ILLink) and trimmable build paths. | ||
| /// </summary> | ||
| public static void GenerateNativeAotBootstrapFiles ( |
There was a problem hiding this comment.
🤖 💡 Code organization — Both GenerateNativeAotBootstrapSources and GenerateAdditionalProviderSources are in the same assembly, so this shared helper could be internal static instead of public static. This follows the repo convention of defaulting new helpers to internal.
Rule: New helpers default to internal
| // Run second build with diagnostic verbosity to see why CoreCompile re-runs | ||
| builder.Verbosity = LoggerVerbosity.Diagnostic; |
There was a problem hiding this comment.
🤖 💡 Testing — Switching to LoggerVerbosity.Diagnostic significantly increases log output and may slow down the test. If this was added to investigate an incremental build issue and the investigation is complete, consider removing the verbose logging and the CoreCompile status print to keep the test fast and focused.
Rule: Testing (Postmortem #9)
- Seal GenerateNativeAotBootstrapSources for consistency - Make GenerateNativeAotBootstrapFiles internal static (not public) - Remove leftover diagnostic verbosity from incremental build test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AssemblyModifierPipeline unconditionally bumps assembly timestamps even when content is unchanged, causing CoreCompile to re-run on NativeAOT incremental builds. Skip the test until this is fixed. See #11265 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract NativeAOT bootstrap file generation (JavaInteropRuntime.java,
NativeAotEnvironmentVars.java) into GenerateNativeAotBootstrapFiles on
GenerateAdditionalProviderSources. Both the legacy ILLink path and the
new GenerateNativeAotBootstrapSources task call the shared method.
Wire GenerateNativeAotBootstrapSources into the trimmable
_GenerateJavaStubs target conditioned on NativeAOT runtime.
Parameterize Build_WithTrimmableTypeMap_Succeeds to cover NativeAOT.
Depends on (includes changes from) #11183.