[Android] InvariantGlobalization support for CoreCLR Android builds #117542
+13
−1
Conversation
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
…ng env variables is sufficient
There was a problem hiding this comment.
Pull Request Overview
This PR enables passing the InvariantGlobalization MSBuild property through the Android build pipeline so that Android apps can opt into invariant globalization mode at runtime.
- Forwards the
InvariantGlobalizationproperty in test builds, Android build targets, and app tasks - Exposes a new
InvariantGlobalizationproperty onAndroidAppBuilderTaskand propagates it intoApkBuilder - Removes the exclusion of the invariant globalization tests so they run against Android CoreCLR
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/build.proj | Adds InvariantGlobalization to the test project invocation parameters |
| src/tasks/AndroidAppBuilder/ApkBuilder.cs | Injects DOTNET_SYSTEM_GLOBALIZATION_INVARIANT when InvariantGlobalization is true |
| src/tasks/AndroidAppBuilder/AndroidAppBuilder.cs | Declares InvariantGlobalization on the task and forwards it to ApkBuilder |
| src/mono/msbuild/android/build/AndroidBuild.targets | Passes the MSBuild InvariantGlobalization property to the Android build targets |
| src/libraries/tests.proj | Removes the exclusion for the invariant globalization tests |
Comments suppressed due to low confidence (2)
src/tasks/AndroidAppBuilder/AndroidAppBuilder.cs:149
- There are no tests verifying that
InvariantGlobalizationflows through the task to the APK and sets the correct environment variable. Consider adding a unit or integration test to validate this behavior.
apkBuilder.InvariantGlobalization = InvariantGlobalization;
src/tasks/AndroidAppBuilder/ApkBuilder.cs:427
- The
InvariantGlobalizationproperty is referenced here but theApkBuilderclass does not define it. Add apublic bool InvariantGlobalization { get; set; }property toApkBuilderto ensure this compiles.
if (InvariantGlobalization)
davidnguyen-tech
changed the title
davidnguyen-tech
changed the title
|
@dotnet-policy-service agree company="Microsoft" |
|
Tagging subscribers to 'arch-android': @vitek-karas, @simonrozsival, @steveisok, @akoeplinger |
vitek-karas
reviewed
Jul 11, 2025
| /// <summary> | ||
| /// Indicates whether we want to use invariant globalization mode. | ||
| /// </summary> | ||
| public bool InvariantGlobalization { get; set; } |
There was a problem hiding this comment.
Just curious - why not pass this through EnvironmentVariables?
Personally, I probably like this solution a little better, but I don't have many arguments either way.
This was referenced Jul 11, 2025
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.
Description
Forward the
InvariantGlobalizationMSBuild property to the CoreCLR runtime for Android applications by connecting the existing infrastructure.Related issues
Issue #117270