From 094f808f2b4587c4409d5d7ac5ed5b992d41864e Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Sat, 25 Mar 2017 10:41:39 -0700 Subject: 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 --- .../System.Globalization.Native/CMakeLists.txt | 28 ++------- src/corefx/System.Globalization.Native/icushim.cpp | 67 +++++++++++++++++----- src/corefx/System.Globalization.Native/icushim.h | 9 +-- 3 files changed, 59 insertions(+), 45 deletions(-) (limited to 'src/corefx') diff --git a/src/corefx/System.Globalization.Native/CMakeLists.txt b/src/corefx/System.Globalization.Native/CMakeLists.txt index 60ef5e8535..55c6854497 100644 --- 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 f8f6dd317d..6e7817e85f 100644 --- 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 8ec13d5737..408ec15edd 100644 --- 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 #include -#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__ -- cgit v1.2.3