Skip to content

Commit

Permalink
Fix InvariantGlobalization=false in a trimmed app (#53453)
Browse files Browse the repository at this point in the history
* Fix InvariantGlobalization=false in a trimmed app.

Move the LoadICU logic into a static ctor, so it runs early in the process, but not as part of querying the GlobalizationMode.Invariant property. This allows for LoadICU to still be invoked, even when the app is trimmed and GlobalizationMode.Invariant is hard-coded by the linker.

While I was changing this code, I also removed the workaround in Browser builds for swallowing errors being returned from LoadICU.

Fix #49073
Fix #49391

* Add trimming tests for InvariantGlobalization

* Update the LoadICU message for mobile workloads.

* Respond to PR feedback.

- Split the substitutions of GlobalizationMode.Invariant between true and false
- Add a trimming test that ensures Invariant=true trims everything it is supposed to

* Add checks for all mobile platforms
  • Loading branch information
eerhardt authored Jun 8, 2021
1 parent 0377558 commit 3ead4ac
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@
<method signature="System.Boolean IsEnabled(System.Diagnostics.Tracing.EventLevel,System.Diagnostics.Tracing.EventKeywords)" body="stub" value="false" />
<method signature="System.Boolean IsEnabled(System.Diagnostics.Tracing.EventLevel,System.Diagnostics.Tracing.EventKeywords,System.Diagnostics.Tracing.EventChannel)" body="stub" value="false" />
</type>
<!-- Note: Invariant=true and Invariant=false are substituted at different levels. 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. -->
<type fullname="System.Globalization.GlobalizationMode">
<method signature="System.Boolean get_Invariant()" body="stub" value="true" feature="System.Globalization.Invariant" featurevalue="true" />
</type>
<type fullname="System.Globalization.GlobalizationMode/Settings">
<method signature="System.Boolean get_Invariant()" body="stub" value="false" feature="System.Globalization.Invariant" featurevalue="false" />
</type>
<type fullname="System.LocalAppContextSwitches">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,42 +7,49 @@ internal static partial class GlobalizationMode
{
private static partial class Settings
{
internal static readonly bool Invariant = GetGlobalizationInvariantMode();
}

internal static bool Invariant => Settings.Invariant;

internal static bool UseNls => false;
/// <summary>
/// Load ICU (when not in Invariant mode) in a static cctor to ensure it is loaded early in the process.
/// Other places, e.g. CompareInfo.GetSortKey, rely on ICU already being loaded before they are called.
/// </summary>
static Settings()
{
if (!Invariant)
{
if (TryGetAppLocalIcuSwitchValue(out string? icuSuffixAndVersion))
{
LoadAppLocalIcu(icuSuffixAndVersion);
}
else
{
int loaded = LoadICU();
if (loaded == 0)
{
Environment.FailFast(GetIcuLoadFailureMessage());
}
}
}
}

private static bool GetGlobalizationInvariantMode()
{
bool invariantEnabled = GetInvariantSwitchValue();
if (!invariantEnabled)
private static string GetIcuLoadFailureMessage()
{
if (TryGetAppLocalIcuSwitchValue(out string? icuSuffixAndVersion))
// These strings can't go into resources, because a resource lookup requires globalization, which requires ICU
if (OperatingSystem.IsBrowser() || OperatingSystem.IsAndroid() ||
OperatingSystem.IsIOS() || OperatingSystem.IsTvOS() || OperatingSystem.IsWatchOS() || OperatingSystem.IsMacCatalyst())
{
LoadAppLocalIcu(icuSuffixAndVersion);
return "Unable to load required ICU Globalization data. Please see https://aka.ms/dotnet-missing-libicu for more information";
}
else
{
int loaded = LoadICU();
if (loaded == 0 && !OperatingSystem.IsBrowser())
{
// This can't go into resources, because a resource lookup requires globalization, which requires ICU
string message = "Couldn't find a valid ICU package installed on the system. " +
"Please install libicu using your package manager and try again. " +
"Alternatively you can set the configuration flag System.Globalization.Invariant to true if you want to run with no globalization support. " +
"Please see https://aka.ms/dotnet-missing-libicu for more information.";
Environment.FailFast(message);
}

// fallback to Invariant mode if LoadICU failed (Browser).
return loaded == 0;
return "Couldn't find a valid ICU package installed on the system. " +
"Please install libicu using your package manager and try again. " +
"Alternatively you can set the configuration flag System.Globalization.Invariant to true if you want to run with no globalization support. " +
"Please see https://aka.ms/dotnet-missing-libicu for more information.";
}
}
return invariantEnabled;
}

internal static bool UseNls => false;

private static void LoadAppLocalIcuCore(ReadOnlySpan<char> version, ReadOnlySpan<char> suffix)
{

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ namespace System.Globalization
{
internal static partial class GlobalizationMode
{
// Order of these properties in Windows matter because GetUseIcuMode is dependent on Invariant.
// So we need Invariant to be initialized first.
internal static bool Invariant { get; } = GetInvariantSwitchValue();

internal static bool UseNls { get; } = !Invariant &&
(AppContextConfigHelper.GetBooleanConfig("System.Globalization.UseNls", "DOTNET_SYSTEM_GLOBALIZATION_USENLS") ||
!LoadIcu());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,18 @@ namespace System.Globalization
{
internal static partial class GlobalizationMode
{
// split from GlobalizationMode so the whole class can be trimmed when Invariant=true.
private static partial class Settings
{
internal static readonly bool PredefinedCulturesOnly = AppContextConfigHelper.GetBooleanConfig("System.Globalization.PredefinedCulturesOnly", "DOTNET_SYSTEM_GLOBALIZATION_PREDEFINED_CULTURES_ONLY");
internal static bool Invariant { get; } = GetInvariantSwitchValue();
}

// 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;

internal static bool PredefinedCulturesOnly => !Invariant && Settings.PredefinedCulturesOnly;

private static bool GetInvariantSwitchValue() =>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Globalization;
using System.Threading;

/// <summary>
/// Ensures setting InvariantGlobalization = false still works in a trimmed app.
/// </summary>
class Program
{
static int Main(string[] args)
{
// since we are using Invariant GlobalizationMode = false, setting the culture matters.
// The app will always use the current culture, so in the Turkish culture, 'i' ToUpper will NOT be "I"
Thread.CurrentThread.CurrentCulture = new CultureInfo("tr-TR");
if ("i".ToUpper() == "I")
{
// 'i' ToUpper was "I", but shouldn't be in the Turkish culture, so fail
return -1;
}

return 100;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Globalization;
using System.Reflection;
using System.Threading;

/// <summary>
/// Ensures setting InvariantGlobalization = true still works in a trimmed app.
/// </summary>
class Program
{
static int Main(string[] args)
{
// since we are using Invariant GlobalizationMode = true, setting the culture doesn't matter.
// The app will always use Invariant mode, so even in the Turkish culture, 'i' ToUpper will be "I"
Thread.CurrentThread.CurrentCulture = new CultureInfo("tr-TR");
if ("i".ToUpper() != "I")
{
// 'i' ToUpper was not "I", so fail
return -1;
}

// Ensure the internal GlobalizationMode class is trimmed correctly
Type globalizationMode = GetCoreLibType("System.Globalization.GlobalizationMode");
const BindingFlags allStatics = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static;
foreach (MemberInfo member in globalizationMode.GetMembers(allStatics))
{
// properties and their backing getter methods are OK
if (member is PropertyInfo || member.Name.StartsWith("get_"))
{
continue;
}

if (OperatingSystem.IsWindows())
{
// Windows still contains a static cctor and a backing field for UseNls
if (member is ConstructorInfo || (member is FieldInfo field && field.Name.Contains("UseNls")))
{
continue;
}
}

// Some unexpected member was left on GlobalizationMode, fail
Console.WriteLine($"Member '{member.Name}' was not trimmed from GlobalizationMode, but should have been.");
return -2;
}

return 100;
}

private static Type GetCoreLibType(string name) =>
typeof(object).Assembly.GetType(name, throwOnError: true);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
<TestConsoleAppSourceFiles Include="GenericArraySortHelperTest.cs" />
<TestConsoleAppSourceFiles Include="InheritedAttributeTests.cs" />
<TestConsoleAppSourceFiles Include="InterfacesOnArrays.cs" />
<TestConsoleAppSourceFiles Include="InvariantGlobalizationFalse.cs">
<ExtraTrimmerArgs>--feature System.Globalization.Invariant false</ExtraTrimmerArgs>
</TestConsoleAppSourceFiles>
<TestConsoleAppSourceFiles Include="InvariantGlobalizationTrue.cs">
<ExtraTrimmerArgs>--feature System.Globalization.Invariant true</ExtraTrimmerArgs>
</TestConsoleAppSourceFiles>
<TestConsoleAppSourceFiles Include="StackFrameHelperTest.cs">
<!-- There is a bug with the linker where it is corrupting the pdbs while trimming
causing the framework to not be able to get source line info any longer. This
Expand Down

0 comments on commit 3ead4ac

Please sign in to comment.