From 497e5fe21426b2ef0bf2f1097531345c3c1b6c0a Mon Sep 17 00:00:00 2001 From: Ian Ker-Seymer Date: Wed, 12 Jul 2023 02:09:13 -0400 Subject: [PATCH] Make the feature opt-in --- bench/Cargo.toml | 2 +- crates/rb-sys-test-helpers-macros/Cargo.toml | 5 ++- crates/rb-sys-test-helpers/Cargo.toml | 5 ++- crates/rb-sys-tests/Cargo.toml | 5 ++- crates/rb-sys-tests/src/stable_api_test.rs | 2 +- crates/rb-sys/Cargo.toml | 10 ++++- crates/rb-sys/build/features.rs | 15 +++++-- crates/rb-sys/build/main.rs | 7 +++- crates/rb-sys/src/stable_api.rs | 42 +++++++++++++------ .../rust_reverse/ext/rust_reverse/Cargo.toml | 6 ++- .../rust_reverse/ext/rust_reverse/extconf.rb | 2 +- .../ext/rust_reverse/extconf_bare.rb | 4 +- fuzz/Cargo.toml | 2 +- 13 files changed, 78 insertions(+), 29 deletions(-) diff --git a/bench/Cargo.toml b/bench/Cargo.toml index 992dce1a..ceb59091 100644 --- a/bench/Cargo.toml +++ b/bench/Cargo.toml @@ -13,7 +13,7 @@ test = false criterion = "0.5.1" rb-sys-test-helpers = { path = "../crates/rb-sys-test-helpers" } rb-sys = { path = "../crates/rb-sys", features = [ - "stable-api-compiled", + "stable-api-compiled-testing", "link-ruby", "fuzz", ] } diff --git a/crates/rb-sys-test-helpers-macros/Cargo.toml b/crates/rb-sys-test-helpers-macros/Cargo.toml index a28a3587..56d3dc7f 100644 --- a/crates/rb-sys-test-helpers-macros/Cargo.toml +++ b/crates/rb-sys-test-helpers-macros/Cargo.toml @@ -20,4 +20,7 @@ quote = "1.0" [dev-dependencies] rb-sys-test-helpers = { path = "../rb-sys-test-helpers" } -rb-sys = { path = "../rb-sys", features = ["link-ruby", "stable-api-compiled"] } +rb-sys = { path = "../rb-sys", features = [ + "link-ruby", + "stable-api-compiled-testing", +] } diff --git a/crates/rb-sys-test-helpers/Cargo.toml b/crates/rb-sys-test-helpers/Cargo.toml index 3bc295dc..93085e52 100644 --- a/crates/rb-sys-test-helpers/Cargo.toml +++ b/crates/rb-sys-test-helpers/Cargo.toml @@ -14,7 +14,10 @@ bench = false doctest = true [dependencies] -rb-sys = { path = "../rb-sys", features = ["link-ruby", "stable-api-compiled"] } +rb-sys = { path = "../rb-sys", features = [ + "link-ruby", + "stable-api-compiled-testing", +] } rb-sys-test-helpers-macros = { version = "0.1.0", path = "../rb-sys-test-helpers-macros" } [build-dependencies] diff --git a/crates/rb-sys-tests/Cargo.toml b/crates/rb-sys-tests/Cargo.toml index 2cde1f89..5dd261a6 100644 --- a/crates/rb-sys-tests/Cargo.toml +++ b/crates/rb-sys-tests/Cargo.toml @@ -12,7 +12,10 @@ doctest = false doc = false [dependencies] -rb-sys = { path = "../rb-sys", features = ["link-ruby", "stable-api-compiled"] } +rb-sys = { path = "../rb-sys", features = [ + "link-ruby", + "stable-api-compiled-testing", +] } rb-sys-env = { path = "../rb-sys-env" } rb-sys-test-helpers = { path = "../rb-sys-test-helpers" } rusty-fork = "0.3.0" diff --git a/crates/rb-sys-tests/src/stable_api_test.rs b/crates/rb-sys-tests/src/stable_api_test.rs index 9e543942..fe6c05d8 100644 --- a/crates/rb-sys-tests/src/stable_api_test.rs +++ b/crates/rb-sys-tests/src/stable_api_test.rs @@ -11,7 +11,7 @@ macro_rules! parity_test { #[allow(unused)] let rust_result = unsafe { stable_api::get_default().$func(data) }; #[allow(unused_unsafe)] - let compiled_c_result = unsafe { stable_api::get_fallback().$func(data) }; + let compiled_c_result = unsafe { stable_api::get_compiled().$func(data) }; assert_eq!( compiled_c_result, rust_result, diff --git a/crates/rb-sys/Cargo.toml b/crates/rb-sys/Cargo.toml index 4a772b1a..3c7f2be9 100644 --- a/crates/rb-sys/Cargo.toml +++ b/crates/rb-sys/Cargo.toml @@ -18,7 +18,10 @@ rb-sys-build = { version = "0.9.79", path = "../rb-sys-build" } [dev-dependencies] rb-sys-test-helpers = { path = "../rb-sys-test-helpers" } -rb-sys = { path = ".", features = ["link-ruby", "stable-api-compiled"] } +rb-sys = { path = ".", features = [ + "link-ruby", + "stable-api-compiled-fallback", +] } rusty-fork = "0.3.0" [features] @@ -28,7 +31,10 @@ fuzz = [] no-link-ruby = [] ruby-static = [] global-allocator = [] -stable-api-compiled = [] +stable-api-compiled-fallback = [] # Fallback to compiled C API +stable-api-compiled-testing = [ + "stable-api-compiled-fallback", +] # For testing the fallback in rb-sys (internal) ruby-macros = [] # deprecated bindgen-rbimpls = ["rb-sys-build/bindgen-rbimpls"] bindgen-deprecated-types = ["rb-sys-build/bindgen-deprecated-types"] diff --git a/crates/rb-sys/build/features.rs b/crates/rb-sys/build/features.rs index 924b0a51..44a251fc 100644 --- a/crates/rb-sys/build/features.rs +++ b/crates/rb-sys/build/features.rs @@ -19,10 +19,19 @@ pub fn is_global_allocator_enabled(rb_config: &RbConfig) -> bool { } pub fn is_compiled_stable_api_needed(ver: &Version) -> bool { - let needs_rust_impls = MIN_SUPPORTED_STABLE_VERSION > *ver || *ver > LATEST_STABLE_VERSION; - let is_feature_enabled = is_env_variable_defined("CARGO_FEATURE_STABLE_API_COMPILED"); + let needs_c_fallback = MIN_SUPPORTED_STABLE_VERSION > *ver || *ver > LATEST_STABLE_VERSION; + let wants_c_fallback = explicitly_enabled_stable_api_compiled_fallback(); - needs_rust_impls || is_feature_enabled + (needs_c_fallback && wants_c_fallback) || testing_stable_api_compiled_fallback() +} + +pub fn explicitly_enabled_stable_api_compiled_fallback() -> bool { + cfg!(rb_sys_use_stable_api_compiled_fallback) + || is_env_variable_defined("CARGO_FEATURE_STABLE_API_COMPILED_FALLBACK") +} + +pub fn testing_stable_api_compiled_fallback() -> bool { + is_env_variable_defined("CARGO_FEATURE_STABLE_API_COMPILED_TESTING") } pub fn is_gem_enabled() -> bool { diff --git a/crates/rb-sys/build/main.rs b/crates/rb-sys/build/main.rs index 637da4fd..f0cfdf53 100644 --- a/crates/rb-sys/build/main.rs +++ b/crates/rb-sys/build/main.rs @@ -79,8 +79,13 @@ fn main() { ); export_cargo_cfg(&mut rbconfig, &mut cfg_capture_file); + if explicitly_enabled_stable_api_compiled_fallback() { + println!("cargo:rustc-cfg=explicitly_enabled_stable_api_compiled_fallback"); + } + if is_compiled_stable_api_needed(¤t_ruby_version) { c_glue::compile().expect("stable API C glue to compile"); + println!("cargo:rustc-cfg=has_stable_api_compiled"); } if is_link_ruby_enabled() { @@ -143,7 +148,7 @@ fn export_cargo_cfg(rbconfig: &mut RbConfig, cap: &mut File) { rustc_cfg(rbconfig, "ruby_api_version", "RUBY_API_VERSION"); if Version::current(rbconfig) <= LATEST_STABLE_VERSION { - println!("cargo:rustc-cfg=ruby_api_stable"); + println!("cargo:rustc-cfg=current_ruby_is_stable"); } if is_global_allocator_enabled(rbconfig) { diff --git a/crates/rb-sys/src/stable_api.rs b/crates/rb-sys/src/stable_api.rs index b53c1ddb..8bc27067 100644 --- a/crates/rb-sys/src/stable_api.rs +++ b/crates/rb-sys/src/stable_api.rs @@ -127,42 +127,58 @@ pub trait StableApiDefinition { unsafe fn rb_type(&self, obj: VALUE) -> crate::ruby_value_type; } -#[cfg(any(not(ruby_api_stable), ruby_lt_2_6))] -use compiled as abi; - -#[cfg(any(not(ruby_api_stable), ruby_lt_2_6, feature = "stable-api-compiled"))] +#[cfg(all( + explicitly_enabled_stable_api_compiled_fallback, + has_stable_api_compiled, + any(not(current_ruby_is_stable), ruby_lt_2_6,) +))] +use compiled as api; + +#[cfg(all( + explicitly_enabled_stable_api_compiled_fallback, + has_stable_api_compiled, +))] mod compiled; +#[cfg(all( + not(has_stable_api_compiled), + any(not(current_ruby_is_stable), ruby_lt_2_6) +))] +compile_error!("Compiled stable API is needed but could not find it."); + #[cfg(ruby_eq_2_6)] #[path = "stable_api/ruby_2_6.rs"] -mod abi; +mod api; #[cfg(ruby_eq_2_7)] #[path = "stable_api/ruby_2_7.rs"] -mod abi; +mod api; #[cfg(ruby_eq_3_0)] #[path = "stable_api/ruby_3_0.rs"] -mod abi; +mod api; #[cfg(ruby_eq_3_1)] #[path = "stable_api/ruby_3_1.rs"] -mod abi; +mod api; #[cfg(ruby_eq_3_2)] #[path = "stable_api/ruby_3_2.rs"] -mod abi; +mod api; /// Get the default stable API definition for the current Ruby version. -pub const fn get_default() -> &'static abi::Definition { - const API: abi::Definition = abi::Definition {}; +pub const fn get_default() -> &'static api::Definition { + const API: api::Definition = api::Definition {}; &API } /// Get the fallback stable API definition for the current Ruby version, which /// is compiled C code that is linked into to this crate. -#[cfg(feature = "stable-api-compiled")] -pub const fn get_fallback() -> &'static compiled::Definition { +#[cfg(all( + explicitly_enabled_stable_api_compiled_fallback, + has_stable_api_compiled, +))] +pub const fn get_compiled() -> &'static compiled::Definition { const COMPILED_API: compiled::Definition = compiled::Definition {}; &COMPILED_API } diff --git a/examples/rust_reverse/ext/rust_reverse/Cargo.toml b/examples/rust_reverse/ext/rust_reverse/Cargo.toml index 92080732..f70084bf 100644 --- a/examples/rust_reverse/ext/rust_reverse/Cargo.toml +++ b/examples/rust_reverse/ext/rust_reverse/Cargo.toml @@ -1,11 +1,13 @@ [package] name = "rust_reverse" version = "0.9.79" -autotests = true # set true if you want to use "cargo test" +autotests = true # set true if you want to use "cargo test" edition = "2018" [dependencies] -rb-sys = { version = "0.9.79", path = "./../../../../crates/rb-sys", features = ["global-allocator"] } +rb-sys = { version = "0.9.79", path = "./../../../../crates/rb-sys", features = [ + "global-allocator", +] } [lib] crate-type = ["cdylib"] diff --git a/examples/rust_reverse/ext/rust_reverse/extconf.rb b/examples/rust_reverse/ext/rust_reverse/extconf.rb index 0073e08e..7b986bf2 100644 --- a/examples/rust_reverse/ext/rust_reverse/extconf.rb +++ b/examples/rust_reverse/ext/rust_reverse/extconf.rb @@ -20,7 +20,7 @@ r.ext_dir = "." # Extra flags to pass to the $RUSTFLAGS environment variable (optional) - r.extra_rustflags << "--cfg=some_nested_config_var_for_crate" + r.extra_rustflags << "--cfg=rb_sys_use_stable_api_compiled_fallback" # Force a rust toolchain to be installed via rustup (optional) # You can also set the env var `RB_SYS_FORCE_INSTALL_RUST_TOOLCHAIN=true` diff --git a/examples/rust_reverse/ext/rust_reverse/extconf_bare.rb b/examples/rust_reverse/ext/rust_reverse/extconf_bare.rb index 9045c8e4..66607082 100644 --- a/examples/rust_reverse/ext/rust_reverse/extconf_bare.rb +++ b/examples/rust_reverse/ext/rust_reverse/extconf_bare.rb @@ -5,4 +5,6 @@ require "mkmf" require "rb_sys/mkmf" -create_rust_makefile("rust_reverse") +create_rust_makefile("rust_reverse") do |r| + r.extra_rustflags << "--cfg=rb_sys_use_stable_api_compiled_fallback" +end diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index a8ac6059..f59de86f 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -11,7 +11,7 @@ cargo-fuzz = true libfuzzer-sys = "0.4" rb-sys-test-helpers = { path = "../crates/rb-sys-test-helpers" } rb-sys = { path = "../crates/rb-sys", features = [ - "stable-api-compiled", + "stable-api-compiled-testing", "link-ruby", "fuzz", ] }