-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Allow extconf.rb
to cancel building the extension
#7372
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
base: master
Are you sure you want to change the base?
Conversation
Ref: https://bugs.ruby-lang.org/issues/20152 There are various gems that ship with a native extension as a way to speedup part of the gem, but also ship with a pure Ruby version of these methods as a fallback. So they only want to compile the extension if the platform supports it, and if not, just fallback Ruby implementation. Right now users rely on one of two hacks to do this. Either they create an empty Makefile, but then still depend on make being available and it feels very hacky, or they publish platform specific packages without any extension in them. Examples include `bootnsap`, `erb`, `hiredis-client`, `ddtrace`, `prism`. This changes attempt to make this use case a first class citizen by checking if the `extconf.rb` did generate a `gem.build_skipped` file. If it did, `make` isn't invoked at all.
Seems reasonable to me, probably needs to be documented somewhere though 👍🏻 |
Is there an existing documentation for Also my plan is to add some |
One problem with this is the one I explained in https://bugs.ruby-lang.org/issues/20152#note-11:
i.e. specifically it is very error-prone to use this feature until the oldest Ruby a gem supports would have this |
In principle the idea of not trying to |
Not that I can think of. I mean if you generate an empty make file by accident today, running a noop I went with a different file because it seemed more deliberate, but I'm fine with trying to detect empty Makefile too. |
This came up again in gsamokovarov/skiptrace#8 and I think everyone would be ok with simply detecting an empty Makefile and not running There's still a remaining issue in #3520: even fake builds still emit files to indicate that something was built for the current JDK + JRuby platform, and switching JDK complains about missing build files even though nothing is built. (@deivid-rodriguez Can we get that issue re-opened too?) |
Sure, I understand better that issue now so happily to reevaluate it. Not sure when I'll get to it though, but happy to keep it open as a remainder to give it another look after we fix this first issue. |
I understand the need and like the idea, but I'm not sure it is good idea to rely on file presence. 🤔 |
@simi Do you mean that you prefer to detect if the Gemfile is empty in order to decide to not run |
@deivid-rodriguez I have no clear idea for now. Checking for empty Makefile seems also just bandaid. |
It's definitely not first class citizen solution, but seems like an easy thing to implement on our side with no downside? And wouldn't be in conflict with potential future approaches. |
At the risk of repeating myself, I think detecting the empty Makefile generated from
Adding a new API could only be used in gems once current RubyGems version is EOL or so, or would need to check a condition + change many gems which I don't think is worth the time or effort, |
I agree with @eregon |
OK, 👍 on empty |
I was thinking more about this, and I realized that even if we fix the "make not available" problem by detecting if the Makefile is empty, we still don't have a good way to fix #3520. Every time platform changes (like when upgrading Java, or macOS), or when the Ruby ABI changes and you have a shared As an alternative, I was thinking of introducing a new extension builder, This builder would get used when the extension is named |
A quick idea: a top-level metadata that indicates an installed gem does not need missing ext check because the ext is optional and not being used? The check would first see if it's a gem with an omitted extension, and not go further. This could be localized to the RUBY_ENGINE since CRuby would rarely omit extension and JRuby frequently will.
It seems to me this would work but it's also a little cumbersome. Can you explain why |
What would "top level metadata" mean in this case? A
We could make it the default, but the warnings that extensions are not built in my opinion are useful. A different "optional builder" would allow skipping those warnings when not needed but keep them when the standard more strict extconf builder is used. |
Also note it's not only about the warnings being annoying. Gems with missing extensions are currently ignored when activating gems, so making the optional builder I'm proposing the default would mean sometimes activating broken gems, for those gems where the extension is actually compulsory. |
Oh, you probably meant gemspec metadata, which can technically include anything, so it's a backwards compatible way of adding stuff to the gemspec. So, something like |
@deivid-rodriguez sorry I don't know the right terminology here. I mean rather than always going to the platform and architecture-specific directory to look for confirmation that the extension was built, introduce a higher level location that can provide metadata for the installed gem not specific to a given platform or architecture. So first it would check for this gem on this Ruby engine, is it expected to have an extension built, and only after that is confirmed does it proceed to the current check. It would be a way to indicate that a given gem on the current Ruby engine does not require an extension to be built. |
Oh, I see! So you're essentially proposing something similar to what this PR is proposing. I thought of that but things did not square, but after another thought maybe that works? We could:
|
Yes something like you proposed but I do not know if that is the best solution or not! I will go with whatever the majority thinks on that regard, so long as I can eliminate
Yeah that sounds right! I would love for there be a way to say something like |
For #3520, building on what @deivid-rodriguez said we could do this automatically without requiring any changes in gems by, after RubyGems calls extconf.rb, it detects if empty/dummy Makefile, if it is:
That way the check for "are extensions builds" just needs to be extended to check that file on top of checking the platform-specific If we want to apply this new logic for already-installed gems when updating RubyGems that's also possible by having the "are extensions builds" check, if it doesn't find any IOW, we already have a marker for "no extension library is expected", it's the empty/dummy Makefile. |
If I understood correctly, I had that same idea but I don't think that works? If we put a I think these are just two different things which need different markers:
|
For a gem with an extension that gets built on CRuby, the extconf.rb would generate a non-dummy Makefile, so there would be no platform-independent If you are thinking about sharing a gem home between CRuby and some other Ruby implementation, that has never been supported or worked AFAIK (except maybe for pure-Ruby gems with no It's the same case for both, everything can be decided as soon as the |
Yes, that's what I'm thinking. That may not have worked well sometimes, but RubyGems does try to act reasonably when that happens. In fact, I think all these "extensions are not built" warnings and logic to rebuild extensions when necessary was added many years ago to support that kind of situation, and many people do it in real life anyways. Recently we have shipped some fixes related to that and as far as I know it currently works just fine. Also for situations where people set a custom GEM_HOME (not scoped to ABI or platforms), we want to handle Ruby upgrades gracefully. |
How does that work?
(that's using the RubyGems shipped with ruby head) And indeed it's unsafe to share it:
These days TruffleRuby and CRuby-head check the ABI version of these And even then, if somehow there was no more That said, if you do want to try to support that, the platform-independent
The problem is that's dependent on the RUBY_ENGINE for example, on CRuby it's typically a problem if the extension is missing, as it would fail with LoadError (or for a few gems use a much slower pure-Ruby backend for some gems, still undesirable). OTOH e.g. for the |
Sorry, I didn't mean sharing I mean using something like For example, run |
Exactly, that's why it would be an "activation time" thing that would allow no warnings on JRuby and a graceful fallback on CRuby, but extensions would still get reinstalled when it matters at "install time". |
Actually I just tried your exact steps (except with Ruby 3.3 and Ruby 3.4-dev), and I got the default |
The ABIs of 3.3 and 3.4-dev are closer, so there is more chances it doesn't fail immediately. If you try with Ruby 3.1 you should be able to reproduce. With 2.7 it even gives an |
We never try to load an extension compiled for a different ABI, no matter how close they are. I can't reproduce either with Ruby 3.1 either, the default json version compiled for 3.1 is loaded. |
As I said before, your RubyGems version in Ruby 3.1 is probably old and does not have the latest improvements. This behavior of ignoring gems with extensions compiled for a different ABI has been there for a long time, but only recently we fixed an issue where it was not applying to default gems like |
I tried just that with 3.3 and 3.2 and bigdecimal:
Which looks like a dynamic loader/linker error with some undefined memory being read, not a RubyGems exception. But indeed with
It's a good safety measure RubyGems refuses to activate the gem in such a case. |
I think it shouldn't allow a graceful fallback on CRuby, it should fail as it currently does on CRuby by refusing to activate the gem (assuming a gem where the extension is built on CRuby, and noop on JRuby via dummy Makefile). So indeed, if sharing a GEM_HOME should be supported between different Ruby implementations I think we should do this:
Agreed on activation time, it's already how this works, just the check would be extended to find the platform-independent but RUBY_ENGINE-dependent and ABI-dependent I get your point now that we cannot look for a dummy Makefile at activation time, because the Makefile might not be the one corresponding to the current Ruby, it might be the one of another Ruby on which |
FWIW on TruffleRuby we currently have this warning:
So it's already very much discouraged and unsupported to reuse a GEM_HOME on TruffleRuby. |
And it should work fine too with the latest RubyGems.
It seems fine to me. It may leave some leftovers for other platform/ABIs, but it seems more practical than reinstalling all your gems. In any case, any change in this message or improvements to clean up artifacts for other platforms/ABIs seem unrelated here.
Alright, so then we're back to almost what this PR proposes, except that:
|
Yes, that sounds good to me (with There is one more complication that a gem may have multiple extensions and some extension might be skipped and another extension of that gem might be not skipped. |
I think we never ignore gems is correct. In JRuby or other implementations where extensions are not really built, then gems with extensions are always "require-ready" and we never need to filter out ("ignore") gems with missing extensions since there's no such thing.
Good point, we'll need to handle that case carefully. |
Ah yes, makes sense, agreed, I misunderstood what was meant there. In fact I was thinking about naming and whether it should be |
Ref: https://bugs.ruby-lang.org/issues/20152
There are various gems that ship with a native extension as a way to speedup part of the gem, but also ship with a pure Ruby version of these methods as a fallback. So they only want to compile the extension if the platform supports it, and if not, just fallback Ruby implementation.
Right now users rely on one of two hacks to do this. Either they create an empty Makefile, but then still depend on make being available and it feels very hacky, or they publish platform specific packages without any extension in them.
Examples include
bootnsap
,erb
,hiredis-client
,ddtrace
,prism
.This changes attempt to make this use case a first class citizen by checking if the
extconf.rb
did generate agem.build_skipped
file. If it did,make
isn't invoked at all.