From 9813884d1ebf309e39288b303b4865cded4696de Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz Date: Tue, 12 Dec 2023 15:19:00 +0100 Subject: [PATCH 1/4] Workaround. --- .../runtime-extra-platforms-wasm.yml | 32 +++++++++---------- .../src/System/Globalization/CultureInfo.cs | 8 +++++ src/libraries/tests.proj | 4 --- .../HybridGlobalizationTests.cs | 1 - 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml b/eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml index f4e9a8447a210..12981837609c3 100644 --- a/eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml +++ b/eng/pipelines/extra-platforms/runtime-extra-platforms-wasm.yml @@ -221,22 +221,22 @@ jobs: - WasmTestOnBrowser - WasmTestOnNodeJS - # Hybrid Globalization AOT tests - https://github.com/dotnet/runtime/issues/94212 - # - template: /eng/pipelines/common/templates/wasm-library-aot-tests.yml - # parameters: - # platforms: - # - browser_wasm - # - browser_wasm_win - # nameSuffix: _HybridGlobalization_AOT - # extraBuildArgs: /p:AotHostArchitecture=x64 /p:AotHostOS=$(_hostedOS) /p:HybridGlobalization=true - # runAOT: true - # isExtraPlatformsBuild: ${{ parameters.isExtraPlatformsBuild }} - # isWasmOnlyBuild: ${{ parameters.isWasmOnlyBuild }} - # alwaysRun: true - # scenarios: - # - normal - # - WasmTestOnBrowser - # - WasmTestOnNodeJS + # Hybrid Globalization AOT tests + - template: /eng/pipelines/common/templates/wasm-library-aot-tests.yml + parameters: + platforms: + - browser_wasm + - browser_wasm_win + nameSuffix: _HybridGlobalization_AOT + extraBuildArgs: /p:AotHostArchitecture=x64 /p:AotHostOS=$(_hostedOS) /p:HybridGlobalization=true + runAOT: true + isExtraPlatformsBuild: ${{ parameters.isExtraPlatformsBuild }} + isWasmOnlyBuild: ${{ parameters.isWasmOnlyBuild }} + alwaysRun: true + scenarios: + - normal + - WasmTestOnBrowser + - WasmTestOnNodeJS - ${{ if and(ne(parameters.isRollingBuild, true), ne(parameters.excludeNonLibTests, true), ne(parameters.debuggerTestsOnly, true)) }}: # Builds only diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs index ab02a7a01892e..b12ebf7d74c68 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs @@ -102,7 +102,11 @@ public partial class CultureInfo : IFormatProvider, ICloneable private static volatile CultureInfo? s_userDefaultUICulture; // The Invariant culture; +#if TARGET_BROWSER + private static CultureInfo? s_InvariantCultureInfo; +#else private static readonly CultureInfo s_InvariantCultureInfo = new CultureInfo(CultureData.Invariant, isReadOnly: true); +#endif // These are defaults that we use if a thread has not opted into having an explicit culture private static volatile CultureInfo? s_DefaultThreadCurrentUICulture; @@ -462,7 +466,11 @@ public static CultureInfo InvariantCulture { get { +#if TARGET_BROWSER + s_InvariantCultureInfo ??= new CultureInfo(CultureData.Invariant, isReadOnly: true); +#else Debug.Assert(s_InvariantCultureInfo != null); +#endif return s_InvariantCultureInfo; } } diff --git a/src/libraries/tests.proj b/src/libraries/tests.proj index f1d957935c6e0..4a9d3854edc07 100644 --- a/src/libraries/tests.proj +++ b/src/libraries/tests.proj @@ -78,10 +78,6 @@ - - - - diff --git a/src/mono/wasm/Wasm.Build.Tests/HybridGlobalizationTests.cs b/src/mono/wasm/Wasm.Build.Tests/HybridGlobalizationTests.cs index 843eb34b5413e..66cee78c5de83 100644 --- a/src/mono/wasm/Wasm.Build.Tests/HybridGlobalizationTests.cs +++ b/src/mono/wasm/Wasm.Build.Tests/HybridGlobalizationTests.cs @@ -22,7 +22,6 @@ public HybridGlobalizationTests(ITestOutputHelper output, SharedBuildPerTestClas .WithRunHosts(host) .UnwrapItemsAsArrays(); - [ActiveIssue("https://github.com/dotnet/runtime/issues/94212")] [Theory] [MemberData(nameof(HybridGlobalizationTestData), parameters: new object[] { /*aot*/ false, RunHost.All })] [MemberData(nameof(HybridGlobalizationTestData), parameters: new object[] { /*aot*/ true, RunHost.All })] From cbfa9a8cb603d73a7e148e5f45d640cf1ca08881 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Thu, 14 Dec 2023 08:29:28 +0000 Subject: [PATCH 2/4] Feedback --- .../src/System/Globalization/CultureInfo.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs index b12ebf7d74c68..b3ad7df8741f3 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs @@ -102,6 +102,7 @@ public partial class CultureInfo : IFormatProvider, ICloneable private static volatile CultureInfo? s_userDefaultUICulture; // The Invariant culture; +// https://github.com/dotnet/runtime/issues/94225 #if TARGET_BROWSER private static CultureInfo? s_InvariantCultureInfo; #else @@ -466,6 +467,7 @@ public static CultureInfo InvariantCulture { get { +// https://github.com/dotnet/runtime/issues/94225 #if TARGET_BROWSER s_InvariantCultureInfo ??= new CultureInfo(CultureData.Invariant, isReadOnly: true); #else From 0fb31c341594bbfbb7a6df4f28b7cf2122edb723 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Tue, 19 Dec 2023 09:16:33 +0000 Subject: [PATCH 3/4] feedback --- .../src/System/Globalization/CultureInfo.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs index b3ad7df8741f3..91ecc5c376d4f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs @@ -104,7 +104,7 @@ public partial class CultureInfo : IFormatProvider, ICloneable // The Invariant culture; // https://github.com/dotnet/runtime/issues/94225 #if TARGET_BROWSER - private static CultureInfo? s_InvariantCultureInfo; + private static CultureInfo? s_InvariantCultureInfo = GlobalizationMode.Hybrid ? null : new CultureInfo(CultureData.Invariant, isReadOnly: true); #else private static readonly CultureInfo s_InvariantCultureInfo = new CultureInfo(CultureData.Invariant, isReadOnly: true); #endif @@ -469,10 +469,10 @@ public static CultureInfo InvariantCulture { // https://github.com/dotnet/runtime/issues/94225 #if TARGET_BROWSER - s_InvariantCultureInfo ??= new CultureInfo(CultureData.Invariant, isReadOnly: true); -#else - Debug.Assert(s_InvariantCultureInfo != null); + if (GlobalizationMode.Hybrid) + s_InvariantCultureInfo ??= new CultureInfo(CultureData.Invariant, isReadOnly: true); #endif + Debug.Assert(s_InvariantCultureInfo != null); return s_InvariantCultureInfo; } } From 1ef5bf0454d20b8f2b17b5427f5cb139dfd6f598 Mon Sep 17 00:00:00 2001 From: Ilona Tomkowicz <32700855+ilonatommy@users.noreply.github.com> Date: Wed, 20 Dec 2023 12:33:15 +0000 Subject: [PATCH 4/4] Fix the source of the problem + improve assert logs. --- .../src/System/AppContextConfigHelper.cs | 4 ++-- .../src/System/Globalization/CultureInfo.cs | 12 +----------- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/AppContextConfigHelper.cs b/src/libraries/System.Private.CoreLib/src/System/AppContextConfigHelper.cs index c16f72184736b..cfb4629187dcb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/AppContextConfigHelper.cs +++ b/src/libraries/System.Private.CoreLib/src/System/AppContextConfigHelper.cs @@ -15,11 +15,11 @@ internal static bool GetBooleanConfig(string switchName, string envVariable, boo string? str = Environment.GetEnvironmentVariable(envVariable); if (str != null) { - if (str.Equals("1", StringComparison.Ordinal) || bool.IsTrueStringIgnoreCase(str)) + if (str == "1" || bool.IsTrueStringIgnoreCase(str)) { return true; } - if (str.Equals("0", StringComparison.Ordinal) || bool.IsFalseStringIgnoreCase(str)) + if (str == "0" || bool.IsFalseStringIgnoreCase(str)) { return false; } diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs index 91ecc5c376d4f..a0789def2711c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureInfo.cs @@ -102,12 +102,7 @@ public partial class CultureInfo : IFormatProvider, ICloneable private static volatile CultureInfo? s_userDefaultUICulture; // The Invariant culture; -// https://github.com/dotnet/runtime/issues/94225 -#if TARGET_BROWSER - private static CultureInfo? s_InvariantCultureInfo = GlobalizationMode.Hybrid ? null : new CultureInfo(CultureData.Invariant, isReadOnly: true); -#else private static readonly CultureInfo s_InvariantCultureInfo = new CultureInfo(CultureData.Invariant, isReadOnly: true); -#endif // These are defaults that we use if a thread has not opted into having an explicit culture private static volatile CultureInfo? s_DefaultThreadCurrentUICulture; @@ -467,12 +462,7 @@ public static CultureInfo InvariantCulture { get { -// https://github.com/dotnet/runtime/issues/94225 -#if TARGET_BROWSER - if (GlobalizationMode.Hybrid) - s_InvariantCultureInfo ??= new CultureInfo(CultureData.Invariant, isReadOnly: true); -#endif - Debug.Assert(s_InvariantCultureInfo != null); + Debug.Assert(s_InvariantCultureInfo != null, "[CultureInfo.InvariantCulture] s_InvariantCultureInfo is null"); return s_InvariantCultureInfo; } }