Enable global invariant on OSX#10470
Conversation
|
@dotnet-bot test Ubuntu x64 corefx_baseline |
| SET(FEATURE_FIXED_ICU_VERSION 1) | ||
|
|
||
| add_definitions(-DOSX_ICU_LIBRARY_PATH=\"${ICUCORE}\") | ||
| add_definitions(-DOSX_PLATFORM) |
There was a problem hiding this comment.
This define is not necessary, use __APPLE__ instead which is defined by the OSX headers. We use the same in CoreCLR and CoreRT.
| // 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() | ||
| #ifdef OSX_PLATFORM |
There was a problem hiding this comment.
None of the functions / version constants above are needed on OSX, could you please ifdef them out?
|
|
||
| #else | ||
|
|
||
| bool InitializeICULinks(char* symbolName, char* symbolVersion) |
There was a problem hiding this comment.
Why is the name InitializeICULinks? I'm not sure what the "links" means. Would you mind renaming it to FindICULibs? That would represent better what the function does.
There was a problem hiding this comment.
I am not good in naming. I used links to include the libraries and symbols. but I can change it as you suggested as I don't have strong opinion on that :-)
|
@dotnet-bot test Ubuntu x64 corefx_baseline |
|
@janvorli I have addressed all your feedback. thanks. |
|
thanks @janvorli for your review. |
Fixes #10329
While building System.Globalization.native for OSX, we used to statically link to ICU library
The change here is instead of static linking to ICU library, we'll dynamic loading the library at run time. This will allow us to not load the ICU library at all when running in the Globalization Invariant mode.
Technically we didn't change from where we load the ICU library.