Skip to content

Commit 794a8da

Browse files
ianksCopilot
andauthored
Fix stable API rust impl file check (#683)
* fix: check stable API rust impl file exists at build time When detecting Ruby versions like 3.5 (ruby-head), the build would fail with a missing file error because is_stable() only checked version ranges rather than verifying a corresponding ruby_X_Y.rs file exists. Replace is_stable() with has_rust_impl() that checks for actual file existence. This allows ruby-head or other versions without impls to correctly fall back to compiled C when stable-api-compiled-fallback is enabled, or error clearly when it's not. * fix(ci): avoid installing standard gem on Ruby < 3.0 The `standard` gem version 1.52.0 dropped support for Ruby 2.7. Since we only need linting on newer Rubies, we simply skip installing and running `standard` on Ruby < 3.0. * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent c087445 commit 794a8da

File tree

5 files changed

+35
-17
lines changed

5 files changed

+35
-17
lines changed

Gemfile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ gem "racc"
1313
gem "base64"
1414
gem "yard"
1515
gem "mutex_m"
16-
gem "standard"
16+
17+
if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.0")
18+
gem "standard", "~> 1.52.0"
19+
end
20+
1721
gem "tsort"
1822

1923
gem "ostruct", "~> 0.6.3"

Rakefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ task test: ["test:cargo", "test:gem", "test:examples"]
7979
desc "Pretty the files"
8080
task :fmt do
8181
sh "cargo fmt"
82-
sh "bundle exec standardrb --fix" if RUBY_VERSION >= "2.7.0"
82+
sh "bundle exec standardrb --fix" if RUBY_VERSION >= "3.0"
8383
Dir.chdir("docsite") do
8484
sh "npm run fmt"
8585
end
@@ -91,7 +91,7 @@ task format: [:fmt]
9191

9292
desc "Lint"
9393
task :lint do
94-
sh "bundle exec standardrb --format #{ENV.key?("CI") ? "github" : "progress"}" if RUBY_VERSION >= "2.7.0"
94+
sh "bundle exec standardrb --format #{ENV.key?("CI") ? "github" : "progress"}" if RUBY_VERSION >= "3.0"
9595
sh "cargo fmt --check"
9696
sh "cargo clippy"
9797
sh "shellcheck $(git ls-files '*.sh')"

crates/rb-sys/build/stable_api_config.rs

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,30 @@ use crate::{
44
features::is_env_variable_defined,
55
version::{Version, MIN_SUPPORTED_STABLE_VERSION},
66
};
7-
use std::{convert::TryFrom, error::Error, path::Path};
7+
use std::{convert::TryFrom, error::Error, path::{Path, PathBuf}};
8+
9+
/// Get the path to the Rust implementation file for the given Ruby version.
10+
fn rust_impl_path(version: Version) -> PathBuf {
11+
let crate_dir = Path::new(env!("CARGO_MANIFEST_DIR"));
12+
crate_dir
13+
.join("src")
14+
.join("stable_api")
15+
.join(format!("ruby_{}_{}.rs", version.major(), version.minor()))
16+
}
17+
18+
/// Check if a Rust implementation file exists for the given Ruby version.
19+
fn has_rust_impl(version: Version) -> bool {
20+
let path = rust_impl_path(version);
21+
path.exists()
22+
}
823

924
pub fn setup(rb_config: &RbConfig) -> Result<(), Box<dyn Error>> {
1025
let ruby_version = Version::current(rb_config);
1126
let ruby_engine = rb_config.ruby_engine();
27+
28+
// Ensure we rebuild if the file is added or removed
29+
let rust_impl_path = rust_impl_path(ruby_version);
30+
println!("cargo:rerun-if-changed={}", rust_impl_path.display());
1231
let strategy = Strategy::try_from((ruby_engine, ruby_version))?;
1332

1433
strategy.apply()?;
@@ -42,7 +61,7 @@ impl TryFrom<(RubyEngine, Version)> for Strategy {
4261
RubyEngine::Mri => {}
4362
}
4463

45-
if current_ruby_version.is_stable() {
64+
if has_rust_impl(current_ruby_version) {
4665
strategy = Some(Strategy::RustOnly(current_ruby_version));
4766
} else {
4867
maybe_warn_old_ruby_version(current_ruby_version);
@@ -76,10 +95,10 @@ impl Strategy {
7695
println!("cargo:rustc-check-cfg=cfg(stable_api_has_rust_impl)");
7796
match self {
7897
Strategy::RustOnly(current_ruby_version) => {
79-
if current_ruby_version.is_stable() {
98+
if has_rust_impl(current_ruby_version) {
8099
println!("cargo:rustc-cfg=stable_api_include_rust_impl");
81100
} else {
82-
return Err(format!("A stable Ruby API is needed but could not find a candidate. If you are using a stable version of Ruby, try upgrading rb-sys. Otherwise if you are testing against ruby-head or Ruby < {}, enable the `stable-api-compiled-fallback` feature in rb-sys.", MIN_SUPPORTED_STABLE_VERSION).into());
101+
return Err(format!("No Rust stable API implementation found for Ruby {}. If you are using a stable version of Ruby, try upgrading rb-sys. Otherwise if you are testing against ruby-head or Ruby < {}, enable the `stable-api-compiled-fallback` feature in rb-sys.", current_ruby_version, MIN_SUPPORTED_STABLE_VERSION).into());
83102
}
84103
}
85104
Strategy::CompiledOnly => {
@@ -88,7 +107,7 @@ impl Strategy {
88107
println!("cargo:rustc-cfg=stable_api_export_compiled_as_api");
89108
}
90109
Strategy::RustThenCompiled(current_ruby_version) => {
91-
if current_ruby_version.is_stable() {
110+
if has_rust_impl(current_ruby_version) {
92111
println!("cargo:rustc-cfg=stable_api_has_rust_impl");
93112
println!("cargo:rustc-cfg=stable_api_include_rust_impl");
94113
} else {
@@ -102,7 +121,7 @@ impl Strategy {
102121

103122
println!("cargo:rustc-cfg=stable_api_enable_compiled_mod");
104123

105-
if current_ruby_version.is_stable() {
124+
if has_rust_impl(current_ruby_version) {
106125
println!("cargo:rustc-cfg=stable_api_include_rust_impl");
107126
} else {
108127
println!("cargo:rustc-cfg=stable_api_export_compiled_as_api");

crates/rb-sys/build/version.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,6 @@ impl Version {
3636
}
3737
}
3838
}
39-
40-
#[allow(dead_code)]
41-
pub fn is_stable(&self) -> bool {
42-
*self >= MIN_SUPPORTED_STABLE_VERSION && *self <= LATEST_STABLE_VERSION
43-
}
4439
}
4540

4641
impl std::fmt::Display for Version {

script/show-asm

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -585,9 +585,9 @@ class ShowAsm
585585
archhdrdir = ruby_config["rubyarchhdrdir"]
586586

587587
cmd = [cc, src_file.path,
588-
"-S", # Output assembly
589-
"-O3", # Optimize
590-
"-fno-stack-protector", # Disable stack canary for fair comparison
588+
"-S", # Output assembly
589+
"-O3", # Optimize
590+
"-fno-stack-protector", # Disable stack canary for fair comparison
591591
"-I#{hdrdir}",
592592
"-I#{archhdrdir}",
593593
"-o", asm_file.path]

0 commit comments

Comments
 (0)