[5.0] Link to the ICU61 libraries that are built for Linux

[spevans]

• The findICU was not finding the libicu{uc,i18n}swift libraries so
  libFoundation.so was still being linked to the system ICU libraries.

In the master branch, ICU_UC_LIBRARY_DEBUG ICU_UC_LIBRARY_RELEASE ICU_I18N_LIBRARY_DEBUG and ICU_I18N_LIBRARY_RELEASE are passed in by build-swift-impl, directly set them in CMakeLists.txt here.

[spevans]

@swift-ci test

[spevans]

cc @ianpartridge @compnerd @millenomi

[ianpartridge]

For background, I spotted this problem when trying to create Swift 5 Docker images on Debian stretch, which ships ICU 57. It wouldn't work because libFoundation wrongly links against 55, instead of the ICU packaged in the toolchain:

$ readelf -aW /usr/lib/swift/linux/libFoundation.so | grep libicu
 0x0000000000000001 (NEEDED)             Shared library: [libicuuc.so.55]
 0x0000000000000001 (NEEDED)             Shared library: [libicui18n.so.55]

[ianpartridge]

cc: @weissi you may also be interested in this.

[spevans]

Im surprised the tests still ran when building the toolchains especially for 14.04

Update: The tests work because this finds the correct header files so generates calls to the correct (renamed) v61 functions and links to the symbols because libswiftCore.so is linked to the correct libicuswift.

[weissi]

And @kevints

[ianpartridge]

What's the process for merging 5.0 SCLF changes at this stage? This looks important to me - @parkera?

[weissi]

Probably @millenomi and/or @parkera are the release managers for SCLF 5.0 and need to approve this. Not 100% sure what the SCLF process here is though.

[compnerd]

Something seems wrong. The build variant should automatically be filtered through in CMake. Setting the variants explicitly seems incorrect.

[spevans]

@compnerd It works on master because they are set in utils/build-script-impl

[from git diff origin/swift-5.0-branch..origin/master -- utils/build-script-impl]

@@ -2611,13 +2659,18 @@ for host in "${ALL_HOSTS[@]}"; do
 
                 if [[ ! "${SKIP_BUILD_LIBICU}" ]] ; then
                     ICU_ROOT=$(build_directory ${host} libicu)/tmp_install
+                    ICU_LIBDIR="$(build_directory ${host} swift)/lib/swift/${SWIFT_HOST_VARIANT}/${SWIFT_HOST_VARIANT_ARCH}"
                     LIBICU_BUILD_ARGS=(
                         -DICU_ROOT:PATH=${ICU_ROOT}
                         -DICU_INCLUDE_DIR:PATH=${ICU_ROOT}/include
                         -DICU_UC_LIBRARIES:FILEPATH=${ICU_LIBDIR}/libicuucswift.so
                         -DICU_UC_LIBRARY:FILEPATH=${ICU_LIBDIR}/libicuucswift.so
+                        -DICU_UC_LIBRARY_DEBUG:FILEPATH=${ICU_LIBDIR}/libicuucswift.so
+                        -DICU_UC_LIBRARY_RELEASE:FILEPATH=${ICU_LIBDIR}/libicuucswift.so
                         -DICU_I18N_LIBRARIES:FILEPATH=${ICU_LIBDIR}/libicui18nswift.so
                         -DICU_I18N_LIBRARY:FILEPATH=${ICU_LIBDIR}/libicui18nswift.so
+                        -DICU_I18N_LIBRARY_DEBUG:FILEPATH=${ICU_LIBDIR}/libicui18nswift.so
+                        -DICU_I18N_LIBRARY_RELEASE:FILEPATH=${ICU_LIBDIR}/libicui18nswift.so
                     )
                 else
                     LIBICU_BUILD_ARGS=()

So this work around is just to fix it without trying to get a patch into the main repo.

[compnerd]

I think that it's better to back port that change, since that enables more of the cross-compilation even though its in the main tree. It shouldn't impact Darwin, and is a build time thing, so its low risk.

[ianpartridge]

@millenomi what do you think? Clock is ticking...

[millenomi]

Sorry for the delay. Both me and @parkera were out of office.

Since this is a Foundation-only concern, I am happy taking a SCF patch that doesn’t get ported back to master.

LGTM.

[weissi]

@millenomi / @parkera / @spevans don't we need this for 5.1 and master too?

[spevans]

@weissi I checked the latest 5.1 snapshot and It doesnt have this issue. There have been quite a large number of cmake changes so this fix is probably out of date for 5.1.