Skip to content

Add support for bundle gem --rust command#5613

Closed
ianks wants to merge 7 commits intoruby:masterfrom
ianks:bundle-gem-rust
Closed

Add support for bundle gem --rust command#5613
ianks wants to merge 7 commits intoruby:masterfrom
ianks:bundle-gem-rust

Conversation

@ianks
Copy link
Copy Markdown
Contributor

@ianks ianks commented Jun 8, 2022

What was the end-user or developer problem that led to this PR?

Despite the recent support of Rust/Cargo as a supported language for developing Ruby gems (#5175), it's still too hard to scaffold out a Rust gem. It should be as easy to scaffold a Rust extension as it is a C extension.

What is your fix for the problem, implemented in this PR?

This PR adds the ability to scaffold a Rust gem by running: bundle gem --rust. A few key points about this integration are:

  1. Works perfectly with rake-compiler via the rb_sys gem
  2. Opted to use the magnus crate, as I believe it offers the most correct and easy to use bindings for Ruby, plus it supports interop with rb-sys. With magnus, gem authors can avoid writing unsafe code when authoring a new gem in Rust.
  3. Scaffolded gems make use of the rb-allocator, so Rust memory usage is properly reported to the Ruby GC.
  4. I prefer using the --rust flag over something like --ext=rust. It's much nicer to read and write.

Looking forward to everyone's feedback!

Make sure the following tasks are checked

ianks and others added 3 commits July 5, 2022 12:49
* master: (125 commits)
  Bump rb-sys in /test/rubygems/test_gem_ext_cargo_builder/custom_name
  Bump rb-sys
  README.md: HTTP => HTTPS
  Avoid resetting specs after every gem install
  Slightly delay loading plugins
  Better solution to support truffleruby platform handling
  Add missing generic
  Extend `gem` DSL with a `force_ruby_platform` option
  Prefer to use modern hash style in docs
  Always show dependency installation output
  Account for default gems not having remote when caching
  Move rubygems source specific logic to rubygems source
  Prevent a warning: `*' interpreted as argument prefix
  Fix unintended double spaces in DSL documentation
  Use modern style hashes in Gemfile DSL docs
  Better format permission errors note
  Reorganize CONTRIBUTING file a bit
  Changelog for Rubygems version 3.3.17
  Changelog for Bundler version 2.3.17
  Add support for platforms :x64_mingw to correctly lookup "x64-mingw-ucrt" on Ruby 3.1
  ...
@chriskilding
Copy link
Copy Markdown

Happy to beta test this once it's semi-ready, if you need a beta tester.

(Bonus: I can test on both an Intel Mac and an M1 Mac)

@simi
Copy link
Copy Markdown

simi commented Jul 12, 2022

Damn, I have missed one. I have code prepared for --ext=rust (backwards compatible) which seems better to me. I tried to keep it as close as possible with C extension (using ext folder, ...). Would you mind to check that one (548189d) as well? It creates "ext/<%= config[:name] %>.rs", but I wasn't sure how to keep it as close as possible to common Rust package.

Maybe we can just keep the standard Rust (Cargo) structure, but in ext or ext-rust subfolder. WDYT? That way for bundle gem newgem --ext=rust you'll find this structure (next to common gem one):

ext/src/newgem.rs
ext/Cargo.toml

So actual changing directory to ext, you'll get the original Cargo feeling. Also we need to add generated dummy tests for nice and smooth experience. Those were last 2 blockers for me to open PR, but due to lack of time and ideas I wasn't able to move it forward.

Btw. we can make --rust alias to --ext=rust if needed.

@ianks
Copy link
Copy Markdown
Contributor Author

ianks commented Jul 13, 2022

Maybe we can just keep the standard Rust (Cargo) structure, but in ext or ext-rust subfolder. WDYT?

Unfortunately this does not provide many advantages, both ext/Cargo.toml and ext/mygem/Cargo.toml will require a root Cargo.toml to provide good editor support.

Given that, I think using the traditional ext/mygem/Cargo.toml structure is the best option, since it makes integration with rake-compiler a bit simpler, and sticks to conventions.

Also we need to add generated dummy tests for nice and smooth experience.

I added some tests in my initial commit, but the bundler unit tests made this hard so I nuked them. I can add them back.

Btw. we can make --rust alias to --ext=rust if needed.

🔥 Sounds good!

@simi
Copy link
Copy Markdown

simi commented Jul 23, 2022

@ianks do you plan to continue on this or should I take over? I would like to merge with my branch and test.

@simi
Copy link
Copy Markdown

simi commented Aug 27, 2022

@ianks friendly pingie. I'll have time this weekend, I can try to finish it if welcomed.

@ianks
Copy link
Copy Markdown
Contributor Author

ianks commented Aug 28, 2022

@simi can you take it from here? things got busy for me!!

@ianks
Copy link
Copy Markdown
Contributor Author

ianks commented Sep 21, 2022

@simi let me know if you run into any issues!

@ankane
Copy link
Copy Markdown

ankane commented Nov 16, 2022

Hi, excited to see this included in Bundler!

I just tried moving a project to this directory structure and think there may be a potential issue.

From what I can tell, the Cargo builder always places the shared library in lib. However, the rake-compiler configuration places it in lib/#{gem_name} (which is what the require_relative expects).

This may be something worth changing or making configurable with the Cargo builder.

Edit: Here's what I needed to change to get it to work with both rb_sys and Cargo.toml (the remove_ext change doesn't apply here and the gemspec change was only made for testing).

@ianks
Copy link
Copy Markdown
Contributor Author

ianks commented Nov 16, 2022

For others that stumble on this, check out the structure I setup for wasmtime-rb, I think it serves as a good standard

@ankane
Copy link
Copy Markdown

ankane commented Nov 16, 2022

Hmm, it doesn't seem to install correctly when using spec.extensions = ["ext/Cargo.toml"] and Ruby 3.2.0-preview2.

On installation, it creates lib/wasmtime-rb.bundle on Mac, but lib/wasmtime.rb has require "wasmtime/ext", so it fails with:

cannot load such file -- wasmtime/ext (LoadError)

Edit: require "wasmtime-rb" should work in this case, but for a gem name like hello, you'll end up with lib/hello.rb and lib/hello.bundle and have to use require "hello.#{RbConfig::CONFIG["DLEXT"]}" to require it.

@ianks
Copy link
Copy Markdown
Contributor Author

ianks commented Nov 16, 2022

Edit: require "wasmtime-rb" should work in this case, but for a gem name like hello, you'll end up with lib/hello.rb and lib/hello.bundle and have to use require "hello.#{RbConfig::CONFIG["DLEXT"]}" to require it.

Yeah you end up having to use a different name each. It's even a bit more entailed because for cross compiled gems you have to load based on the Ruby version.

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

This PR can be closed now, correct?

@simi
Copy link
Copy Markdown

simi commented Dec 21, 2022 via email

@deivid-rodriguez
Copy link
Copy Markdown
Contributor

Thanks for getting this started @ianks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants