Open goyox86 opened 1 year ago
Also, I decided to use rutie even though someone said magnus is better because magnus simply doesn't build with the target i686-pc-windows-gnu. If you switch to rb-sys we'll not be able to use rutie as well which is kind of problematic.
Hey @NuriYuri, I'm more than happy to support i686-pc-windows-gnu
, but just haven't gotten around to doing it since I don't have a machine to test on. Can you make an issue on the rb-sys
repo with the errors you are getting so we can try to resolve them?
Hi, thanks for the feedback :)
Fortunately I have the machine for those tests so opened an issue on rb-sys as advised :)
Since I can now build rb-sys I got the following issue with the code in your branch (type missmatch)
Compiling rb-sys v0.9.71 error[E0308]: mismatched types --> src\rubysys\array.rs:92:16 | 92 | if flags & (RArrayEmbed::Flag as u64) == 0 { | ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u32`, found `u64` error[E0277]: no implementation for `u32 & u64` --> src\rubysys\array.rs:92:14 | 92 | if flags & (RArrayEmbed::Flag as u64) == 0 { | ^ no implementation for `u32 & u64` | = help: the trait `BitAnd` is not implemented for `u32` = help: the following other types implement trait `BitAnd `: <&'a i128 as BitAnd > <&'a i16 as BitAnd > <&'a i32 as BitAnd > <&'a i64 as BitAnd > <&'a i8 as BitAnd > <&'a isize as BitAnd > <&'a u128 as BitAnd > <&'a u16 as BitAnd > and 40 others error[E0308]: mismatched types --> src\rubysys\string.rs:124:13 | 124 | flags & (RStringEmbed::NoEmbed as u64) == 0 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `u32`, found `u64` error[E0277]: no implementation for `u32 & u64` --> src\rubysys\string.rs:124:11 | 124 | flags & (RStringEmbed::NoEmbed as u64) == 0 | ^ no implementation for `u32 & u64` | = help: the trait `BitAnd ` is not implemented for `u32` = help: the following other types implement trait `BitAnd `: <&'a i128 as BitAnd > <&'a i16 as BitAnd > <&'a i32 as BitAnd > <&'a i64 as BitAnd > <&'a i8 as BitAnd > <&'a isize as BitAnd > <&'a u128 as BitAnd > <&'a u16 as BitAnd > and 40 others error[E0308]: mismatched types --> src\rubysys\string.rs:190:13 | 190 | flags & STR_TMPLOCK as u64 != 0 | ^^^^^^^^^^^^^^^^^^ expected `u32`, found `u64` error[E0277]: no implementation for `u32 & u64` --> src\rubysys\string.rs:190:11 | 190 | flags & STR_TMPLOCK as u64 != 0 | ^ no implementation for `u32 & u64` | = help: the trait `BitAnd ` is not implemented for `u32` = help: the following other types implement trait `BitAnd `: <&'a i128 as BitAnd > <&'a i16 as BitAnd > <&'a i32 as BitAnd > <&'a i64 as BitAnd > <&'a i8 as BitAnd > <&'a isize as BitAnd > <&'a u128 as BitAnd > <&'a u16 as BitAnd > and 40 others error[E0308]: mismatched types --> src\rubysys\typed_data.rs:63:20 | 63 | dsize: rb_data_type_fn.dsize, | ^^^^^^^^^^^^^^^^^^^^^ expected `u32`, found `u64` | = note: expected enum `Option u32>` found enum `Option u64>` error[E0277]: the trait bound `u32: From ` is not satisfied --> src\rubysys\typed_data.rs:77:20 | 77 | flags: rb_data_type.flags.into(), | ^^^^^^^^^^^^^^^^^^ ---- required by a bound introduced by this call | | | the trait `From ` is not implemented for `u32` | = help: the following other types implement trait `From `: > > > > > > > > and 68 others = note: required for `Value` to implement `Into ` error[E0308]: mismatched types --> src\rubysys\value.rs:187:9 | 186 | fn from(value: Value) -> Self { | ---- expected `u64` because of return type 187 | value.value | ^^^^^^^^^^^ expected `u64`, found `u32` | help: you can convert a `u32` to a `u64` | 187 | value.value.into() | +++++++ Some errors have detailed explanations: E0277, E0308. For more information about an error, try `rustc --explain E0277`. error: could not compile `rutie` due to 9 previous errors
Branch I built: goyox86/rb-sys-in-place
Commit I built: 5f422a9 (HEAD -> goyox86/rb-sys-in-place, origin/goyox86/rb-sys-in-place) Better Ruby version check and we don't need that TravisCI script anymore.
@NuriYuri Could you use the Github "spoiler" feature to hide your long console output from your earliest post? Here's an example on how: https://gist.github.com/jbsulli/03df3cdce94ee97937ebda0ffef28287#how-to
@goyox86 you may want to explore using rb-sys-test-helpers
for the tests, it handles some of the nasty threading bugs due to the way cargo spawns threads for each test
@goyox86 you may want to explore using
rb-sys-test-helpers
for the tests, it handles some of the nasty threading bugs due to the way cargo spawns threads for each test
Oh! Going to check that out I was actually debugging segfaults today because of VM::init and thread safety in tests. Thanks for the heads up @ianks !
@ianks @danielpclark I think we are in a good position for another review pass. We are green on 2.7, 3.0. 3.1, 3.2 and HEAD on Linux, macOS, Windows. ❕ ❕
Hey! Great work. Any updates on merging this?
FYI Tried to run this PR with ruby 3.2.2 and temporal sdk-ruby. Had to make few changes until it runs:
src/rubysys/value.rs
line 109 added !self.is_nil() &&
:
if !self.is_nil() && self.is_immediate() {
This fix was from #180 .
src/dsl.rs
line 673
#[cfg(ruby_gte_2_7)]
let reserved_bytes: [*mut $crate::types::c_void; 1] = [::std::ptr::null_mut(); 1];
#[cfg(ruby_lt_2_7)]
let reserved_bytes: [*mut $crate::types::c_void; 2] = [::std::ptr::null_mut(); 2];
to
let reserved_bytes: [*mut $crate::types::c_void; 1] = [::std::ptr::null_mut(); 1];
Amazing work @goyox86 ! I'm excited for this. We're almost there.
@goyox86 Once my current comments are addressed I would be ready to merge this in to the main branch and release a new gem release with the changes. As to the question on the description as to whether the commits should be squashed - I don't believe there is a need for that.
Hey @danielpclark! Great news I will make some time this week to address the comments 🙏
@goyox86 Where are we at on this? I'm expecting a thorough documentation of breaking changes notes in the README. I don't know though if you were expecting to do that. With all the changes it's not likely the one method is the only breaking change. What are the changes to the process or usage? Once I'm relatively certain we're informing people of what they're in for with the upgrade I'll move forward with this.
Also looking at the test suite. I believe you added a config for static so that we have both dynamic and statically linked Rubies tested. But I'm not seeing the result of that.
Happy to give another review once ready as well, since it has been awhile since the first one
Addresses https://github.com/danielpclark/rutie/issues/163
This PR migrates using the
rb-sys
crate to interface with CRuby instead of the homegrown derived from Rubysys. Additionally it incorporates improvements to the Rust crate and migrates the CI to Github Actions along some refreshments to Rust crate code (Rust 2021).Highlights
rb_sys
we now support Ruby2.7
,3.0
,3.1
,3.2
andmaster
on Linux, MacOS, and Windows and master all integrated now in CI.rb_sys
infrastructure to setup our build environment.rb_sys
instead of defining them ourselves.From
instead ofInto
to get theInto
impl for free).rake
version 12.rb_sys
'sruby_test
macro we don't need to lock globals on tests, and there are no weird crashes in MacOS and Windows.rb_sys
facilities where possible.Fixnum
s around and we don't have those anymore, so I replaced byInteger
.I think there are two breaking changes, I marked them as To-dos, they are most around marking stuff as
unsafe
.Left to do
rb_sys
that allows me to use the new cargo features, since I am using head from Git now. @ianks WDYT?libruby
linking see if the new version ofrutie
would be backwards compatible.