[caraioniurie47]

When CMAKE_ICU_DIR is set for a Unix target other than Android, Apple, Browser and WASI, CMakeLists.txt skips find_path(UCURR_H ...) and adds ${CMAKE_ICU_DIR}/include only after include(configure.cmake). The configure checks for HAVE_UDAT_STANDALONE_SHORTER_WEEKDAYS and HAVE_UCOL_CLONE therefore compile without the ICU headers from that directory.

With no ICU headers in the default include path, both checks fail with "file not found" and config.h gets 0 for both, while the build still succeeds. With system ICU headers installed, the checks find those through the default include path instead of the ones in CMAKE_ICU_DIR.

With HAVE_UDAT_STANDALONE_SHORTER_WEEKDAYS 0, pal_calendarData.c uses UDAT_STANDALONE_NARROW_WEEKDAYS for super-short day names. With HAVE_UCOL_CLONE 0 and STATIC_ICU defined (the LOCAL_BUILD configuration), pal_collation.c calls ucol_safeClone, which is deprecated in ICU 78.3 and fails the build under -Werror.

The fix adds ${CMAKE_ICU_DIR}/include to CMAKE_REQUIRED_INCLUDES in configure.cmake when CMAKE_ICU_DIR is defined, and then builds the ucol_clone check as a static library, so it only compiles. With ICU symbol renaming disabled, ucol_clone is a function declaration, and the check would otherwise need to link against ICU libraries that this path never sets. Builds that do not set CMAKE_ICU_DIR are unaffected.

src/native/libs/System.Globalization.Native has no tests of its own, so the change was checked by building:

• ./build.sh libs.native -c Release -cmakeargs "-DCMAKE_ICU_DIR=<prefix>" with a static ICU 78.3 (--enable-static --disable-shared) on a machine without system ICU headers: config.h had both values 0 before the change and both 1 after it, from a clean artifacts/obj/native.
• The same with system ICU headers (libicu-dev 74.2) installed: after the change the try-compile command lines in CMakeConfigureLog.yaml include -I<prefix>/include; before it they had no -I option.
• CC=clang cmake -S src/native/libs/System.Globalization.Native -B <build-dir> -DLOCAL_BUILD:STRING=1 -DCLR_CMAKE_TARGET_UNIX:STRING=1 -DCMAKE_ICU_DIR=<prefix> -DCMAKE_C_FLAGS=-Werror then make -C <build-dir>, without system ICU headers: failed on 'ucol_safeClone_78' is deprecated before the change; built without warnings after it.
• The same cmake and make with a static ICU 78.3 built with --disable-renaming, applied either by putting its uconfig.h.prepend at the top of uconfig.h or by adding -DU_DISABLE_RENAMING=1 to CMAKE_C_FLAGS: with the include change alone, HAVE_UCOL_CLONE was 0 (undefined reference to 'ucol_clone') and the build failed on 'ucol_safeClone' is deprecated; with the check compile-only, it was 1 and the build succeeded.
• ./build.sh libs.native -c Release without CMAKE_ICU_DIR: config.h and libSystem.Globalization.Native.so are byte-identical before and after the change.

A build directory configured with CMAKE_ICU_DIR before this change keeps its cached check results; configure it again from an empty directory.

Resolves #135571

Note

AI-generated, written at my direction and reviewed by me before posting.
ICU 78.3 throughout (icu4c-78.3-sources.tgz, --enable-static --disable-shared).
The first three bullets were measured for the first commit on dotnet/runtime d148189 (System.Globalization.Native is unchanged from there to this PR's base), Ubuntu 24.04.4 LTS x64, clang 18.1.3, CMake 3.28.3, .NET SDK 11.0.100-rc.1.26420.103.
At this PR's base on Ubuntu 22.04.5 LTS x64, clang 14.0.0, CMake 3.31.12 (system libicu-dev 70.1, its headers hidden by a bind mount where a bullet says "without system ICU headers"), with both commits: the first and third bullets again, the --disable-renaming bullet and the last bullet. The paragraph after the bullets was measured there for the first commit: a build with the change reverted, then the change applied and the same command run again in the same directory.
Commands: the builds listed above, each from a removed build directory, with grep HAVE_ on the generated config.h, the try-compile entries of CMakeConfigureLog.yaml, and sha256sum of libSystem.Globalization.Native.so.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

[tarekgh]

@caraioniurie47 Could CMAKE_ICU_DIR also point to an ICU build with symbol renaming disabled?

With the normal ICU configuration, ucol_clone is a macro that maps to the versioned symbol, so check_symbol_exists succeeds from the header alone. If symbol renaming is disabled, ucol_clone is a function declaration and the check needs to link. In this path, however, find_library is skipped and ICUUC and ICUI18N appear to remain empty, so HAVE_UCOL_CLONE may still be detected as 0 even when the custom ICU provides it.

Is ICU without symbol renaming intended to be supported through CMAKE_ICU_DIR? If so, should the check also use the libraries under CMAKE_ICU_DIR, or otherwise be made compile-only for this case?

[tarekgh]

CC @janvorli

[janvorli]

LGTM, thank you!

[caraioniurie47]

@caraioniurie47 Could CMAKE_ICU_DIR also point to an ICU build with symbol renaming disabled?

With the normal ICU configuration, ucol_clone is a macro that maps to the versioned symbol, so check_symbol_exists succeeds from the header alone. If symbol renaming is disabled, ucol_clone is a function declaration and the check needs to link. In this path, however, find_library is skipped and ICUUC and ICUI18N appear to remain empty, so HAVE_UCOL_CLONE may still be detected as 0 even when the custom ICU provides it.

Is ICU without symbol renaming intended to be supported through CMAKE_ICU_DIR? If so, should the check also use the libraries under CMAKE_ICU_DIR, or otherwise be made compile-only for this case?

@tarekgh Thanks. In this repo only the wasm build sets CMAKE_ICU_DIR, and its values are hard-coded in configure.cmake, but supporting such an ICU looks cheap. I'm measuring the check on an ICU built with --disable-renaming and will push a fix to this PR with the results. Could the merge wait for that push?

[caraioniurie47]

@tarekgh Confirmed: with renaming disabled the check still gave HAVE_UCOL_CLONE 0. The check's result is used only by a LOCAL_BUILD, and nothing in this repo passes CMAKE_ICU_DIR to one (build-local.sh does not), so this affects hand-configured builds only. The latest commit builds the ucol_clone check as a static library when CMAKE_ICU_DIR is set, so it only compiles; it saves and restores CMAKE_TRY_COMPILE_TARGET_TYPE as src/native/libs/configure.cmake does. The headers and the libraries come from the same directory, so the declaration is enough.

--disable-renaming does not change the installed uconfig.h (ICU writes the define to uconfig.h.prepend instead), so I measured both ways to apply it: that file put at the top of uconfig.h, and -DU_DISABLE_RENAMING=1 in CMAKE_C_FLAGS. Both gave the same results, configuring and building as build-local.sh does, plus CMAKE_ICU_DIR and -Werror, without system ICU headers:

ucol_clone check	HAVE_UCOL_CLONE	build
first commit (headers only)	0, undefined reference to 'ucol_clone'	fails: 'ucol_safeClone' is deprecated
linking the CMAKE_ICU_DIR archives (libicui18n.a libicuuc.a libicudata.a stdc++ m dl pthread)	1	succeeds
compile-only (latest commit)	1	succeeds

I chose compile-only because a link check has to list ICU's dependencies in link order: the same archives in the order CMakeLists.txt links them (libicuuc.a libicui18n.a libicudata.a stdc++) failed on ICU's own C++ symbols and gave 0. With a default (renamed) ICU, the latest commit still gives 1 and builds.

Note

AI-generated, written at my direction and reviewed by me before posting.
Measured at this PR's base (90b22d9) with each change the table names applied, on Ubuntu 22.04.5 LTS x64, clang 14.0.0, CMake 3.31.12, ICU 78.3 (icu4c-78.3-sources.tgz, --enable-static --disable-shared --disable-renaming); the default-ICU sentence used the same ICU built without --disable-renaming.

[tarekgh]

Thanks @caraioniurie47!