Enable global invariant on OSX (#10470)

* Enable Globalization Invariant on OSX

* Fix typo

* Fix small typo

* Remove static link to ICU lib

* Addressing the feedback

diff --git a/src/corefx/System.Globalization.Native/CMakeLists.txt b/src/corefx/System.Globalization.Native/CMakeLists.txt
index 60ef5e8..55c6854 100644 (file)
--- a/src/corefx/System.Globalization.Native/CMakeLists.txt
+++ b/src/corefx/System.Globalization.Native/CMakeLists.txt
@@ -32,8 +32,8 @@ else()
         message(FATAL_ERROR "Cannot find libicucore, skipping build for System.Globalization.Native. .NET globalization is not expected to function.")
         return()
     endif()
-    # On Darwin, we always use the OS provided ICU
-    SET(FEATURE_FIXED_ICU_VERSION 1)
+
+    add_definitions(-DOSX_ICU_LIBRARY_PATH=\"${ICUCORE}\")
 endif()
 
 include(configure.cmake)
@@ -50,14 +50,9 @@ set(NATIVEGLOBALIZATION_SOURCES
     localeStringData.cpp
     normalization.cpp
     timeZoneInfo.cpp
+    icushim.cpp
 )
 
-if (NOT FEATURE_FIXED_ICU_VERSION)
-    list(APPEND NATIVEGLOBALIZATION_SOURCES
-        icushim.cpp
-    )
-endif()
-
 include_directories(${UTYPES_H})
 
 _add_library(System.Globalization.Native
@@ -76,21 +71,8 @@ set_target_properties(System.Globalization.Native_Static PROPERTIES OUTPUT_NAME
 # Disable the "lib" prefix.
 set_target_properties(System.Globalization.Native PROPERTIES PREFIX "")
 
-if (FEATURE_FIXED_ICU_VERSION)
-    add_definitions(-DFEATURE_FIXED_ICU_VERSION)
-endif()
-
 if(NOT CLR_CMAKE_PLATFORM_DARWIN)
-    if (FEATURE_FIXED_ICU_VERSION)
-        target_link_libraries(System.Globalization.Native
-            ${ICUUC}
-            ${ICUI18N}
-        )
-        target_link_libraries(System.Globalization.Native_Static
-            ${ICUUC}
-            ${ICUI18N}
-        )
-    elseif(NOT CMAKE_SYSTEM_NAME STREQUAL FreeBSD AND NOT CMAKE_SYSTEM_NAME STREQUAL NetBSD)
+    if (NOT CMAKE_SYSTEM_NAME STREQUAL FreeBSD AND NOT CMAKE_SYSTEM_NAME STREQUAL NetBSD)
         target_link_libraries(System.Globalization.Native
             dl
         )
@@ -100,7 +82,7 @@ if(NOT CLR_CMAKE_PLATFORM_DARWIN)
     endif()
 else()
     target_link_libraries(System.Globalization.Native
-        ${ICUCORE}
+        dl
     )
 
     add_definitions(-DU_DISABLE_RENAMING=1)

diff --git a/src/corefx/System.Globalization.Native/icushim.cpp b/src/corefx/System.Globalization.Native/icushim.cpp
index f8f6dd3..6e7817e 100644 (file)
--- a/src/corefx/System.Globalization.Native/icushim.cpp
+++ b/src/corefx/System.Globalization.Native/icushim.cpp
@@ -19,6 +19,33 @@ FOR_ALL_ICU_FUNCTIONS
 static void* libicuuc = nullptr;
 static void* libicui18n = nullptr;
 
+// .x.x.x, considering the max number of decimal digits for each component
+static const int MaxICUVersionStringLength = 33;
+
+#ifdef __APPLE__
+
+bool FindICULibs(char* symbolName, char* symbolVersion)
+{
+#ifndef OSX_ICU_LIBRARY_PATH
+    static_assert(false, "The ICU Library path is not defined");
+#endif // OSX_ICU_LIBRARY_PATH
+
+    // Usually OSX_ICU_LIBRARY_PATH is "/usr/lib/libicucore.dylib"
+    libicuuc = dlopen(OSX_ICU_LIBRARY_PATH, RTLD_LAZY);
+    
+    if (libicuuc == nullptr)
+    {
+        return false;
+    }
+
+    // in OSX all ICU APIs exist in the same library libicucore.A.dylib 
+    libicui18n = libicuuc;
+
+    return true;
+}
+
+#else // __APPLE__
+
 // Version ranges to search for each of the three version components
 // The rationale for major version range is that we support versions higher or
 // equal to the version we are built against and less or equal to that version
@@ -31,9 +58,6 @@ static const int MaxMinorICUVersion = 5;
 static const int MinSubICUVersion = 1;
 static const int MaxSubICUVersion = 5;
 
-// .x.x.x, considering the max number of decimal digits for each component
-static const int MaxICUVersionStringLength = 33;
-
 // Get filename of an ICU library with the requested version in the name
 // There are three possible cases of the version components values:
 // 1. Only majorVer is not equal to -1 => result is baseFileName.majorver
@@ -170,11 +194,7 @@ bool FindLibWithMajorMinorSubVersion(int* majorVer, int* minorVer, int* subVer)
     return false;
 }
 
-// GlobalizationNative_LoadICU
-// This method get called from the managed side during the globalization initialization. 
-// This method shouldn't get called at all if we are running in globalization invariant mode
-// return 0 if failed to load ICU and 1 otherwise
-extern "C" int32_t GlobalizationNative_LoadICU()
+bool FindICULibs(char* symbolName, char* symbolVersion)
 {
     int majorVer = -1;
     int minorVer = -1;
@@ -187,12 +207,8 @@ extern "C" int32_t GlobalizationNative_LoadICU()
         !FindLibWithMajorVersion(&majorVer))
     {
         // No usable ICU version found
-        return 0;
+        return false;
     }
-
-    char symbolName[128];
-    char symbolVersion[MaxICUVersionStringLength + 1] = "";
-
     // Find out the format of the version string added to each symbol
     // First try just the unversioned symbol
     if (dlsym(libicuuc, "u_strlen") == nullptr)
@@ -212,12 +228,30 @@ extern "C" int32_t GlobalizationNative_LoadICU()
                 sprintf(symbolName, "u_strlen%s", symbolVersion);
                 if (dlsym(libicuuc, symbolName) == nullptr)
                 {
-                    return 0;
+                    return false;
                 }
             }
         }
     }
 
+    return true;
+
+}
+
+#endif // __APPLE__
+
+// GlobalizationNative_LoadICU
+// This method get called from the managed side during the globalization initialization. 
+// This method shouldn't get called at all if we are running in globalization invariant mode
+// return 0 if failed to load ICU and 1 otherwise
+extern "C" int32_t GlobalizationNative_LoadICU()
+{
+    char symbolName[128];
+    char symbolVersion[MaxICUVersionStringLength + 1] = "";
+
+    if (!FindICULibs(symbolName, symbolVersion))
+        return 0;
+
     // Get pointers to all the ICU functions that are needed
 #define PER_FUNCTION_BLOCK(fn, lib) \
     static_assert((sizeof(#fn) + MaxICUVersionStringLength + 1) <= sizeof(symbolName), "The symbolName is too small for symbol " #fn); \
@@ -228,6 +262,11 @@ extern "C" int32_t GlobalizationNative_LoadICU()
     FOR_ALL_ICU_FUNCTIONS
 #undef PER_FUNCTION_BLOCK
 
+#ifdef __APPLE__
+    // libicui18n initialized with libicuuc so we null it to avoid double closing same handle   
+    libicui18n = nullptr;
+#endif // __APPLE__
+
     return 1;
 }
 

diff --git a/src/corefx/System.Globalization.Native/icushim.h b/src/corefx/System.Globalization.Native/icushim.h
index 8ec13d5..408ec15 100644 (file)
--- a/src/corefx/System.Globalization.Native/icushim.h
+++ b/src/corefx/System.Globalization.Native/icushim.h
@@ -4,17 +4,14 @@
 //
 
 // Enable calling ICU functions through shims to enable support for 
-// multiple versions of ICU if the FEATURE_FIXED_ICU_VERSION is
-// not defined.
+// multiple versions of ICU.
 
 #ifndef __ICUSHIM_H__
 #define __ICUSHIM_H__
 
 #include "config.h"
 
-#ifndef FEATURE_FIXED_ICU_VERSION
 #define U_DISABLE_RENAMING 1
-#endif
 
 // All ICU headers need to be included here so that all function prototypes are
 // available before the function pointers are declared below.
@@ -36,8 +33,6 @@
 #include <unicode/utf16.h>
 #include <unicode/utypes.h>
 
-#ifndef FEATURE_FIXED_ICU_VERSION
-
 // List of all functions from the ICU libraries that are used in the System.Globalization.Native.so
 #define FOR_ALL_UNCONDITIONAL_ICU_FUNCTIONS \
     PER_FUNCTION_BLOCK(u_charsToUChars, libicuuc) \
@@ -238,6 +233,4 @@ FOR_ALL_ICU_FUNCTIONS
 #define usearch_last(...) usearch_last_ptr(__VA_ARGS__)
 #define usearch_openFromCollator(...) usearch_openFromCollator_ptr(__VA_ARGS__)
 
-#endif // !FEATURE_ICU_VERSION_RESILIENT
-
 #endif // __ICUSHIM_H__

diff --git a/src/mscorlib/System.Private.CoreLib.csproj b/src/mscorlib/System.Private.CoreLib.csproj
index 5da3cdd..928a344 100644 (file)
--- a/src/mscorlib/System.Private.CoreLib.csproj
+++ b/src/mscorlib/System.Private.CoreLib.csproj
@@ -533,8 +533,7 @@
     <Compile Include="$(BclSourcesRoot)\System\Globalization\DateTimeStyles.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Globalization\DigitShapes.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Globalization\EastAsianLunisolarCalendar.cs" />
-    <Compile Include="$(BclSourcesRoot)\System\Globalization\GlobalizationMode.cs"  Condition="'$(BuildOS)' != 'OSX'"/>
-    <Compile Include="$(BclSourcesRoot)\System\Globalization\GlobalizationMode.OSX.cs"  Condition="'$(BuildOS)' == 'OSX'"/>
+    <Compile Include="$(BclSourcesRoot)\System\Globalization\GlobalizationMode.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Globalization\GregorianCalendar.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Globalization\GregorianCalendarHelper.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Globalization\GregorianCalendarTypes.cs" />
@@ -576,7 +575,7 @@
     <Compile Include="$(BclSourcesRoot)\System\Globalization\CompareInfo.Unix.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Globalization\CultureData.Unix.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Globalization\CultureInfo.Unix.cs" />
-    <Compile Include="$(BclSourcesRoot)\System\Globalization\GlobalizationMode.Unix.cs" Condition="'$(BuildOS)' != 'OSX'"/>
+    <Compile Include="$(BclSourcesRoot)\System\Globalization\GlobalizationMode.Unix.cs" />
     <Compile Include="$(BclSourcesRoot)\System\Globalization\TextInfo.Unix.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsUnix)' != 'true'">

diff --git a/src/mscorlib/src/System/Globalization/GlobalizationMode.OSX.cs b/src/mscorlib/src/System/Globalization/GlobalizationMode.OSX.cs
deleted file mode 100644 (file)
index 077ce18..0000000
--- a/src/mscorlib/src/System/Globalization/GlobalizationMode.OSX.cs
+++ /dev/null
@@ -1,12 +0,0 @@
-// Licensed to the .NET Foundation under one or more agreements.
-// The .NET Foundation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-namespace System.Globalization
-{
-    internal sealed partial class GlobalizationMode
-    {
-        // private const string c_InvariantModeConfigSwitch = "System.Globalization.Invariant";
-        internal static bool Invariant { get; } = false;
-    }
-}