From 6e002b6d8019ea215b84f18f4c7d99de14141b50 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sat, 13 Jan 2024 15:16:47 -0600 Subject: [PATCH] Use cfg directly instead of going through features Features should be for user-specifiable build configurations but our dynamic, target-based conditional compilation is something else. --- Cargo.toml | 1 - build.rs | 18 +++++++++++------- src/env_dispatch.rs | 2 +- src/fork_exec/postfork.rs | 2 +- src/history.rs | 4 ++-- src/locale.rs | 4 ++-- src/signal.rs | 4 ++-- src/wutil/gettext.rs | 4 ++-- 8 files changed, 21 insertions(+), 18 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index df213a9f0..a3a9b1e1b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -66,7 +66,6 @@ benchmark = [] # The following features are auto-detected by the build-script and should not be enabled manually. asan = [] -bsd = [] [lints] rust.non_camel_case_types = "allow" diff --git a/build.rs b/build.rs index 9b06ab72d..f94702ef8 100644 --- a/build.rs +++ b/build.rs @@ -75,19 +75,23 @@ fn main() { let mut detector = Target::new_from(build).unwrap(); // Keep verbose mode on until we've ironed out rust build script stuff detector.set_verbose(true); - detect_features(detector); + detect_cfgs(detector); } -/// Dynamically enables certain features at build-time, without their having to be explicitly -/// enabled in the `cargo build --features xxx` invocation. +/// Check target system support for certain functionality dynamically when the build is invoked, +/// without their having to be explicitly enabled in the `cargo build --features xxx` invocation. +/// +/// We are using [`rsconf::enable_cfg()`] instead of [`rsconf::enable_feature()`] as rust features +/// should be used for things that a user can/would reasonably enable or disable to tweak or coerce +/// behavior, but here we are testing for whether or not things are supported altogether. /// /// This can be used to enable features that we check for and conditionally compile according to in /// our own codebase, but [can't be used to pull in dependencies](0) even if they're gated (in /// `Cargo.toml`) behind a feature we just enabled. /// /// [0]: https://github.com/rust-lang/cargo/issues/5499 -fn detect_features(target: Target) { - for (feature, handler) in [ +fn detect_cfgs(target: Target) { + for (name, handler) in [ // Ignore the first entry, it just sets up the type inference. Model new entries after the // second line. ( @@ -100,8 +104,8 @@ fn detect_features(target: Target) { ("localeconv_l", &|target| Ok(target.has_symbol_in::("localeconv_l", &[]))), ] { match handler(&target) { - Err(e) => rsconf::warn!("{}: {}", feature, e), - Ok(true) => rsconf::enable_feature(feature), + Err(e) => rsconf::warn!("{}: {}", name, e), + Ok(true) => rsconf::enable_cfg(name), Ok(false) => (), } } diff --git a/src/env_dispatch.rs b/src/env_dispatch.rs index 8b166566b..670253227 100644 --- a/src/env_dispatch.rs +++ b/src/env_dispatch.rs @@ -779,7 +779,7 @@ fn init_locale(vars: &EnvStack) { new_msg_locale.to_string_lossy() ); - #[cfg(feature = "gettext")] + #[cfg(gettext)] { if old_msg_locale.as_c_str() != new_msg_locale { // Make change known to GNU gettext. diff --git a/src/fork_exec/postfork.rs b/src/fork_exec/postfork.rs index bd4320acd..96e8a097f 100644 --- a/src/fork_exec/postfork.rs +++ b/src/fork_exec/postfork.rs @@ -133,7 +133,7 @@ pub fn execute_setpgid(pid: pid_t, pgroup: pid_t, is_parent: bool) -> i32 { // 12.2) does not consider a child that has already forked, exec'd, and exited to "exist" // and returns ESRCH (process not found) instead of EACCES (child has called exec). // See https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=251227 - #[cfg(any(feature = "bsd", target_os = "macos"))] + #[cfg(any(bsd, target_os = "macos"))] if err == libc::ESRCH && is_parent { // Handle this just like we would EACCES above, as we're virtually certain that // setpgid(2) was called against a process that was at least at one point in time a diff --git a/src/history.rs b/src/history.rs index e93a5927e..cf2470bf9 100644 --- a/src/history.rs +++ b/src/history.rs @@ -1404,7 +1404,7 @@ fn format_history_record( const max_tstamp_length: usize = 100; let mut timestamp_str = [0_u8; max_tstamp_length]; // The libc crate fails to declare strftime on BSD. - #[cfg(feature = "bsd")] + #[cfg(bsd)] extern "C" { fn strftime( buf: *mut libc::c_char, @@ -1413,7 +1413,7 @@ fn strftime( timeptr: *const libc::tm, ) -> usize; } - #[cfg(not(feature = "bsd"))] + #[cfg(not(bsd))] use libc::strftime; if unsafe { strftime( diff --git a/src/locale.rs b/src/locale.rs index 7ec5d0067..17236b452 100644 --- a/src/locale.rs +++ b/src/locale.rs @@ -59,7 +59,7 @@ unsafe fn lconv_to_locale(lconv: &libc::lconv) -> Locale { } /// Read the numeric locale, or None on any failure. -#[cfg(feature = "localeconv_l")] +#[cfg(localeconv_l)] unsafe fn read_locale() -> Option { extern "C" { fn localeconv_l(loc: libc::locale_t) -> *const libc::lconv; @@ -88,7 +88,7 @@ unsafe fn read_locale() -> Option { result } -#[cfg(not(feature = "localeconv_l"))] +#[cfg(not(localeconv_l))] unsafe fn read_locale() -> Option { // Bleh, we have to go through localeconv, which races with setlocale. // TODO: There has to be a better way to do this. diff --git a/src/signal.rs b/src/signal.rs index b66a2b005..143e858c6 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -389,10 +389,10 @@ const fn new(signal: i32, name: &'static wstr, desc: &'static wstr) -> Self { LookupEntry::new(libc::SIGSYS, L!("SIGSYS"), L!("Bad system call")), LookupEntry::new(libc::SIGIOT, L!("SIGIOT"), L!("Abort (Alias for SIGABRT)")), - #[cfg(any(feature = "bsd", target_os = "macos"))] + #[cfg(any(bsd, target_os = "macos"))] LookupEntry::new(libc::SIGEMT, L!("SIGEMT"), L!("Unused signal")), - #[cfg(any(feature = "bsd", target_os = "macos"))] + #[cfg(any(bsd, target_os = "macos"))] LookupEntry::new(libc::SIGINFO, L!("SIGINFO"), L!("Information request")), #[cfg(target_os = "linux")] diff --git a/src/wutil/gettext.rs b/src/wutil/gettext.rs index 34426004f..c4716a89d 100644 --- a/src/wutil/gettext.rs +++ b/src/wutil/gettext.rs @@ -9,7 +9,7 @@ use errno::{errno, set_errno}; use once_cell::sync::{Lazy, OnceCell}; -#[cfg(feature = "gettext")] +#[cfg(gettext)] mod internal { use libc::c_char; use std::ffi::CStr; @@ -28,7 +28,7 @@ pub fn fish_textdomain(domainname: &CStr) -> *mut c_char { unsafe { textdomain(domainname.as_ptr()) } } } -#[cfg(not(feature = "gettext"))] +#[cfg(not(gettext))] mod internal { use libc::c_char; use std::ffi::CStr;