From 1266c67718e14532c6a371d10aaca60ca3d5e5f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20F=C3=A9ron?= Date: Mon, 23 Oct 2023 18:32:09 +0200 Subject: [PATCH 1/5] Move linking config to build.rs --- README.md | 1 + rust_icu_sys/build.rs | 27 ++++++++++++++++++++++++++- rust_icu_sys/src/lib.rs | 22 ++-------------------- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 53df1317..9673d470 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,7 @@ Feature | Default? | Description `renaming` | Yes | If set, ICU bindings are generated with version numbers appended. This is called "renaming" in ICU, and is normally needed only when linking against specific ICU version is required, for example to work around having to link different ICU versions. See [the ICU documentation](https://unicode-org.github.io/icu/userguide/icu/design.html) for a discussion of renaming. **This feature MUST be used when `bindgen` is NOT used.** `icu_config` | Yes | If set, the binary icu-config will be used to configure the library. Turn this feature off if you do not want `build.rs` to try to autodetect the build environment. You will want to skip this feature if your build environment configures ICU in a different way. **This feature is only meaningful when `bindgen` feature is used; otherwise it has no effect.** `icu_version_in_env` | No | If set, ICU bindings are made for the ICU version specified in the environment variable `RUST_ICU_MAJOR_VERSION_NUMBER`, which is made available to cargo at build time. See section below for details on how to use this feature. **This feature is only meaningful when `bindgen` feature is NOT used; otherwise it has no effect.** +`static` | No | If set, link ICU libraries statically (and the standard C++ dynamically). You can use `RUST_ICU_LINK_SEARCH_DIR` to add an extra path to the search path if you have a build of ICU in a non-standard directory. # Prerequisites diff --git a/rust_icu_sys/build.rs b/rust_icu_sys/build.rs index e81e68fe..2922387b 100644 --- a/rust_icu_sys/build.rs +++ b/rust_icu_sys/build.rs @@ -478,6 +478,24 @@ macro_rules! versioned_function { } } +fn rustc_link_libs() { + if cfg!(feature = "static") { + println!("cargo:rustc-link-lib=static=icuuc"); + println!("cargo:rustc-link-lib=static=icui18n"); + println!("cargo:rustc-link-lib=static:+whole-archive,-bundle=icudata"); + // On systems such as macOS, libc++ is the default library + if cfg!(target_vendor = "apple") { + println!("cargo:rustc-link-lib=dylib=c++"); + } else { + println!("cargo:rustc-link-lib=dylib=stdc++"); + } + } else { + println!("cargo:rustc-link-lib=dylib=icuuc"); + println!("cargo:rustc-link-lib=dylib=icui18n"); + println!("cargo:rustc-link-lib=dylib=icudata"); + } +} + #[cfg(feature = "use-bindgen")] fn main() -> Result<(), anyhow::Error> { std::env::set_var("RUST_BACKTRACE", "full"); @@ -486,10 +504,17 @@ fn main() -> Result<(), anyhow::Error> { return Ok(()); } inner::icu_config_autodetect()?; + rustc_link_libs(); println!("done:true"); Ok(()) } /// No-op if use-bindgen is disabled. #[cfg(not(feature = "use-bindgen"))] -fn main() {} +fn main() { + // can be used to provide an extra path to find libicuuc, libicui18n and libicudata + if let Ok(lib_dir) = std::env::var("RUST_ICU_LINK_SEARCH_DIR") { + println!("cargo:rustc-link-search=native={}", lib_dir); + } + rustc_link_libs(); +} diff --git a/rust_icu_sys/src/lib.rs b/rust_icu_sys/src/lib.rs index d4c64fc2..9e56f01d 100644 --- a/rust_icu_sys/src/lib.rs +++ b/rust_icu_sys/src/lib.rs @@ -23,7 +23,7 @@ non_upper_case_globals, unused_imports, rustdoc::bare_urls, - deref_nullptr, + deref_nullptr )] #[cfg(all(feature = "icu_version_in_env", feature = "icu_config"))] @@ -80,24 +80,6 @@ impl std::fmt::Display for UErrorCode { extern crate libc; -// A "fake" extern used to express link preferences. The libraries mentioned -// below will be converted to "-l" flags to the linker. -#[cfg_attr(not(feature = "static"), link(name = "icui18n", kind = "dylib"))] -#[cfg_attr(not(feature = "static"), link(name = "icuuc", kind = "dylib"))] -#[cfg_attr(feature = "static", link(name = "icui18n", kind = "static"))] -#[cfg_attr(feature = "static", link(name = "icuuc", kind = "static"))] -#[cfg_attr(feature = "static", link(name = "icudata", kind = "static"))] -// On systems such as macOS, libc++ is the default library -#[cfg_attr( - all(target_vendor = "apple", feature = "static"), - link(name = "c++", kind = "dylib") -)] -#[cfg_attr( - not(all(target_vendor = "apple", feature = "static")), - link(name = "stdc++", kind = "dylib") -)] -extern "C" {} - impl From for UCharCategory { fn from(value: i8) -> Self { match value { @@ -132,7 +114,7 @@ impl From for UCharCategory { 28 => UCharCategory::U_INITIAL_PUNCTUATION, 29 => UCharCategory::U_FINAL_PUNCTUATION, 30 => UCharCategory::U_CHAR_CATEGORY_COUNT, - _ => { + _ => { panic!("could not convert: {}", value); } } From abe1cc965e07700b8f7ad96e2aa7a2974e7465c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20F=C3=A9ron?= Date: Tue, 24 Oct 2023 10:51:48 +0200 Subject: [PATCH 2/5] Add mention of static feature in README and macos CI job --- .github/workflows/test.yml | 11 +++++++++++ Makefile | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4491ae09..644148e4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -46,3 +46,14 @@ jobs: - uses: actions/checkout@v2 - name: 'Test static-bindgen' run: 'make static-bindgen' + test-static-linking: + runs-on: ${{ matrix.runs-on }} + strategy: + matrix: + runs-on: [ubuntu-latest, macos-latest] + steps: + - uses: actions/checkout@v2 + - if: matrix.runs-on == 'macos-latest' + run: make macos-test + - if: matrix.runs-on == 'ubuntu-latest' + run: make test \ No newline at end of file diff --git a/Makefile b/Makefile index 8e8add9e..0a8c5b7c 100644 --- a/Makefile +++ b/Makefile @@ -73,6 +73,13 @@ docker-test: ${DOCKER_REPO}/${DOCKER_TEST_ENV}:${USED_BUILDENV_VERSION} .PHONY: docker-test +# Test with the homebrew version of icu4c on macOS with static linking (the default way of linking for distribution on Apple platforms) +macos-test: + brew install icu4c + RUST_ICU_LINK_SEARCH_DIR=$(brew --prefix)/opt/icu4c/lib \ + cargo test --no-default-features --features=icu_version_64_plus,icu_version_67_plus,icu_version_68_plus,icu_version_in_env,renaming,static +.PHONY: macos-test + # Refreshes the static bindgen output (contents of ./rust_icu_sys/bindgen) based # on the currently present ICU versions in the test environment. # From c49693f22533663514a37c6d5ff68b4670171a55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20F=C3=A9ron?= Date: Tue, 24 Oct 2023 11:20:36 +0200 Subject: [PATCH 3/5] Fix macos-test make recipe --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 0a8c5b7c..2ac5bf5c 100644 --- a/Makefile +++ b/Makefile @@ -77,6 +77,7 @@ docker-test: macos-test: brew install icu4c RUST_ICU_LINK_SEARCH_DIR=$(brew --prefix)/opt/icu4c/lib \ + RUST_ICU_MAJOR_VERSION_NUMBER=${RUST_ICU_MAJOR_VERSION_NUMBER} \ cargo test --no-default-features --features=icu_version_64_plus,icu_version_67_plus,icu_version_68_plus,icu_version_in_env,renaming,static .PHONY: macos-test From deb0da74b3ba9f4f8b1e7a90de65f0a80548023e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20F=C3=A9ron?= Date: Tue, 24 Oct 2023 11:22:26 +0200 Subject: [PATCH 4/5] Cancel previously running workflows on the same branch --- .github/workflows/test.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 644148e4..8629cbb9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,6 +6,9 @@ on: branches: [ main ] schedule: - cron: "43 7 * * 0" +concurrency: + group: ${{ github.ref }} + cancel-in-progress: true jobs: test-default-features: From d3144ab99ec5b289db09c9054b4d3885cf6dfc73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20F=C3=A9ron?= Date: Tue, 24 Oct 2023 11:32:02 +0200 Subject: [PATCH 5/5] Fix macOS makefile search path --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2ac5bf5c..79df80df 100644 --- a/Makefile +++ b/Makefile @@ -76,7 +76,7 @@ docker-test: # Test with the homebrew version of icu4c on macOS with static linking (the default way of linking for distribution on Apple platforms) macos-test: brew install icu4c - RUST_ICU_LINK_SEARCH_DIR=$(brew --prefix)/opt/icu4c/lib \ + RUST_ICU_LINK_SEARCH_DIR="$(shell brew --prefix)/opt/icu4c/lib" \ RUST_ICU_MAJOR_VERSION_NUMBER=${RUST_ICU_MAJOR_VERSION_NUMBER} \ cargo test --no-default-features --features=icu_version_64_plus,icu_version_67_plus,icu_version_68_plus,icu_version_in_env,renaming,static .PHONY: macos-test