-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add --ext=rust
support to bundle gem
for creating simple gems with Rust extensions
#6149
Conversation
0a66ffe
to
b86351c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added comment for cometic.
This looks good for the most part. Great work @simi Although I do really think we should default to Magnus (with the rb-sys-interop feature). Defaulting to rb-sys is not something I would encourage for gem authors. Are you open to doing that? |
So essentially the CLI interface I’d recommend would be like this:
|
What about following?
|
I'm going to submit |
Can we alias |
I'd prefer not to, because really gem authors should be using a high-level / safe library like |
fa6ffed
to
2fc1993
Compare
@ianks ok, for now I decided to just got with Next to those changes, I have updated CIs to be able to compile Rust by default (when selected). I have used diferent approach per CI since each work in different way. At some of them I have opted-in to install latest RubyGems and Bundler, since bundler can now auto-switch. That seems the easiest option for now to keep it compatible accross various Rubies. |
Also I have added |
@@ -23,5 +23,8 @@ jobs: | |||
with: | |||
ruby-version: ${{ matrix.ruby }} | |||
bundler-cache: true | |||
<%- if config[:ext] == 'rust' -%> | |||
rubygems: 'latest' # for best Rust extension support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use https://github.com/oxidize-rb/actions/blob/main/setup-ruby-and-rust/readme.md for CI on rust if possible. It’s well supported and this current configuration will probably not work in all scenarios, it also will be slow due to no caching etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ianks there is only v1 tag and that doesn't support needed rubygems
option. Would it be possible to tag new version?
Can we keep CI improvement discussion out of this PR? I'll open following PR with all suggested improvements once merged. |
@deivid-rodriguez I have reverted all changes into CI templates for this PR. I'll continue on improving those in following PR keeping in mind all suggestions from comments in here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks @simi!
Looks good to me! Thanks so much @simi, this will have a huge impact. Let's give @matsadler a chance to chime in as well :) |
@ianks cool, we would like to release this soon before Ruby 3.2 (to be included in there). I'm going to prepare followup (based on this one) for CI thingies meanwhile. Also I would like to support this by upgrading guides and maybe writing small blog explaning how this works. |
No comments from me apart from this is very cool. Let me know if you'd like a hand with any documentation, like if you want examples of how to do something in particular with Magnus. |
Should I squash, rebase and merge? |
You can merge this without |
d6f5d71
to
6853738
Compare
@deivid-rodriguez can you please check 6853738 is what you suggested? I can squash and merge once approved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
- generates simple Rust extension skeleton - currently based on magnus rust crate - limit to only supported RubyGems
bundle gem
for simple Rust extension.
bundle gem
for simple Rust extension.--ext=rust
support to bundle gem
for creating simple gems with Rust extensions
Supporting just raw Rust without magnus. I would like to add additional extension skeleton using magnus (like
--ext=magnus
).based on combination of:
bundle gem --rust
command #5613I would like to add another PR as followup to do something at least "almost usual" in generated extensions (for example define "hello" method and return "hello world") for both C and Rust variants (and potentially
magnus
as well once introduced).@ianks feel free to review 🙏