From 56fc42191ee4b43c2adacf7b36cc7cc4c896857f Mon Sep 17 00:00:00 2001 From: Randy Eckenrode Date: Tue, 26 Nov 2024 20:22:00 -0500 Subject: [PATCH 1/4] darwin.ICU: use char16_t instead of uint16_t in the C++ API This matches the default configuration of ICU upstream. Otherwise, some packages (such as Boost) fail to build against darwin.ICU. This should be safe ABI-wise because (according to the comments), char16_t and uint16_t have the same memory representation. The reason why Apple did not make the change is due to a build failure in one of their proprietary frameworks when using char16_t. --- .../apple-source-releases/ICU/package.nix | 4 ++ .../patches/define-uchar-as-char16_t.patch | 53 +++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 pkgs/os-specific/darwin/apple-source-releases/ICU/patches/define-uchar-as-char16_t.patch diff --git a/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix b/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix index 937f5a5a14ca..1e38c3a28459 100644 --- a/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix +++ b/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix @@ -24,6 +24,10 @@ let sourceRoot = "source/icu/icu4c/source"; patches = [ + # Apple defaults to `uint16_t` for compatibility with building one of their private frameworks, + # but nixpkgs needs `char16_t` for compatibility with packages that expect upstream ICU with `char16_t`. + # According to `unicode/umachine.h`, these types are bit-compatible but distinct in C++. + ./patches/define-uchar-as-char16_t.patch # Skip MessageFormatTest test, which is known to crash sometimes and should be suppressed if it does. ./patches/suppress-icu-check-crash.patch ]; diff --git a/pkgs/os-specific/darwin/apple-source-releases/ICU/patches/define-uchar-as-char16_t.patch b/pkgs/os-specific/darwin/apple-source-releases/ICU/patches/define-uchar-as-char16_t.patch new file mode 100644 index 000000000000..65437d4cb5f2 --- /dev/null +++ b/pkgs/os-specific/darwin/apple-source-releases/ICU/patches/define-uchar-as-char16_t.patch @@ -0,0 +1,53 @@ +diff --git a/icu/icu4c/source/common/unicode/umachine.h b/icu/icu4c/source/common/unicode/umachine.h +index 9483031569..e451ad7c02 100644 +--- a/common/unicode/umachine.h ++++ b/common/unicode/umachine.h +@@ -387,39 +387,6 @@ + * @stable ICU 4.4 + */ + +-#if APPLE_ICU_CHANGES +-// rdar://121241618 (StarlightE: VideosUI-883.40.54#24 has failed to build in install; cannot initialize a variable of type 'const UChar *' (aka 'const char16_t) +-#if 0 +- // #if 1 is normal (Apple uses 0 to force us to still use uint16_t). UChar defaults to char16_t in C++. +- // For configuration testing of UChar=uint16_t temporarily change this to #if 0. +- // The intltest Makefile #defines UCHAR_TYPE=char16_t, +- // so we only #define it to uint16_t if it is undefined so far. +-#elif !defined(UCHAR_TYPE) +-# define UCHAR_TYPE uint16_t +-#endif +- +-#if defined(U_COMBINED_IMPLEMENTATION) || defined(U_COMMON_IMPLEMENTATION) || \ +- defined(U_I18N_IMPLEMENTATION) || defined(U_IO_IMPLEMENTATION) || \ +- defined (U_TOOLUTIL_IMPLEMENTATION) +- // Inside the ICU library code, never configurable. +- typedef char16_t UChar; +-#elif defined(T_CTEST_IMPLEMENTATION) +- // internally to ctestfw, we want to use char16_t in C++ and uint_16 in C +- #if U_CPLUSPLUS_VERSION != 0 +- typedef char16_t UChar; // C++ +- #else +- typedef uint16_t UChar; // C +- #endif +-#elif defined(UCHAR_TYPE) +- typedef UCHAR_TYPE UChar; +-#elif U_CPLUSPLUS_VERSION != 0 +- typedef char16_t UChar; // C++ +-#else +- typedef uint16_t UChar; // C +-#endif +- +-#else +- + #if 1 + // #if 1 is normal. UChar defaults to char16_t in C++. + // For configuration testing of UChar=uint16_t temporarily change this to #if 0. +@@ -441,8 +408,6 @@ + typedef uint16_t UChar; // C + #endif + +-#endif // APPLE_ICU_CHANGES +- + /** + * \var OldUChar + * Default ICU 58 definition of UChar. From 49936d62185923e0a0c5752bd64c325139638b49 Mon Sep 17 00:00:00 2001 From: Randy Eckenrode Date: Tue, 26 Nov 2024 20:22:00 -0500 Subject: [PATCH 2/4] darwin.ICU: enable the C++ API by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ICU C++ API is not ABI stable. Since ICU is normally shipped as a system library, Apple has disabled it by default. This breaks using ICU as the default implementation on Darwin in nixpkgs because packages expect to use the C++ API without having to opt into it. This change enables the C++ API by default based on the assumption that the packages using it in nixpkgs are built from source, and any binaries that would link against `libicucore.B.dylib` won’t be using the C++ API and should not suffer from ABI problems, and if they did use the C++ API, they would have ABI problems regardless because the symbols are still present in the ICU dylibs. --- .../apple-source-releases/ICU/package.nix | 2 + .../patches/enable-cxx-api-by-default.patch | 54 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 pkgs/os-specific/darwin/apple-source-releases/ICU/patches/enable-cxx-api-by-default.patch diff --git a/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix b/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix index 1e38c3a28459..0cb5a3c2b2e2 100644 --- a/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix +++ b/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix @@ -28,6 +28,8 @@ let # but nixpkgs needs `char16_t` for compatibility with packages that expect upstream ICU with `char16_t`. # According to `unicode/umachine.h`, these types are bit-compatible but distinct in C++. ./patches/define-uchar-as-char16_t.patch + # Enable the C++ API by default to match the upstream ICU packaging in nixpkgs + ./patches/enable-cxx-api-by-default.patch # Skip MessageFormatTest test, which is known to crash sometimes and should be suppressed if it does. ./patches/suppress-icu-check-crash.patch ]; diff --git a/pkgs/os-specific/darwin/apple-source-releases/ICU/patches/enable-cxx-api-by-default.patch b/pkgs/os-specific/darwin/apple-source-releases/ICU/patches/enable-cxx-api-by-default.patch new file mode 100644 index 000000000000..3b324aeca4ec --- /dev/null +++ b/pkgs/os-specific/darwin/apple-source-releases/ICU/patches/enable-cxx-api-by-default.patch @@ -0,0 +1,54 @@ +diff --git a/icu/icu4c/source/common/unicode/utypes.h b/icu/icu4c/source/common/unicode/utypes.h +index a3e0ec911c..e361c658ef 100644 +--- a/common/unicode/utypes.h ++++ b/common/unicode/utypes.h +@@ -66,20 +66,6 @@ + * \def U_SHOW_CPLUSPLUS_API + * @internal + */ +-#if APPLE_ICU_CHANGES +-// rdar://60884991 #58 Replace installsrc patching with changes directly in header files +-// Apple modifies the default to be 0, not 1 +-#ifdef __cplusplus +-# ifndef U_SHOW_CPLUSPLUS_API +-# define U_SHOW_CPLUSPLUS_API 0 +-# endif +-#else +-# undef U_SHOW_CPLUSPLUS_API +-# define U_SHOW_CPLUSPLUS_API 0 +-#endif +- +-#else +- + #ifdef __cplusplus + # ifndef U_SHOW_CPLUSPLUS_API + # define U_SHOW_CPLUSPLUS_API 1 +@@ -89,28 +75,6 @@ + # define U_SHOW_CPLUSPLUS_API 0 + #endif + +-#endif // APPLE_ICU_CHANGES +- +- +-#if APPLE_ICU_CHANGES +-// rdar://30624081 64b8ed9b89.. Add #if U_SHOW_CPLUSPLUS_API..#endif around C++ interfaces that don’t have it +-// rdar://24075048 0f5f76d43c.. update ICU for AAS to use VS2015, remove old ICU 4.0 shims, fix build issues +-/* +- * Apple-specific warning if U_SHOW_CPLUSPLUS_API set and the compile +- * is not for a build of ICU itself (ICU_DATA_DIR is always defined +- * for ICU builds, and is unlikely to be defined for client builds). +- * Windows VSC compliler does not like #warning, skip for it. +- */ +-#if U_SHOW_CPLUSPLUS_API +-#ifndef ICU_DATA_DIR +-#if U_PLATFORM!=U_PF_WINDOWS +-#warning Do not set U_SHOW_CPLUSPLUS_API for code that ships with the OS, it is only for local tools. +-#warning ICU C++ functionality may not be used by any OS client, it is not binary compatible across updates. +-#endif +-#endif +-#endif +-#endif // APPLE_ICU_CHANGES +- + /** @{ API visibility control */ + + /** From d8a222f4b5b8cc358d4000809a8de63f272b26be Mon Sep 17 00:00:00 2001 From: Randy Eckenrode Date: Sat, 4 Jan 2025 15:07:10 -0500 Subject: [PATCH 3/4] darwin.ICU: add `stdenv` for compatibility with Tensorflow override --- pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix | 1 + 1 file changed, 1 insertion(+) diff --git a/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix b/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix index 0cb5a3c2b2e2..ff9bab6c9cee 100644 --- a/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix +++ b/pkgs/os-specific/darwin/apple-source-releases/ICU/package.nix @@ -5,6 +5,7 @@ fixDarwinDylibNames, mkAppleDerivation, python3, + stdenv, # Necessary for compatibility with python3Packages.tensorflow, which tries to override the stdenv testers, }: From d85513502835d4ab055c567b0d17e91347221ce7 Mon Sep 17 00:00:00 2001 From: Randy Eckenrode Date: Tue, 26 Nov 2024 20:22:00 -0500 Subject: [PATCH 4/4] icu: make darwin.ICU the default on Darwin --- pkgs/top-level/all-packages.nix | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkgs/top-level/all-packages.nix b/pkgs/top-level/all-packages.nix index 4182436fa084..9652b4eb9491 100644 --- a/pkgs/top-level/all-packages.nix +++ b/pkgs/top-level/all-packages.nix @@ -9376,7 +9376,12 @@ with pkgs; icu76 ; - icu = icu74; + # Use Apple’s fork of ICU by default, which provides additional APIs that are not present in upstream ICU. + # + # `icuReal` is provided in case the upstream icu package is needed on Darwin instead of the fork. + # Note that the versioned icu packages always correspond to the upstream versions. + icuReal = icu74; + icu = if stdenv.hostPlatform.isDarwin then darwin.ICU else icuReal; idasen = with python3Packages; toPythonApplication idasen;