[xaprepare] Check in Ndk.projitems, remove generator#11733
Open
jonathanpeppers wants to merge 5 commits into
Open
[xaprepare] Check in Ndk.projitems, remove generator#11733jonathanpeppers wants to merge 5 commits into
jonathanpeppers wants to merge 5 commits into
Conversation
Mirrors PR #11608 which checked in Mono.Android.Apis.projitems. Convert `build-tools/scripts/Ndk.projitems.in` into a checked-in `build-tools/scripts/Ndk.projitems` with the `@NDK_*@` placeholders resolved from the current constants in BuildAndroidPlatforms.cs, and remove the `Get_Ndk_projitems` generator from xaprepare. - Point `Ndk.targets` at the new static, in-tree location. - Delete `Get_Ndk_projitems` and its dispatch entry in Step_GenerateFiles.cs. - The NDK constants in BuildAndroidPlatforms.cs are left intact; they are still consumed by Get_XABuildConfig_cs and the cmake presets generator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of hardcoding the NDK version (28c), pkg revision (28.2.13676358) and minimum API level (24) -- which duplicated values already defined in Configuration.props -- source them from $(_XAAndroidNdkRelease), $(_XAAndroidNdkPkgRevision) and $(AndroidMinimumDotNetApiLevel). Those are all set before Configuration.props imports Ndk.targets, so the projitems file no longer needs to be kept in sync on an NDK/API bump. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the xaprepare cleanup by checking in build-tools/scripts/Ndk.projitems as a static file and removing the Get_Ndk_projitems generator path from Step_GenerateFiles, so builds no longer regenerate a file whose contents only change on NDK bumps.
Changes:
- Check in a resolved
build-tools/scripts/Ndk.projitems(NDK 28c / pkg rev 28.2.13676358 / min API 24) and stop generating it at prepare time. - Update
build-tools/scripts/Ndk.targetsto import the in-tree projitems instead ofbin/Build$(Configuration)/.... - Remove the
Get_Ndk_projitemsgenerator method/dispatch and update related documentation references.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| build-tools/xaprepare/xaprepare/Steps/Step_GenerateFiles.cs | Removes the Ndk.projitems generation step from the xaprepare file-generation list. |
| build-tools/xaprepare/xaprepare/ConfigAndData/Configurables.cs | Updates doc comment to point at the checked-in projitems file. |
| build-tools/scripts/Ndk.targets | Imports Ndk.projitems from the scripts directory instead of from bin/Build$(Configuration). |
| build-tools/scripts/Ndk.projitems | Adds the resolved, checked-in projitems file that defines NDK version/pkg revision and ABI metadata. |
Comments suppressed due to low confidence (2)
build-tools/scripts/Ndk.targets:8
Ndk.projitemsis now checked in, so theExists(...)condition on the<Import>can hide a missing/incorrect checkout and lead to confusing downstream MSBuild errors (the items/properties would just not be defined). Consider removing the condition so the build fails fast if the file is missing.
<ProjitemsFile>$(MSBuildThisFileDirectory)Ndk.projitems</ProjitemsFile>
</PropertyGroup>
<Import
Project="$(ProjitemsFile)"
Condition="Exists('$(ProjitemsFile)')"/>
build-tools/scripts/Ndk.projitems:6
- The header comment says to update API-level values when changing
NdkMinimumAPI/NdkMinimumAPILegacy32inBuildAndroidPlatforms.cs, but those values are actually sourced from$(AndroidMinimumDotNetApiLevel)(Configuration.props). To avoid future mismatches, the comment should point at Configuration.props for API-level changes (and at BuildAndroidPlatforms.cs for NDK version/revision).
The legacy 32-bit minimum API level used to be lower than the .NET minimum API level, but they are identical now (both $(AndroidMinimumDotNetApiLevel)). Remove the two redundant per-ABI properties that nothing else references -- AndroidNdkApiLevel_Arm (armeabi-v7a .NET) and AndroidNdkApiLevel_X86_Legacy (x86 legacy) -- and point each item's ApiLevel/ApiLevelNET metadata at the single remaining per-ABI property. The arm64-v8a and x86_64 properties are left in place since shipped targets (Build.Tasks.targets, Common.props.in, NativeAOT.targets) still consume them by name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ApiLevelNET was the minimum API level for .NET (.NET 6+) Android, as opposed to the legacy Xamarin.Android (Mono) ApiLevel. Those values used to differ but are identical now, and the repo is .NET-only. The only consumer of the ApiLevelNET metadata was libunwind-xamarin.targets; point it at ApiLevel instead and drop the now-redundant ApiLevelNET metadata from all four AndroidSupportedTargetJitAbi items in Ndk.projitems. The resolved value (24) is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Mirrors #11608 (which checked in
Mono.Android.Apis.projitemsthe same way). See also #11568, #11580, #11613, #11631, #11657, #11658.What
build-tools/scripts/Ndk.projitems.inwas a template with@NDK_*@placeholders that theGet_Ndk_projitemsgenerator in xaprepare filled in from constants inBuildAndroidPlatforms.cs. Those substitution values only change on rare NDK bumps, so generating the file on every build adds no value.This PR checks in the resolved file as a static
build-tools/scripts/Ndk.projitemsand removes the generator.Changes
Add
build-tools/scripts/Ndk.projitems— the static, checked-in file. Rather than hardcoding values, it sources them from the existing properties inConfiguration.props(which is imported beforeNdk.targets):AndroidNdkVersion→$(_XAAndroidNdkRelease)AndroidNdkPkgRevision→$(_XAAndroidNdkPkgRevision)AndroidNdkApiLevel_*→$(AndroidMinimumDotNetApiLevel)So there is nothing to keep in sync on an NDK/API bump.
build-tools/scripts/Ndk.targets— pointProjitemsFileat the new in-tree location ($(MSBuildThisFileDirectory)Ndk.projitems) instead ofbin/Build$(Configuration)/Ndk.projitems.Delete
build-tools/scripts/Ndk.projitems.in.Step_GenerateFiles.cs— remove theGet_Ndk_projitemsmethod and its dispatch entry.Configurables.cs— update a doc comment reference fromNdk.projitems.in→Ndk.projitems.Simplifications (from review)
AndroidNdkApiLevel_Arm,AndroidNdkApiLevel_X86_Legacy). Thearm64-v8a/x86_64properties are kept because shipped targets (Build.Tasks.targets,Common.props.in,NativeAOT.targets) still consume them by name.ApiLevelNETitem metadata.ApiLevelNETwas the .NET (.NET 6+) Android minimum API level vs. the legacy Xamarin.Android/MonoApiLevel; the two are identical now and this repo is .NET-only. Its only consumer wassrc/native/common/libunwind/libunwind-xamarin.targets, which now readsApiLevel. DroppedApiLevelNETfrom all fourAndroidSupportedTargetJitAbiitems.The NDK constants in
BuildAndroidPlatforms.csare intentionally left untouched — they're still consumed byGet_XABuildConfig_csand the cmake presets generator.Verification
dotnet build build-tools/xaprepare/xaprepare/xaprepare.csproj -c Debug→ 0 errors / 0 warnings.Configuration.props); it produces all fourAndroidSupportedTargetJitAbiitems withApiLevel=24 and the expectedAndroidRIDmetadata, matching prior generator output (NDK28c/ pkg28.2.13676358).git grep Ndk.projitems.in,git grep Ndk_projitems,git grep ApiLevelNET, and the oldbin/Build$(Configuration)/Ndk.projitemspath all return 0 hits.