From b17124d8d23fc9c438b49f5b78593d4fb93278fb Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 16 May 2023 13:02:22 -0500 Subject: [PATCH] Add rsconf build system and check for gettext symbols This is more complicated than it needs to be thanks to the presence of CMake and the C++ ffi in the picture. rsconf can correctly detect the required libraries and instruct rustc to link against them, but since we generate a static rust library and have CMake link it against the C++ binaries, we are still at the mercy of CMake picking up the symbols we want. Unfortunately, we could detect the gettext symbols but discover at runtime that they weren't linked in because CMake was compiled with `-DWITH_GETTEXT=0` or similar (as the macOS CI runner does). This means we also need to pass state between CMake and our build script to communicate which CMake options were enabled. --- cmake/Rust.cmake | 9 +++++ fish-rust/Cargo.lock | 9 +++++ fish-rust/Cargo.toml | 1 + fish-rust/build.rs | 81 +++++++++++++++++++++++++++++++++++++------- 4 files changed, 88 insertions(+), 12 deletions(-) diff --git a/cmake/Rust.cmake b/cmake/Rust.cmake index bd836fed6..3ec5482e6 100644 --- a/cmake/Rust.cmake +++ b/cmake/Rust.cmake @@ -51,12 +51,21 @@ else() corrosion_set_hostbuild(${fish_rust_target}) endif() +# Temporary hack to propogate CMake flags/options to build.rs. We need to get CMake to evaluate the +# truthiness of the strings if they are set. +set(CMAKE_WITH_GETTEXT "1") +if(DEFINED WITH_GETTEXT AND NOT "${WITH_GETTEXT}") + set(CMAKE_WITH_GETTEXT "0") +endif() + # Tell Cargo where our build directory is so it can find config.h. corrosion_set_env_vars(${fish_rust_target} "FISH_BUILD_DIR=${CMAKE_BINARY_DIR}" "FISH_AUTOCXX_GEN_DIR=${fish_autocxx_gen_dir}" "FISH_RUST_TARGET_DIR=${rust_target_dir}" "PREFIX=${CMAKE_INSTALL_PREFIX}" + # Temporary hack to propogate CMake flags/options to build.rs. + "CMAKE_WITH_GETTEXT=${CMAKE_WITH_GETTEXT}" ) target_include_directories(${fish_rust_target} INTERFACE diff --git a/fish-rust/Cargo.lock b/fish-rust/Cargo.lock index 397b94178..6440fa0fa 100644 --- a/fish-rust/Cargo.lock +++ b/fish-rust/Cargo.lock @@ -356,6 +356,7 @@ dependencies = [ "pcre2", "printf-compat", "rand", + "rsconf", "unixstring", "widestring", "widestring-suffix", @@ -816,6 +817,14 @@ version = "0.6.29" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" +[[package]] +name = "rsconf" +version = "0.1.0" +source = "git+https://github.com/mqudsi/rsconf?branch=master#5966dd64796528e79e0dc9ba61b1dac679640273" +dependencies = [ + "cc", +] + [[package]] name = "rustc-hash" version = "1.1.0" diff --git a/fish-rust/Cargo.toml b/fish-rust/Cargo.toml index db042e9cd..317f6b174 100644 --- a/fish-rust/Cargo.toml +++ b/fish-rust/Cargo.toml @@ -32,6 +32,7 @@ autocxx-build = "0.23.1" cc = { git = "https://github.com/mqudsi/cc-rs", branch = "fish" } cxx-build = { git = "https://github.com/fish-shell/cxx", branch = "fish" } cxx-gen = { git = "https://github.com/fish-shell/cxx", branch = "fish" } +rsconf = { git = "https://github.com/mqudsi/rsconf", branch = "master" } [lib] crate-type = ["staticlib"] diff --git a/fish-rust/build.rs b/fish-rust/build.rs index eeb809753..cc661871a 100644 --- a/fish-rust/build.rs +++ b/fish-rust/build.rs @@ -1,3 +1,4 @@ +use rsconf::{LinkType, Target}; use std::error::Error; fn main() { @@ -19,7 +20,19 @@ fn main() { let autocxx_gen_dir = std::env::var("FISH_AUTOCXX_GEN_DIR") .unwrap_or(format!("{}/{}", fish_build_dir, "fish-autocxx-gen/")); - detect_features(); + let mut build = cc::Build::new(); + // Add to the default library search path + build.flag_if_supported("-L/usr/local/lib/"); + rsconf::add_library_search_path("/usr/local/lib"); + let mut detector = Target::new_from(build).unwrap(); + // Keep verbose mode on until we've ironed out rust build script stuff + // Note that if autocxx fails to compile any rust code, you'll see the full and unredacted + // stdout/stderr output, which will include things that LOOK LIKE compilation errors as rsconf + // tries to build various test files to try and figure out which libraries and symbols are + // available. IGNORE THESE and scroll to the very bottom of the build script output, past all + // these errors, to see the actual issue. + detector.set_verbose(true); + detect_features(detector); // Emit cxx junk. // This allows "Rust to be used from C++" @@ -80,9 +93,7 @@ fn main() { b.flag_if_supported("-std=c++11") .flag("-Wno-comment") .compile("fish-rust-autocxx"); - for file in source_files { - println!("cargo:rerun-if-changed={file}"); - } + rsconf::rebuild_if_paths_changed(&source_files); } /// Dynamically enables certain features at build-time, without their having to be explicitly @@ -93,19 +104,20 @@ fn main() { /// `Cargo.toml`) behind a feature we just enabled. /// /// [0]: https://github.com/rust-lang/cargo/issues/5499 -fn detect_features() { - for (feature, detector) in [ - // Ignore the first line, it just sets up the type inference. Model new entries after the +fn detect_features(target: Target) { + for (feature, handler) in [ + // Ignore the first entry, it just sets up the type inference. Model new entries after the // second line. ( "", - &(|| Ok(false)) as &dyn Fn() -> Result>, + &(|_: &Target| Ok(false)) as &dyn Fn(&Target) -> Result>, ), ("bsd", &detect_bsd), + ("gettext", &have_gettext), ] { - match detector() { - Err(e) => eprintln!("ERROR: {feature} detect: {e}"), - Ok(true) => println!("cargo:rustc-cfg=feature=\"{feature}\""), + match handler(&target) { + Err(e) => rsconf::warn!("{}: {}", feature, e), + Ok(true) => rsconf::enable_feature(feature), Ok(false) => (), } } @@ -117,7 +129,7 @@ fn detect_features() { /// Rust offers fine-grained conditional compilation per-os for the popular operating systems, but /// doesn't necessarily include less-popular forks nor does it group them into families more /// specific than "windows" vs "unix" so we can conditionally compile code for BSD systems. -fn detect_bsd() -> Result> { +fn detect_bsd(_: &Target) -> Result> { // Instead of using `uname`, we can inspect the TARGET env variable set by Cargo. This lets us // support cross-compilation scenarios. let mut target = std::env::var("TARGET").unwrap(); @@ -134,3 +146,48 @@ fn detect_bsd() -> Result> { assert!(result, "Target incorrectly detected as not BSD!"); Ok(result) } + +/// Detect libintl/gettext and its needed symbols to enable internationalization/localization +/// support. +fn have_gettext(target: &Target) -> Result> { + // The following script correctly detects and links against gettext, but so long as we are using + // C++ and generate a static library linked into the C++ binary via CMake, we need to account + // for the CMake option WITH_GETTEXT being explicitly disabled. + rsconf::rebuild_if_env_changed("CMAKE_WITH_GETTEXT"); + if let Some(with_gettext) = std::env::var_os("CMAKE_WITH_GETTEXT") { + if with_gettext.eq_ignore_ascii_case("0") { + return Ok(false); + } + } + + // In order for fish to correctly operate, we need some way of notifying libintl to invalidate + // its localizations when the locale environment variables are modified. Without the libintl + // symbol _nl_msg_cat_cntr, we cannot use gettext even if we find it. + let mut libraries = Vec::new(); + let mut found = 0; + let symbols = ["gettext", "_nl_msg_cat_cntr"]; + for symbol in &symbols { + // Historically, libintl was required in order to use gettext() and co, but that + // functionality was subsumed by some versions of libc. + if target.has_symbol_in::<&str>(symbol, &[]) { + // No need to link anything special for this symbol + found += 1; + continue; + } + for library in ["intl", "gettextlib"] { + if target.has_symbol(symbol, library) { + libraries.push(library); + found += 1; + continue; + } + } + } + match found { + 0 => Ok(false), + 1 => Err(format!("gettext found but cannot be used without {}", symbols[1]).into()), + _ => { + rsconf::link_libraries(&libraries, LinkType::Default); + Ok(true) + } + } +}