Help with magnus migration

[ianks]

So I had a little time and figured I'd give yrb a whirl. Great work on it! I figured I'd contribute some improvements along the way:

1. Use the git version of magnus for full support of rb-sys. This vastly reduces the number of edge cases when linking with libruby.
2. Add a ci.yml workflow for testing on each PR and push (uses a new GitHub action I wrote to make this easy). Everything is green, including windows!✅
3. Fix for cross compiling issue with bundler on rcd. This resolves makes cross compilation work on 4 platforms ✅
4. Integrate clippy into the workflow (no strict checks yet, but added the config for VSCode and warnings in CI)

Hopefully you find this useful!

[eliias]

@ianks Thanks a lot for this contribution 💚.

Could you help me understand what the Send trait exactly does for us? I somewhat follow the explanation from "the book" (https://doc.rust-lang.org/nomicon/send-and-sync.html), but I wonder if this is to make the lib thread-safe from a Ruby perspective, or is this just best practice for all data structures that are eventually passed between the Ruby/Rust world?

[ianks]

but I wonder if this is to make the lib thread-safe from a Ruby perspective, or is this just best practice for all data structures that are eventually passed between the Ruby/Rust world?

Send is simply a marker trait, meaning we "manually" have to guarantee that the types are safe to be sent across threads. In this case, I'm operating under the assumption that yrb isn't sending this data across its own spawned Rust threads, and that all access is done when the GVL is held. I didn't dive into the implementation too much though, it might be wise to use a Mutex instead of RefCell if Rust threads are involved.

[ianks]

@eliias See your approval, but it won’t let me merge. I’ll let you take care of that part, but if you need anything else just let me know. Happy to help.

[eliias]

@eliias See your approval, but it won’t let me merge. I’ll let you take care of that part, but if you need anything else just let me know. Happy to help.

Ah… my bad, I forgot about permissions. Will merge 👍, and thanks again.

[ianks]

FWIW, I think that windows failure is a legit bug. The tests have passed before so it may be flaky. Ideally you can merge with this failure and address it in a follow up?

[eliias]

FWIW, I think that windows failure is a legit bug. The tests have passed before so it may be flaky. Ideally you can merge with this failure and address it in a follow up?

Yeah, I have seen similar behavior with XMLText/XMLElement previously in test runs (and not on Windows). @Horusiath could you take a look at the test output and help us assess if this is a y-rb or a y-crdt issue?

[Horusiath]

I've recreated the failing scenario in Rust, and couldn't reproduce the issue. Since the code there doesn't use serialisation, I assume that problem must exists somewhere in the memory mapping between Rust and Ruby classes.

[eliias]

I've recreated the failing scenario in Rust, and couldn't reproduce the issue. Since the code there doesn't use serialisation, I assume that problem must exists somewhere in the memory mapping between Rust and Ruby classes.

@Horusiath thanks for checking, I will try to reproduce somehow. It is a bit weird, it only happens with XML nodes, and it also fails intermittently 🤷.