-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Update --ext=rust to support compiling the native extension from source
#7610
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
Conversation
|
Thanks for opening a pull request and helping make RubyGems and Bundler better! Someone from the RubyGems team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality. We use GitHub Actions to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of GitHub Actions in the PR status window below. If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #rubygems or #bundler channel on Slack. For more information about contributing to the RubyGems project feel free to review our CONTRIBUTING guide |
8e28b14 to
60fb7ec
Compare
60fb7ec to
1f7fed4
Compare
1f7fed4 to
5d5967c
Compare
|
👋 Hey @simi, thanks for chiming in on this PR. I've noticed the branch was outdated with the I'd appreciate your review when you have a moment :) |
|
Hello @karreiro and thanks for your PR. Can you please explain how to reproduce your errors? I just tried in latest bundler to create empty skeleton gem, pack it into gem and install with no problems. I have did following. $ gem list | grep rb_sys # ensure rb_sys is not installed already
$ bundle gem --ext=rust ryba > /dev/null # create empty gem
$ cd ryba # move into gem directory
$ cargo c # resolve the Cargo.lock somehow
$ git add Cargo.lock # add lock to the final gem, since gem files are collected from git by default
$ gem build --force # build gem, ignore TODO in gemspec
$ gem install ryba-0.1.0.gem # install local gem |
|
Hello @simi, thank you for reviewing this PR :) SetupTo reproduce the error, let's create 3 ryba gems: ryba1
ryba2
ryba3
ResultsNow, we have 3 .gem files:
Let's move to a machine without Rust installed now. ryba1 (result)When we run ryba2 (result)When we run ryba3 (result)When we run |
5d5967c to
478876d
Compare
478876d to
0dab1c6
Compare
|
I just rebased this branch :) |
Yes, it addresses the same issue as #7695 and closes #7693. However, this PR goes a bit further by moving require "rb_sys/mkmf"
heh, I share the same feeling... the We could possibly propose making that opt-in on |
Cool, yeah, that makes sense. If
Yep, seems something separate and should not affect this PR. Still surprising though! |
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.
This looks good to me, but I'll let @simi give final approval!
|
@deivid-rodriguez yes, I'll test and merge. |
0dab1c6 to
5342efe
Compare
|
@simi I did test this myself this month and it worked fine, I think it'd be fine to merge if you don't have too much time to do the testing yourself. @ianks Since you proposed a slightly overlapping PR at #7608 (and you're rb-sys author after all :)), would like to give this a quick look? We can them move on to #7608 after merging this. |
5342efe to
f47987a
Compare
|
@deivid-rodriguez I'll test soon 🙏, initially looks good. |
|
Ok, thank you @simi 😃. |
f47987a to
9b0ec80
Compare
Update `--ext=rust` to support compiling the native extension from source (cherry picked from commit e5d8348)
What was the end-user or developer problem that led to this PR?
I needed to apply some tiny changes to the skeleton generated by the
--ext=rustoption to compile my native extension from source.What is your fix for the problem, implemented in this PR?
When gem consumers install a gem with a native extension and their platform is not included the pre-compiled bundles, they need to compile it locally and rely on the
extconf.rbfile.Referencing the
extconf.rbfile in the gemspec is required to make that work. We also need to includerb_sysas a dependency because the Rustextconf.rbdepends on it.Here's a demo of this change in action :)
Screen.Recording.2024-04-25.at.21.22.36.mov
Closes #7693.