Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect the default locale name during startup on Apple platforms #68706

Merged
merged 8 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/coreclr/dlls/mscoree/coreclr/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,15 @@ if(FEATURE_MERGE_JIT_AND_ENGINE)
set(CLRJIT_STATIC clrjit_static)
endif(FEATURE_MERGE_JIT_AND_ENGINE)

if (CLR_CMAKE_TARGET_OSX OR CLR_CMAKE_TARGET_MACCATALYST)
include(CMakeFindFrameworks)
find_library(FOUNDATION Foundation REQUIRED)
endif()

target_sources(coreclr PUBLIC $<TARGET_OBJECTS:cee_wks_core>)
target_link_libraries(coreclr PUBLIC ${CORECLR_LIBRARIES} ${CLRJIT_STATIC} cee_wks)
target_link_libraries(coreclr PUBLIC ${CORECLR_LIBRARIES} ${CLRJIT_STATIC} cee_wks ${FOUNDATION})
target_sources(coreclr_static PUBLIC $<TARGET_OBJECTS:cee_wks_core>)
target_link_libraries(coreclr_static PUBLIC ${CORECLR_LIBRARIES} clrjit_static cee_wks_mergeable)
target_link_libraries(coreclr_static PUBLIC ${CORECLR_LIBRARIES} clrjit_static cee_wks_mergeable ${FOUNDATION})
target_compile_definitions(coreclr_static PUBLIC CORECLR_EMBEDDED)

if(CLR_CMAKE_TARGET_WIN32)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,17 @@ public void CurrentCulture()
}
}

[Fact]
[PlatformSpecific(TestPlatforms.OSX | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not going to pass on windows and Linux too?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible to fail on Linux and Windows. On Windows it is unlikely the users will set the user locale to Invariant but it is possible. For Linux, some distros set the locale to "C" which we replace it with Invariant. That means it is likely to fail on Linux.

public void CurrentCulture_Default_Not_Invariant()
{
// On OSX-like platforms, it should default to what the default system culture is
// set to. Since we shouldn't assume en-US, we just test if it's not the invariant
// culture.
Assert.NotEqual(CultureInfo.CurrentCulture, CultureInfo.InvariantCulture);
Assert.NotEqual(CultureInfo.CurrentUICulture, CultureInfo.InvariantCulture);
}

[Fact]
public void CurrentCulture_Set_Null_ThrowsArgumentNullException()
{
Expand Down Expand Up @@ -125,7 +136,7 @@ public void CurrentCulture_BasedOnLangEnvVar(string langEnvVar, string expectedC
}, expectedCultureName, new RemoteInvokeOptions { StartInfo = psi }).Dispose();
}

[PlatformSpecific(TestPlatforms.AnyUnix)] // When LANG is empty or unset, should default to the invariant culture on Unix.
[PlatformSpecific(TestPlatforms.AnyUnix)]
[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[InlineData("")]
[InlineData(null)]
Expand All @@ -141,13 +152,28 @@ public void CurrentCulture_DefaultWithNoLang(string langEnvVar)
psi.Environment["LANG"] = langEnvVar;
}

// When LANG is empty or unset, on Unix it should default to the invariant culture.
// On OSX-like platforms, it should default to what the default system culture is
// set to. Since we shouldn't assume en-US, we just test if it's not the invariant
// culture.
RemoteExecutor.Invoke(() =>
{
Assert.NotNull(CultureInfo.CurrentCulture);
Assert.NotNull(CultureInfo.CurrentUICulture);

Assert.Equal("", CultureInfo.CurrentCulture.Name);
Assert.Equal("", CultureInfo.CurrentUICulture.Name);
if (PlatformDetection.IsOSXLike)
{
Assert.NotEqual("", CultureInfo.CurrentCulture.Name);
Assert.NotEqual("", CultureInfo.CurrentUICulture.Name);

Assert.NotEqual(CultureInfo.CurrentCulture, CultureInfo.InvariantCulture);
Assert.NotEqual(CultureInfo.CurrentUICulture, CultureInfo.InvariantCulture);
}
else
{
Assert.Equal("", CultureInfo.CurrentCulture.Name);
Assert.Equal("", CultureInfo.CurrentUICulture.Name);
}
}, new RemoteInvokeOptions { StartInfo = psi }).Dispose();
}

Expand Down
7 changes: 7 additions & 0 deletions src/mono/mono/mini/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ if(HAVE_SYS_ICU AND NOT HOST_WASI)
pal_timeZoneInfo.c
entrypoints.c
${pal_icushim_sources_base})

if (TARGET_DARWIN)
set(icu_shim_sources_base
${icu_shim_sources_base}
pal_locale.m)
endif()

addprefix(icu_shim_sources "${ICU_SHIM_PATH}" "${icu_shim_sources_base}")
set_source_files_properties(${icu_shim_sources} PROPERTIES COMPILE_DEFINITIONS OSX_ICU_LIBRARY_PATH="${OSX_ICU_LIBRARY_PATH}")
set_source_files_properties(${icu_shim_sources} PROPERTIES COMPILE_FLAGS "-I\"${ICU_INCLUDEDIR}\" -I\"${CLR_SRC_NATIVE_DIR}/libs/System.Globalization.Native/\" -I\"${CLR_SRC_NATIVE_DIR}/libs/Common/\" ${ICU_FLAGS}")
Expand Down
11 changes: 10 additions & 1 deletion src/native/libs/System.Globalization.Native/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ set(NATIVEGLOBALIZATION_SOURCES
pal_icushim.c
)

if (CLR_CMAKE_TARGET_OSX OR CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS)
set(NATIVEGLOBALIZATION_SOURCES ${NATIVEGLOBALIZATION_SOURCES} pal_locale.m)
endif()

# time zone names are filtered out of icu data for the browser and associated functionality is disabled
if (NOT CLR_CMAKE_TARGET_BROWSER)
set(NATIVEGLOBALIZATION_SOURCES ${NATIVEGLOBALIZATION_SOURCES} pal_timeZoneInfo.c)
Expand All @@ -76,6 +80,11 @@ endif()
include_directories("../Common")

if (GEN_SHARED_LIB)
if (CLR_CMAKE_TARGET_OSX OR CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS)
include(CMakeFindFrameworks)
find_library(FOUNDATION Foundation REQUIRED)
endif()

add_library(System.Globalization.Native
SHARED
${NATIVEGLOBALIZATION_SOURCES}
Expand All @@ -84,12 +93,12 @@ if (GEN_SHARED_LIB)

target_link_libraries(System.Globalization.Native
dl
${FOUNDATION}
)

install_with_stripped_symbols (System.Globalization.Native PROGRAMS .)
endif()


add_library(System.Globalization.Native-Static
STATIC
${NATIVEGLOBALIZATION_SOURCES}
Expand Down
28 changes: 24 additions & 4 deletions src/native/libs/System.Globalization.Native/pal_locale.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,17 @@ int32_t FixupLocaleName(UChar* value, int32_t valueLength)
return i;
}

// We use whatever ICU give us as the default locale except if it is en_US_POSIX. We'll map
// this POSIX locale to Invariant instead. The reason is POSIX locale collation behavior
// is not desirable at all because it doesn't support case insensitive string comparisons.
// We use whatever ICU give us as the default locale except if it is en_US_POSIX.
//
// On Apple related platforms (OSX, iOS, tvOS, MacCatalyst), we'll take what the system locale is.
// On all other platforms we'll map this POSIX locale to Invariant instead.
// The reason is POSIX locale collation behavior is not desirable at all because it doesn't support case insensitive string comparisons.
const char* DetectDefaultLocaleName()
{
const char* icuLocale = uloc_getDefault();
steveisok marked this conversation as resolved.
Show resolved Hide resolved
const char* icuLocale;

icuLocale = uloc_getDefault();

if (strcmp(icuLocale, "en_US_POSIX") == 0)
{
return "";
Expand Down Expand Up @@ -216,6 +221,16 @@ int32_t GlobalizationNative_GetDefaultLocaleName(UChar* value, int32_t valueLeng

const char* defaultLocale = DetectDefaultLocaleName();

#ifdef __APPLE__
char* appleLocale = NULL;

if (strcmp(defaultLocale, "") == 0)
{
appleLocale = DetectDefaultAppleLocaleName();
defaultLocale = appleLocale;
}
#endif

uloc_getBaseName(defaultLocale, localeNameBuffer, ULOC_FULLNAME_CAPACITY, &status);
u_charsToUChars_safe(localeNameBuffer, value, valueLength, &status);

Expand All @@ -236,6 +251,11 @@ int32_t GlobalizationNative_GetDefaultLocaleName(UChar* value, int32_t valueLeng
}
}

#ifdef __APPLE__
if (appleLocale)
free(appleLocale);
#endif

return UErrorCodeToBool(status);
}

Expand Down
30 changes: 30 additions & 0 deletions src/native/libs/System.Globalization.Native/pal_locale.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

#include <stdlib.h>
#include "pal_locale_internal.h"

#import <Foundation/Foundation.h>

char* DetectDefaultAppleLocaleName()
{
NSLocale *currentLocale = [NSLocale currentLocale];
NSString *localeName = @"";

if (!currentLocale)
{
return strdup([localeName UTF8String]);
}

if ([currentLocale.languageCode length] > 0 && [currentLocale.countryCode length] > 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currentLocale

do you know if possible having a collation parts too? I mean ICU can have a locale name like:

de_DE@collation=phoneb which will be translated to .NET name as de-DE_phoneb.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, I would suggest getting localeIdentifier then normalize it to the .NET culture name form.

{
localeName = [NSString stringWithFormat:@"%@-%@", currentLocale.languageCode, currentLocale.countryCode];
}
else
{
localeName = [currentLocale.localeIdentifier stringByReplacingOccurrencesOfString:@"_" withString:@"-"];
steveisok marked this conversation as resolved.
Show resolved Hide resolved
}

return strdup([localeName UTF8String]);
akoeplinger marked this conversation as resolved.
Show resolved Hide resolved
}

10 changes: 10 additions & 0 deletions src/native/libs/System.Globalization.Native/pal_locale_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,13 @@ Detect the default locale for the machine, defaulting to Invaraint if
we can't compute one (different from uloc_getDefault()) would do.
*/
const char* DetectDefaultLocaleName(void);

#ifdef __APPLE__
/*
Function:
DetectDefaultSystemLocaleName

Detects the default locale string for Apple platforms
*/
char* DetectDefaultAppleLocaleName(void);
#endif