Android: Changes to allow libicu be statically linked as well as libc…

…++_static

This merges with prior FindICU.cmake changes which allow for ICU to be
static libraries.

This change is needed for Android because dynamically linking libicu is
not reliable on Android. There are multiple problems:
- libICU is sometimes an internal/private component on Android. In some
cases it appears that tryiing to LoadLibrary on libICU will silently
skip because the OS already has one loaded. If the symbol names don't
match, you get unresolved symbol errors at runtime resulting from a
crash without a good understanding why. (This happened to me on my Nexus
7 2013 with 5.0.2). And if the symbols match, but the versions are
different, you may have even tougher problems.

- If your application needs ICU, you now run the risk of having
conflicting versions of ICU. This is a particular risk for middleware
where the user may have not built Swift themselves.

- Dealing with loading the ICU dependencies is annoying on Android
because you must write Java code to load all the libraries, and in the
correct order. And your build process must also be aware of all the
depenencies so all the .so's get packaged.

So static linking ICU solves all these problems.

But additionally, the issue of linking the C++ standard library becomes
an issue. Again, there are similar issues with dynamically linking
because Android doesn't provide a pre-installed system-wide one we can
rely on. Additionally:

- Android actually provides 3 or 4 different C++ standard libraries you
can pick from, all of which are incompatible with each other.

- Every new NDK release risks a new version of each of the standard
libraries which breaks binary compatibility. If the user is just using
the Swift compiler and doesn't think about these issues, this may cause
hard problems.

- Also, statically linking ICU caused building command line executables
(tests) to fail with unresolved symbols. Statically linking C++ fixes
this problem and the original behavior is preserved.

So static linking solves these problems too.

Two new switches are needed to build for Android:
--android-icu-data-include /path/to/include
--android-icu-data /path/to/libicudata

CMakeLists.txt

@@ -139,6 +139,10 @@ set(SWIFT_ANDROID_ICU_I18N "" CACHE STRING
   "Path to a directory containing libicui18n.so")
 set(SWIFT_ANDROID_ICU_I18N_INCLUDE "" CACHE STRING
   "Path to a directory containing headers libicui18n")
+set(SWIFT_ANDROID_ICU_DATA "" CACHE STRING
+  "Path to a directory containing libicudata.so")
+set(SWIFT_ANDROID_ICU_DATA_INCLUDE "" CACHE STRING
+  "Path to a directory containing headers libicudata")
 
 #
 # User-configurable Darwin-specific options.

cmake/modules/AddSwift.cmake

@@ -206,8 +206,11 @@ function(_add_variant_link_flags
     list(APPEND result
         "-ldl"
         "-L${SWIFT_ANDROID_NDK_PATH}/toolchains/arm-linux-androideabi-${SWIFT_ANDROID_NDK_TOOLCHAIN_VERSION}/prebuilt/linux-x86_64/lib/gcc/arm-linux-androideabi/${SWIFT_ANDROID_NDK_TOOLCHAIN_VERSION}"
-        "${SWIFT_ANDROID_NDK_PATH}/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a/libc++_shared.so"
-        "-L${SWIFT_ANDROID_ICU_UC}" "-L${SWIFT_ANDROID_ICU_I18N}")
+        "-L${SWIFT_ANDROID_ICU_UC}" "-L${SWIFT_ANDROID_ICU_I18N}" "-L${SWIFT_ANDROID_ICU_DATA}"
+        # FIXME: This is to find libc++_static.a. This will need to be more flexible for different architectures.
+        # NOTE: I'm not sure why I need both lines (to the file and to the path). But without both, the link failed.
+        "-L${SWIFT_ANDROID_NDK_PATH}/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a/libc++_static.a"
+        "-L${SWIFT_ANDROID_NDK_PATH}/sources/cxx-stl/llvm-libc++/libs/armeabi-v7a")
   else()
     list(APPEND result "-lobjc")
   endif()
@@ -1190,6 +1193,18 @@ function(_add_swift_library_single target name)
   _list_escape_for_shell("${c_compile_flags}" c_compile_flags)
   _list_escape_for_shell("${link_flags}" link_flags)
 
+
+  # ANDROID hack: We want to statically link libc++ to avoid potential downstream conflicts with user binaries.
+  # Static linking flags are order sensitive and this must come after all the C++ that uses it.
+  # More specifically, this must come after the libicu link flags, though if other C++ libraries are linked, this must always be at the end.
+  # Because the order is tricky, there wasn't a very natural place for this block of code, so this seemed like the best place.
+  # Note that this must be linked in the target/client library, not the host compiler.
+  if("${SWIFTLIB_SINGLE_SDK}" STREQUAL "ANDROID")
+    list(APPEND SWIFTLIB_SINGLE_PRIVATE_LINK_LIBRARIES
+      "-lc++_static")
+  endif()
+
+
   # Set compilation and link flags.
   set_property(TARGET "${target}" APPEND_STRING PROPERTY
       COMPILE_FLAGS " ${c_compile_flags}")

cmake/modules/FindICU.cmake

@@ -1,5 +1,25 @@
 # Find libicu's libraries
 
+# For the given usage:
+# find_package(ICU REQUIRED COMPONENTS uc i18n data)
+#
+# This will set the following variables:
+# ICU_I18N_INCLUDE_DIR
+# ICU_UC_INCLUDE_DIR
+# ICU_DATA_INCLUDE_DIR
+# ICU_I18N_LIBRARY
+# ICU_UC_LIBRARY
+# ICU_DATA_LIBRARY
+#
+# Additionally, this module supports specifying ICU_ROOT,
+# following the convention laid out by a more popular and rigorous FindICU.cmake 
+# Two different variations are supported:
+# (1) Setting an explicit value in CMake, e.g. cmake -DICU_ROOT=/path/to/icu_root
+# (2) Setting an environmental variable called ICU_ROOT.
+# The explicit setting takes precedent over the environmental variable.
+# Both take precedent over the standard system locations.
+# This is useful when you need to replace your version of ICU (e.g. different version, supply a static library version)
+
 include(FindPackageHandleStandardArgs)
 
 find_package(PkgConfig)
@@ -11,18 +31,21 @@ foreach(MODULE ${ICU_FIND_COMPONENTS})
   list(APPEND ICU_REQUIRED 
     ICU_${MODULE}_INCLUDE_DIR ICU_${MODULE}_LIBRARIES)
 
-  pkg_check_modules(PC_ICU_${MODULE} QUIET icu-${module})
-  if(${PC_ICU_${MODULE}_FOUND})
-    set(ICU_${MODULE}_DEFINITIONS ${PC_ICU_${MODULE}_CFLAGS_OTHER})
-
-    find_path(ICU_${MODULE}_INCLUDE_DIR unicode
-      HINTS ${PC_ICU_${MODULE}_INCLUDEDIR} ${PC_ICU_${MODULE}_INCLUDE_DIRS})
-    set(ICU_${MODULE}_INCLUDE_DIR ${ICU_${MODULE}_INCLUDE_DIR})
-
-    find_library(ICU_${MODULE}_LIBRARY NAMES icu${module}
-      HINTS ${PC_ICU_${MODULE}_LIBDIR} ${PC_ICU_${MODULE}_LIBRARY_DIRS})
-    set(ICU_${MODULE}_LIBRARIES ${ICU_${MODULE}_LIBRARY})
-  endif()
+  # We are not using pkg-config because some systems do not ship with one for ICU (e.g. Ubuntu 12.04, Windows?)
+  # and not all systems even provide libICU so there may not be a pkg-config that is availble depending how it was built/installed.
+  # Cross-compiling is another potential pitfall.
+  # CMake's built in find_ mechanisms are generally more than sufficient and tend to handle the above issues better.
+  # Also the original pkg-config code could not handle the libicudata case.
+
+  find_path(ICU_${MODULE}_INCLUDE_DIR unicode/utypes.h
+    HINTS ${ICU_ROOT} $ENV{ICU_ROOT}
+    PATH_SUFFIXES include)
+  set(ICU_${MODULE}_INCLUDE_DIR ${ICU_${MODULE}_INCLUDE_DIR})
+
+  find_library(ICU_${MODULE}_LIBRARY NAMES icu${module}
+    HINTS ${ICU_ROOT} $ENV{ICU_ROOT}
+    PATH_SUFFIXES lib)
+  set(ICU_${MODULE}_LIBRARIES ${ICU_${MODULE}_LIBRARY})
 endforeach()
 
 if(NOT "${SWIFT_ANDROID_ICU_UC_INCLUDE}" STREQUAL "")
@@ -31,6 +54,9 @@ endif()
 if(NOT "${SWIFT_ANDROID_ICU_I18N_INCLUDE}" STREQUAL "")
   set(ICU_I18N_INCLUDE_DIR "${SWIFT_ANDROID_ICU_I18N_INCLUDE}")
 endif()
+if(NOT "${SWIFT_ANDROID_ICU_DATA_INCLUDE}" STREQUAL "")
+  set(ICU_DATA_INCLUDE_DIR "${SWIFT_ANDROID_ICU_DATA_INCLUDE}")
+endif()
 
 find_package_handle_standard_args(ICU DEFAULT_MSG ${ICU_REQUIRED})
 mark_as_advanced(${ICU_REQUIRED})

stdlib/public/core/CMakeLists.txt

@@ -151,9 +151,18 @@ else()
   #set(LINK_FLAGS
   #  -Wl,--whole-archive swiftRuntime -Wl,--no-whole-archive)
   list(APPEND swift_core_private_link_libraries swiftRuntime swiftStdlibStubs)
-  find_package(ICU REQUIRED COMPONENTS uc i18n)
+  find_package(ICU REQUIRED COMPONENTS uc i18n data)
+  # If linking against static library versions of ICU, the order matters.
+  # i18N needs uc and data
+  # uc needs data
+  # We need to link against i18n and uc explicitly for dynamic linking.
+  # We also need to link against data for static libraries.
+  # TODO: Detect if the library is static and conditionally link.
+  # NOTE: If ICU is built with data archive, then the data library is a stub.
+  # (This explicit link is harmless in the dynamic library case.)
+  # NOTE: If ICU is built with data archive, then the data library still exists as a stub.
   list(APPEND swift_core_private_link_libraries
-      ${ICU_UC_LIBRARY} ${ICU_I18N_LIBRARY})
+      ${ICU_I18N_LIBRARY} ${ICU_UC_LIBRARY} ${ICU_DATA_LIBRARY})
 endif()
 
 if(CMAKE_SYSTEM_NAME STREQUAL "Linux" AND NOT "${SWIFT_PRIMARY_VARIANT_SDK}" STREQUAL "ANDROID")

stdlib/public/stubs/CMakeLists.txt

@@ -14,9 +14,9 @@ else()
   set(swift_stubs_unicode_normalization_sources
       UnicodeNormalization.cpp)
   set(swift_stubs_link_libraries
-      ${ICU_UC_LIBRARY} ${ICU_I18N_LIBRARY})
+      ${ICU_UC_LIBRARY} ${ICU_I18N_LIBRARY} ${ICU_DATA_LIBRARY})
   include_directories(
-      ${ICU_UC_INCLUDE_DIR} ${ICU_I18N_INCLUDE_DIR})
+      ${ICU_UC_INCLUDE_DIR} ${ICU_I18N_INCLUDE_DIR} ${ICU_DATA_INCLUDE_DIR})
 endif()
 
 add_swift_library(swiftStdlibStubs IS_STDLIB IS_STDLIB_CORE

utils/build-script

@@ -624,6 +624,14 @@ the number of parallel build jobs to use""",
         "--android-icu-i18n-include",
         help="Path to a directory containing headers libicui18n",
         metavar="PATH")
+    android_group.add_argument(
+        "--android-icu-data",
+        help="Path to a directory containing libicudata.so",
+        metavar="PATH")
+    android_group.add_argument(
+        "--android-icu-data-include",
+        help="Path to a directory containing headers libicudata",
+        metavar="PATH")
     android_group.add_argument(
         "--android-deploy-device-path",
         help="Path on an Android device to which built Swift stdlib products "
@@ -669,14 +677,17 @@ the number of parallel build jobs to use""",
     if args.android:
         if args.android_ndk is None or \
                 args.android_ndk_version is None or \
+                args.android_icu_data is None or \
+                args.android_icu_data_include is None or \
                 args.android_icu_uc is None or \
                 args.android_icu_uc_include is None or \
                 args.android_icu_i18n is None or \
                 args.android_icu_i18n_include is None:
             print_with_argv0("When building for Android, --android-ndk, "
                              "--android-ndk-version, --android-icu-uc, "
                              "--android-icu-uc-include, --android-icu-i18n, "
-                             "and --android-icu-i18n-include must be "
+                             "--android-icu-i18n-include, --android-icu-data, "
+                             "and --android-icu-data-include must be "
                              "specified.")
             return 1
         # FIXME: lib/Driver/ToolChains.cpp depends on this environment variable
@@ -999,6 +1010,8 @@ the number of parallel build jobs to use""",
             "--android-icu-uc-include", args.android_icu_uc_include,
             "--android-icu-i18n", args.android_icu_i18n,
             "--android-icu-i18n-include", args.android_icu_i18n_include,
+            "--android-icu-data", args.android_icu_data,
+            "--android-icu-data-include", args.android_icu_data_include,
         ]
         if args.android_deploy_device_path:
             build_script_impl_args += [
@@ -1025,6 +1038,8 @@ the number of parallel build jobs to use""",
             "--android-icu-uc-include", args.android_icu_uc_include,
             "--android-icu-i18n", args.android_icu_i18n,
             "--android-icu-i18n-include", args.android_icu_i18n_include,
+            "--android-icu-data", args.android_icu_data,
+            "--android-icu-data-include", args.android_icu_data_include,
         ]
     build_script_impl_args += build_script_impl_inferred_args
 

utils/build-script-impl

@@ -212,6 +212,8 @@ KNOWN_SETTINGS=(
     android-icu-uc-include             ""        "Path to a directory containing headers for libicuuc"
     android-icu-i18n                   ""        "Path to a directory containing libicui18n.so"
     android-icu-i18n-include           ""        "Path to a directory containing headers libicui18n"
+    android-icu-data                   ""        "Path to a directory containing libicudata.so"
+    android-icu-data-include           ""        "Path to a directory containing headers libicudata"
     android-deploy-device-path         ""        "Path on an Android device to which built Swift stdlib products will be deployed"
     export-compile-commands            ""        "set to generate JSON compilation databases for each build product"
 )
@@ -1566,6 +1568,8 @@ for deployment_target in "${HOST_TARGET}" "${CROSS_TOOLS_DEPLOYMENT_TARGETS[@]}"
                         -DSWIFT_ANDROID_ICU_UC_INCLUDE="${ANDROID_ICU_UC_INCLUDE}"
                         -DSWIFT_ANDROID_ICU_I18N="${ANDROID_ICU_I18N}"
                         -DSWIFT_ANDROID_ICU_I18N_INCLUDE="${ANDROID_ICU_I18N_INCLUDE}"
+                        -DSWIFT_ANDROID_ICU_DATA="${ANDROID_ICU_DATA}"
+                        -DSWIFT_ANDROID_ICU_DATA_INCLUDE="${ANDROID_ICU_DATA_INCLUDE}"
                     )
                 fi