Conversation
ianks
force-pushed
the
stable-rust-impl-ensure
branch
from
1c2ea35 to
41477b7
Compare
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.
ianks
force-pushed
the
stable-rust-impl-ensure
branch
from
41477b7 to
6ab2129
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a build failure when using Ruby versions like 3.5 (ruby-head) by replacing the version range check with an actual file existence check. Previously, is_stable() only checked if a version fell within a predefined range, causing builds to fail when the implementation file didn't actually exist.
Key changes:
- Replaced
is_stable()method withhas_rust_impl()that checks for actual Rust implementation file existence - Updated error messages to include the specific Ruby version for better debugging
- Added version constraint to the
standardgem in Gemfile
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/rb-sys/build/version.rs | Removes the deprecated is_stable() method that only checked version ranges |
| crates/rb-sys/build/stable_api_config.rs | Implements has_rust_impl() function that checks for actual file existence and updates all call sites |
| script/show-asm | Formatting adjustment for command-line argument comments |
| Gemfile | Adds version constraint to standard gem (appears unrelated to main fix) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9
to
+24
| /// Check if a Rust implementation file exists for the given Ruby version. | ||
| fn has_rust_impl(version: Version) -> bool { | ||
| let crate_dir = Path::new(env!("CARGO_MANIFEST_DIR")); | ||
| let path = crate_dir.join("src").join("stable_api").join(format!( | ||
| "ruby_{}_{}.rs", | ||
| version.major(), | ||
| version.minor() | ||
| )); | ||
|
|
||
| let exists = path.exists(); | ||
|
|
||
| // Ensure we rebuild if the file is added or removed | ||
| println!("cargo:rerun-if-changed={}", path.display()); | ||
|
|
||
| exists | ||
| } |
There was a problem hiding this comment.
The has_rust_impl() function performs filesystem checks and is called multiple times during the build (lines 62, 96, 108, 122). Consider caching the result to avoid redundant filesystem operations, especially since the version doesn't change during a single build.
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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ianks
force-pushed
the
stable-rust-impl-ensure
branch
from
675f50e to
d1d15c9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When detecting Ruby versions like 3.5 (ruby-head), the build fails with a
missing file error because is_stable() only checked version ranges.
This fix replaces is_stable() with has_rust_impl() that checks if the actual
ruby_X_Y.rs file exists. Versions without impls (like 3.5, 4.1) now correctly
fall back to compiled C when stable-api-compiled-fallback is enabled, or error
clearly when it's not.
Fixes the issue where ruby-head (3.5.0) couldn't build rb-sys with stable API.