-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Initial changes for globalization hybrid mode #81470
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f8e72d8
a82743d
954ed0f
1362d45
cb23688
af32324
83f4e7b
4ac3e6b
60e090a
26ce9a9
a10f09d
dc347f5
09d599e
bacc227
e844a40
f7f730a
46c9e68
1b7aa37
789613b
0b9f32d
1aee1e0
b382101
1556d17
5ab240a
5028045
ad4c98b
3eb5cae
27424bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,7 @@ configurations but their defaults might vary as any SDK can set the defaults dif | |
| EnableUnsafeBinaryFormatterSerialization | System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization | BinaryFormatter serialization support is trimmed when set to false | | ||
| EventSourceSupport | System.Diagnostics.Tracing.EventSource.IsSupported | Any EventSource related code or logic is trimmed when set to false | | ||
| InvariantGlobalization | System.Globalization.Invariant | All globalization specific code and data is trimmed when set to true | | ||
| HybridGlobalization | System.Globalization.Hybrid | Loading ICU + native implementations | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we still planning to add the feature switch So for HybridGlobalization, is there anything we want to/can trim out if its known that HybridGlobalization is being used? From what I am understanding, HybridGlobalization is to help reduce the reliance on ICU. When its true, it doesn't make any other code completely unreachable right now does it? Or is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to have feature switch HybridGlobalization as without it we will not understand to load fully icu or no. In ideal case IcuGetLocaleInfo should not be used in HybridGlobalization when all functionalities will be implemented by native functions. Also for HybridGlobalization we will load smaller icu files (trimmed out the functionalities that are implemented by native functions), this will be done in upcoming PRs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mkhamoyan the feature switches listed here are linker feature switches, i.e. where the linker replaces/hardcodes the value of the corresponding AppContext switch so that unused code can be trimmed out. We don't need that, we have a regular AppContext switch that is unaffected by trimming so having it listed in this doc is confusing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will remove it from here. Do we have doc for regular AppContext switches? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As far as I know we just have the public docs where we could add it: https://learn.microsoft.com/en-us/dotnet/core/runtime-config/globalization Note that in our case we'd just have runtimeconfig.json and environment variable options since the MSBuild property would need wiring up in the dotnet/sdk. |
||
| PredefinedCulturesOnly | System.Globalization.PredefinedCulturesOnly | Don't allow creating a culture for which the platform does not have data | | ||
| UseSystemResourceKeys | System.Resources.UseSystemResourceKeys | Any localizable resources for system assemblies is trimmed when set to true | | ||
| HttpActivityPropagationSupport | System.Net.Http.EnableActivityPropagation | Any dependency related to diagnostics support for System.Net.Http is trimmed when set to false | | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Runtime.InteropServices; | ||
|
||
internal static partial class Interop | ||
{ | ||
internal static partial class Globalization | ||
{ | ||
[LibraryImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_GetLocaleNameNative", StringMarshalling = StringMarshalling.Utf8)] | ||
internal static unsafe partial string GetLocaleNameNative(string localeName); | ||
|
||
[LibraryImport(Libraries.GlobalizationNative, EntryPoint = "GlobalizationNative_GetLocaleInfoStringNative", StringMarshalling = StringMarshalling.Utf8)] | ||
internal static unsafe partial string GetLocaleInfoStringNative(string localeName, uint localeStringData); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<PropertyGroup> | ||
<TargetFrameworks>$(NetCoreAppCurrent)-ios;$(NetCoreAppCurrent)-tvos;$(NetCoreAppCurrent)-maccatalyst;$(NetCoreAppCurrent)-osx</TargetFrameworks> | ||
<TestRuntime>true</TestRuntime> | ||
<HybridGlobalization>true</HybridGlobalization> | ||
</PropertyGroup> | ||
<ItemGroup> | ||
<Compile Include="HybridMode.cs" /> | ||
</ItemGroup> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Collections.Generic; | ||
using Xunit; | ||
|
||
namespace System.Globalization.Tests | ||
{ | ||
public class HybridModeTests | ||
{ | ||
public static IEnumerable<object[]> EnglishName_TestData() | ||
{ | ||
yield return new object[] { "en-US", "English (United States)" }; | ||
yield return new object[] { "fr-FR", "French (France)" }; | ||
} | ||
|
||
public static IEnumerable<object[]> NativeName_TestData() | ||
{ | ||
yield return new object[] { "en-US", "English (United States)" }; | ||
yield return new object[] { "fr-FR", "français (France)" }; | ||
yield return new object[] { "en-CA", "English (Canada)" }; | ||
} | ||
|
||
[Theory] | ||
[MemberData(nameof(EnglishName_TestData))] | ||
public void TestEnglishName(string cultureName, string expected) | ||
{ | ||
CultureInfo myTestCulture = new CultureInfo(cultureName); | ||
Assert.Equal(expected, myTestCulture.EnglishName); | ||
} | ||
|
||
[Theory] | ||
[MemberData(nameof(NativeName_TestData))] | ||
public void TestNativeName(string cultureName, string expected) | ||
{ | ||
CultureInfo myTestCulture = new CultureInfo(cultureName); | ||
Assert.Equal(expected, myTestCulture.NativeName); | ||
} | ||
|
||
[Theory] | ||
[InlineData("de-DE", "de")] | ||
[InlineData("en-US", "en")] | ||
public void TwoLetterISOLanguageName(string name, string expected) | ||
{ | ||
Assert.Equal(expected, new CultureInfo(name).TwoLetterISOLanguageName); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
using System.Diagnostics; | ||
|
||
namespace System.Globalization | ||
{ | ||
internal sealed partial class CultureData | ||
{ | ||
/// <summary> | ||
/// This method uses the sRealName field (which is initialized by the constructor before this is called) to | ||
/// initialize the rest of the state of CultureData based on the underlying OS globalization library. | ||
/// </summary> | ||
private bool InitNativeCultureDataCore() | ||
{ | ||
Debug.Assert(_sRealName != null); | ||
Debug.Assert(!GlobalizationMode.Invariant); | ||
string realNameBuffer = _sRealName; | ||
|
||
_sWindowsName = GetLocaleNameNative(realNameBuffer); | ||
return true; | ||
} | ||
|
||
internal static unsafe string GetLocaleNameNative(string localeName) | ||
{ | ||
return Interop.Globalization.GetLocaleNameNative(localeName); | ||
} | ||
|
||
private string GetLocaleInfoNative(LocaleStringData type) | ||
{ | ||
Debug.Assert(!GlobalizationMode.Invariant); | ||
Debug.Assert(_sWindowsName != null, "[CultureData.GetLocaleInfoNative] Expected _sWindowsName to be populated already"); | ||
|
||
return GetLocaleInfoNative(_sWindowsName, type); | ||
mdh1418 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// For LOCALE_SPARENT we need the option of using the "real" name (forcing neutral names) instead of the | ||
// "windows" name, which can be specific for downlevel (< windows 7) os's. | ||
private static unsafe string GetLocaleInfoNative(string localeName, LocaleStringData type) | ||
{ | ||
Debug.Assert(localeName != null, "[CultureData.GetLocaleInfoNative] Expected localeName to be not be null"); | ||
|
||
return Interop.Globalization.GetLocaleInfoStringNative(localeName, (uint)type); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -13,13 +13,19 @@ internal static partial class GlobalizationMode | |||||||||
private static partial class Settings | ||||||||||
{ | ||||||||||
internal static bool Invariant { get; } = AppContextConfigHelper.GetBooleanConfig("System.Globalization.Invariant", "DOTNET_SYSTEM_GLOBALIZATION_INVARIANT"); | ||||||||||
#if TARGET_OSX || TARGET_MACCATALYST || TARGET_IOS || TARGET_TVOS | ||||||||||
internal static bool Hybrid { get; } = AppContextConfigHelper.GetBooleanConfig("System.Globalization.Hybrid", "DOTNET_SYSTEM_GLOBALIZATION_HYBRID"); | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be under iOS ifdef - since all uses are under iOS ifdef as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, plus wasm and wasi as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now I will put under ioslike platforms. For wasm we have seperate PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If runtime/src/libraries/System.Runtime/tests/TrimmingTests/System.Runtime.TrimmingTests.proj Lines 16 to 19 in 9b38f2a
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once more functionalities will be implemented this will be used and we can have tests in TrimmingTests for hybrid mode. |
||||||||||
#endif | ||||||||||
internal static bool PredefinedCulturesOnly { get; } = AppContextConfigHelper.GetBooleanConfig("System.Globalization.PredefinedCulturesOnly", "DOTNET_SYSTEM_GLOBALIZATION_PREDEFINED_CULTURES_ONLY", GlobalizationMode.Invariant); | ||||||||||
} | ||||||||||
|
||||||||||
// Note: Invariant=true and Invariant=false are substituted at different levels in the ILLink.Substitutions file. | ||||||||||
// This allows for the whole Settings nested class to be trimmed when Invariant=true, and allows for the Settings | ||||||||||
// static cctor (on Unix) to be preserved when Invariant=false. | ||||||||||
internal static bool Invariant => Settings.Invariant; | ||||||||||
#if TARGET_OSX || TARGET_MACCATALYST || TARGET_IOS || TARGET_TVOS | ||||||||||
internal static bool Hybrid => Settings.Hybrid; | ||||||||||
#endif | ||||||||||
internal static bool PredefinedCulturesOnly => Settings.PredefinedCulturesOnly; | ||||||||||
|
||||||||||
private static bool TryGetAppLocalIcuSwitchValue([NotNullWhen(true)] out string? value) => | ||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.