[NativeAOT] Make workload linker the default for NativeAOT builds#11340
Merged
Conversation
Change _AndroidUseWorkloadNativeLinker default from false to true so NativeAOT builds use the workload-provided linker and sysroot by default, removing the NDK requirement. Update SetRuntime(NativeAOT) to set _SkipNdkResolution=true so all existing NativeAOT tests exercise the NDK-free workload linker path. Update NativeAotBuildTests with three tests covering the full matrix: - BuildNativeAot_WithoutNdk: workload linker (default), no NDK -> succeeds - BuildNativeAot_WithNdkLinker: opt-out to NDK linker, with NDK -> succeeds - BuildNativeAot_WithoutNdk_WorkloadLinkerDisabled_Fails: opt-out, no NDK -> fails Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: Claude:claude-opus-4.6-1m
Contributor
There was a problem hiding this comment.
Pull request overview
This PR switches NativeAOT builds to use the workload-provided linker/sysroot by default (removing the NDK requirement for the default path) and adjusts the test suite to validate both the default workload-linker behavior and the opt-out NDK-linker behavior.
Changes:
- Default
_AndroidUseWorkloadNativeLinkertotruefor NativeAOT builds. - Update the NativeAOT test runtime helper to skip NDK resolution by default for NativeAOT test projects.
- Revise/expand
NativeAotBuildTeststo cover success/failure across the workload-linker vs NDK-linker matrix.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/ProjectExtensions.cs | Changes NativeAOT test runtime setup to force the NDK-free path. |
| src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/NativeAotBuildTests.cs | Updates/extends NativeAOT build tests for the new default and opt-out behavior. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets | Switches the default NativeAOT linker selection toward workload-provided tooling. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 2
The default was set inside _AndroidLinkNativeAotSharedLibrary, but SetIlcToolchainPath in _AndroidBeforeIlcCompile runs earlier and checks the property. With the default set too late, ILC would use the NDK bin dir instead of AndroidBinUtilsDirectory when the property was not explicitly set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: Claude:claude-opus-4.6-1m
Update _NdkRequired in GetAndroidDependencies to only require NDK for PublishAot when _AndroidUseWorkloadNativeLinker is not enabled. Update NativeAotRequiresNdk test to explicitly opt out of the workload linker so it continues to verify NDK is required on the old path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: Claude:claude-opus-4.6-1m
Skip EnableLLVM and AndroidEnableProfiledAot for NativeAOT (Mono concepts not applicable). AotAssemblies+NativeAOT continues to run and correctly asserts NDK is not required. Fix NativeAotRequiresNdk to override _SkipNdkResolution=false so NDK resolution runs and the ndk-bundle dependency can be detected when testing the opt-out path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Assisted-by: Claude:claude-opus-4.6-1m
Member
|
/review |
|
✅ Android PR Reviewer completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ LGTM
Clean, well-structured PR. The default change, targets updates, and test reorganization are all logically consistent.
Summary of changes:
_AndroidUseWorkloadNativeLinkerdefault moved from inside_AndroidLinkNativeAotSharedLibrary(too late) to the top-levelPropertyGroup(correctly early) and flipped fromfalse→true_NdkRequiredcondition inGetAndroidDependenciesnow respects the workload linker flagSetRuntime(NativeAOT)helper sets_SkipNdkResolution=trueso all NativeAOT tests exercise the NDK-free path by default, with tests that need NDK explicitly opting back in- Tests cover all three meaningful configurations: workload linker (default), NDK linker, and failure case
Issue counts: 0 ❌ · 0
What's good:
- Moving the property default earlier correctly ensures
SetIlcToolchainPathand_NdkRequiredboth see the right value - The previous reviewer's concern about the late default was addressed
- Test matrix is clear and complete with descriptive names
- The
GetDependencyNdkRequiredConditionsupdates correctly handle the NativeAOT + EnableLLVM combination
Generated by Android PR Reviewer for issue #11340 · ● 3.7M
Comment on lines
+47
to
+61
| [Test] | ||
| public void BuildNativeAot_WithoutNdk_WorkloadLinkerDisabled_Fails () | ||
| { | ||
| var proj = new XamarinAndroidApplicationProject { | ||
| IsRelease = true, | ||
| }; | ||
| proj.SetRuntime (AndroidRuntime.NativeAOT); | ||
|
|
||
| using var builder = CreateApkBuilder (); | ||
| builder.ThrowOnBuildFailure = false; | ||
| Assert.IsFalse ( | ||
| builder.Build (proj, parameters: new [] { | ||
| "_AndroidUseWorkloadNativeLinker=false", | ||
| }), | ||
| "Build should succeed with workload linker and no NDK." | ||
| "Build should fail without NDK when workload linker is disabled." |
There was a problem hiding this comment.
🤖 💡 Testing — This test only asserts that the build fails, but doesn't verify why it fails. If a future regression causes the build to fail for an unrelated reason (e.g. a missing runtime pack), this test would still pass, masking the real issue.
Consider asserting on a specific error message or error code in the build output, e.g.:
Assert.IsFalse (
builder.Build (proj, parameters: new [] {
"_AndroidUseWorkloadNativeLinker=false",
}),
"Build should fail without NDK when workload linker is disabled."
);
StringAssertEx.Contains ("expected-error-string", builder.LastBuildOutput, "Expected NDK-related error.");Rule: Test assertions must be specific
| IsRelease = true, | ||
| }; | ||
| proj.SetRuntime (AndroidRuntime.NativeAOT); | ||
| proj.SetProperty ("_SkipNdkResolution", "false"); |
There was a problem hiding this comment.
🤖 💡 Testing — SetRuntime(NativeAOT) now sets _SkipNdkResolution=true, but this test immediately overrides it to false. This is correct and necessary — the test exercises the NDK linker path, which requires real NDK resolution. Just noting that this test will fail on CI agents without an NDK installed, which appears to be fine for this repo's CI setup.
Rule: Deterministic test data
jonathanpeppers
approved these changes
May 15, 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.
Change _AndroidUseWorkloadNativeLinker default from false to true so NativeAOT builds use the workload-provided linker and sysroot by default, removing the NDK requirement.
Update SetRuntime(NativeAOT) to set _SkipNdkResolution=true so all existing NativeAOT tests exercise the NDK-free workload linker path.
Update NativeAotBuildTests with three tests covering the full matrix: