Skip to content

Commit

Permalink
Use cfg directly instead of going through features
Browse files Browse the repository at this point in the history
Features should be for user-specifiable build configurations but our dynamic,
target-based conditional compilation is something else.
  • Loading branch information
mqudsi committed Jan 13, 2024
1 parent 4f8265d commit 6e002b6
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 18 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
18 changes: 11 additions & 7 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
(
Expand All @@ -100,8 +104,8 @@ fn detect_features(target: Target) {
("localeconv_l", &|target| Ok(target.has_symbol_in::<String>("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) => (),
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/env_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/fork_exec/postfork.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -1413,7 +1413,7 @@ fn format_history_record(
timeptr: *const libc::tm,
) -> usize;
}
#[cfg(not(feature = "bsd"))]
#[cfg(not(bsd))]
use libc::strftime;
if unsafe {
strftime(
Expand Down
4 changes: 2 additions & 2 deletions src/locale.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Locale> {
extern "C" {
fn localeconv_l(loc: libc::locale_t) -> *const libc::lconv;
Expand Down Expand Up @@ -88,7 +88,7 @@ unsafe fn read_locale() -> Option<Locale> {
result
}

#[cfg(not(feature = "localeconv_l"))]
#[cfg(not(localeconv_l))]
unsafe fn read_locale() -> Option<Locale> {
// Bleh, we have to go through localeconv, which races with setlocale.
// TODO: There has to be a better way to do this.
Expand Down
4 changes: 2 additions & 2 deletions src/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,10 +389,10 @@ const SIGNAL_TABLE : &[LookupEntry] = &[
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")]
Expand Down
4 changes: 2 additions & 2 deletions src/wutil/gettext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::wchar::prelude::*;
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;
Expand All @@ -28,7 +28,7 @@ mod internal {
unsafe { textdomain(domainname.as_ptr()) }
}
}
#[cfg(not(feature = "gettext"))]
#[cfg(not(gettext))]
mod internal {
use libc::c_char;
use std::ffi::CStr;
Expand Down

0 comments on commit 6e002b6

Please sign in to comment.