Uh oh!
There was an error while loading. Please reload this page.
Determine Turkish casing from the locale name instead of collation - #132002
Open
MsfPablo wants to merge 3 commits into
Open
Determine Turkish casing from the locale name instead of collation#132002MsfPablo wants to merge 3 commits into
MsfPablo wants to merge 3 commits into
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
Tagging subscribers to 'arch-android': @vitek-karas, @simonrozsival, @steveisok, @akoeplinger |
tarekgh
commented
Aug 7, 2026
Member
/azp run runtime-extra-platforms |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
This was referenced Aug 7, 2026
Open
Member
Thank you @MsfPablo for looking into this. Not sure if you're able to acces the CI results at: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1544675&view=logs&jobId=c9e87f9f-b882-5f07-e8c2-f8cdb4f422a2&j=c9e87f9f-b882-5f07-e8c2-f8cdb4f422a2&t=f555a9c0-ef97-5d93-0768-acf0622a0bcb If not, here is the extract of the failing tests: |
MsfPablo
commented
Aug 11, 2026
Author
@dotnet-policy-service agree Thanks @matouskozak for the CI extract — I can see the same 8 failures land on Android, and the symptoms (Expected: A few things I want to nail down before I push a follow-up commit, ideally with @tarekgh's CI results replicated locally:
I'll re-examine the Sorry for the CI thrash — I should have at least traced the whole |
TextInfo.NeedsTurkishCasing detected the Turkish dotted/dotless i casing rules by probing whether the culture collates U+0131 equal to 'I' under IgnoreCase. On Android the system ICU does not provide the collation data that probe depends on, so the check returned false for tr-TR and casing silently fell back to the non-Turkish path, making i.ToUpper() yield 'I' instead of U+0130. Fixesdotnet#106560
These ToLower/ToUpper cases were skipped on Android and LinuxBionic because the collation-based Turkish detection did not work there. The casing path no longer depends on collation data, so they can run.
The previous commit routed IcuChangeCase through GlobalizationNative_* ChangeCaseTurkish for tr/az locales by deriving the language from the locale name. The follow-up CI run on Android still failed with the same 8 tests returning U+0069/U+0049 instead of U+0131/U+0130, which means IcuChangeCase was never reached at all on that platform. The culprit is PopulateIsAsciiCasingSameAsInvariant in TextInfo.cs: the same CompareInfo.Compare probe that NeedsTurkishCasing used to depend on is also used here to detect whether ASCII casing matches the invariant culture. On Android, where the system ICU's collation data is incomplete, the probe incorrectly returns True for tr-TR, so ChangeCaseCommon's ASCII fast path (line 262 / 321) calls Ascii.ToLower/ToUpper, which hardcodes the U+0049 -> U+0069 mapping and never invokes ChangeCaseCore. That matches the failure symptom exactly. Mirror the locale-name derivation already used by NeedsTurkishCasing: short-circuit tr/az to False directly in the populator, and only fall through to the CompareInfo probe for other cultures. Also drop the test guards for Android and LinuxBionic (already cherry-picked as a separate commit), so the regression is now exercised in CI on all platforms.
MsfPabloforce-pushed
the
turkish-casing-106560
branch
from
August 11, 2026 22:02
8979933 to
7a2a403CompareMsfPablo
commented
Aug 11, 2026
Author
Confirmed the hypothesis from point (1) in my last comment — the failing tests never reached The mechanism. Both overloads of The The fix (commit Also pushed ( Sorry it took a follow-up commit rather than getting it right the first time — I should have traced the whole |
matouskozak
commented
Aug 12, 2026
Member
@MsfPablo were you able to verify the fix locally? We run thousands of tests on Android CI and as such iterating on a solution using just the CI is inefficient. We have instructions on how to build runtime for Android in https://github.com/dotnet/runtime/blob/main/docs/workflow/building/coreclr/android.md. |
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Fixes#106560
Problem
On Android,
"i".ToUpper(new CultureInfo("tr-TR"))returnsI(U+0049) instead ofİ(U+0130), and"I".ToLower(...)returnsiinstead ofı. The same code is correct on Windows/Linux/macOS.Cause
TextInfo.NeedsTurkishCasingdoes not look at the locale at all — it infers the need for Turkish casing by probing the collation tailoring:Android uses the system ICU, which does not carry the
tr/azcollation tailoring that makes U+0131 compare equal toIunderIgnoreCase(the same underlying gap tracked by #60568). The probe therefore returnsfalsefortr-TR,IcuChangeCasetakes theChangeCasepath instead ofChangeCaseTurkish, and the dotted/dotlessimapping is silently lost.GlobalizationNative_ChangeCaseTurkishitself is fine and is already built for Android — it is simply never called.Fix
Decide from the locale's language subtag instead, which is what ICU itself does (
ustrcase_getCaseLocaleselectsUCASE_LOC_TURKISHfor languagestrandaz). This removes the dependency on collation data and makes the behavior identical on every ICU platform.This should be behavior-preserving elsewhere:
trandazare exactly the languages whose collation tailoring made the old probe returntrue, and the root collation does not equate U+0131 withI. The existingen-US-POSIXregression case — which exists precisely because the old check was collation-based — still passes, since its language subtag isen.Tests
The Turkish
ToUpper/ToLowercases inTextInfoTestswere skipped onAndroid/LinuxBionicwith the comment "Android has its own ICU, which doesn't work well with tr". Those guards are removed so the cases now run on those platforms and cover this fix. They exercisetr,tr-TR,azandaz-Latn-AZ.Verification
I want to be upfront: I have not built or run this locally. Building dotnet/runtime for Android and running the globalization suite on a device/emulator was not something I could do in my environment, so I am relying on CI (including the Android legs) to validate it. The change is small and self-contained, and the re-enabled test cases are the intended proof. Happy to iterate if CI shows anything unexpected — in particular if some locale I have not considered relied on the old collation-derived answer.
cc @matouskozak@tarekgh@ilonatommy