-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Don't fail if there is no makefile, simply don't do anything. #8879
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
Don't fail if there is no makefile, simply don't do anything. #8879
Conversation
75d17a1 to
480d641
Compare
See <ruby/rubygems#8879> for a better fix.
See <ruby/rubygems#8879> for a better fix.
480d641 to
4450cdb
Compare
|
I'm +1 for this of course. May also help #3520? |
|
I don't really understand the full issue here, but in order to support #3520 we'd probably need to communicate the fact that no native extension was built back to the index that's tracking what gems are compatible with what, if any such index exists. |
|
LGTM as well, seems convenient and cleaner than a dummy makefile. |
deivid-rodriguez
left a comment
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.
Thank you!
More work will be needed for #3520, but this is a good change and everyone agreed with it over at #7372 and https://bugs.ruby-lang.org/issues/20152.
The implementation seems fine to me, too 👍.
I'll leave this opened for a couple more days in case additional reviewers show up, but I'm good with this!
4450cdb to
0e92346
Compare
|
Let's do this, thanks so much @ioquatix! |
On some platforms, no native extensions should be created. This is typical for JRuby. Due to limitations in the extension builder, it is typical to create a dummy makefile that does nothing:
https://github.com/search?q=File.write+Makefile+path%3A**%2Fextconf.rb&type=code
This seems like a bad pattern.
I think it would be preferable to just do:
Make sure the following tasks are checked