-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Make JRuby compatible with capybara-webkit (and possibly other gems) #4031
Make JRuby compatible with capybara-webkit (and possibly other gems) #4031
Conversation
I agree. mkmf should work to generate makefiles, since they have other uses in gems than building C exts. Will review today. |
The PR looks straightforward enough. I wonder, though...is there a better time for us to warn that C exts aren't supported than at compile time? I mean, it would be nice if mkmf still errored out once it became obvious that the gem contained an extension rather than a simple library. |
I've thought about this too. The only thing I could think of was changing certain parts of I am not familiar with the details of JRuby's usage of ruby's stdlib, though. Is it fine to keep source files with small changes in the "stdlib" directory? Or is there a place where stdlib code can be monkey patched? |
@joankaradimov We do maintain a lightly-patched stdlib fork at https://github.com/jruby/ruby in the |
I'll look into it. |
this will most probably not work with JRUBY_HOME inside a jar, i.e. the jruby-complete.jar which is used my maven and gradle to install gems. this means some gems fail differently with jruby installation and those jar versions of jruby - if I understand this all right. |
@mkristian You are just saying that non-ext C libs built with extconf/mkmf will fail when installing from inside a jar, yes? |
@headius I am not sure what I am saying - maybe just try the whole thing with jruby-complete.jar as well as it acts sometimes differently then the filesystem installation of jruby. |
@mkristian I tested a bit. It behaves as I expect it to. I'm pasting some console output. For some context - I'm on Windows (sorry for the windows command line) and capybara-webkit has a small issue that I have resolved locally. So I'm building and installing the gem from the local file system. I'm also using a freshly built jruby-complete located in Building the gem works just fine... C:\dev\capybara-webkit>java -classpath C:\dev\jruby\maven\jruby-complete\target\jruby-complete-9.1.3.0-SNAPSHOT.jar org.jruby.Main --command jgem build capybara-webkit.gemspec
[.......... a bunch of warnings are printed .......]
Successfully built RubyGem
Name: capybara-webkit
Version: 1.11.1
File: capybara-webkit-1.11.1.gem Installation fails... C:\dev\capybara-webkit>java -jar C:\dev\jruby\maven\jruby-complete\target\jruby-complete-9.1.3.0-SNAPSHOT.jar --command jgem install capybara-webkit-1.11.1.gem
ERROR: While executing gem ... (Gem::FilePermissionError)
You don't have write permissions for the uri:classloader:/META-INF/jruby.home/lib/ruby/gems/shared directory. But once the C:\dev\capybara-webkit>set GEM_HOME=C:\dev\jruby-gem-home
C:\dev\capybara-webkit>java -jar C:\dev\jruby\maven\jruby-complete\target\jruby-complete-9.1.3.0-SNAPSHOT.jar --command jgem install capybara-webkit-1.11.1.gem
Building native extensions. This could take a while...
uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/ext/ext_conf_builder.rb:56: warning: Tempfile#unlink or delete called on open file; ignoring
Successfully installed capybara-webkit-1.11.1
1 gem installed |
@joankaradimov very much appreciated as jruby-complete.jar had some issues with installing native gems in the past. yes, setting GEM_HOME is needed for jruby-complete. again, thanks. |
@headius About that warning message - I'm having some issues with finding a single place where to raise the |
bda57ff
to
a4c78b9
Compare
I've updated the PR with some changes. They are mostly in As for my progress - it's been very limited. Long story short - I don't think it's possible to have adequate error messages on Windows. My idea for the warning message was to let the configuration phase pass. And fail only when the actual Makefile is generated. The idea is to not limit people in the usage of the What I did so far was test various gems with So I'm thinking of switching environments to Linux and continuing there. This practically means Windows users will never see a clear message indicating that the issue is jRuby's lack of C extensions. Will that be acceptable? |
@mkristian @headius you guys have been interacting on this one. Is the latest rounds of changes: a) safe (releasing in next week or so?) b) good to go? |
@enebo from my side this was OK |
I think we can go with this as a nice incremental step for 9.1.3.0. 👍 |
In the project I'm working on we've successfully been using Capybara with JRuby. We've considered switching drivers to Capybara WebKit as it is emerging as one of the faster Capybara drivers. In our case we have measured it is between 2 and 3 times faster (compared to Selenium/Firefox and Poltergeist/PhantomJS).
It has a quirk, though. It uses
mkmf
to build some C code. The code in question is not a native extension and does not depend in any way on Ruby's C extension API. But because of the lack ofmkmf
it will not install under JRuby. It will run just fine if a pre-built gem is manually copied, though.There is this discussion: thoughtbot/capybara-webkit#725, but nothing seems to have come out of it. Further - a friend who develops this gem used to do something similar. Eventually he did fix his JRuby compatibility. But he did point me to libv8 which does it too. There are probably other examples out there of which I am not aware of.
Knowing that JRuby's goal is compatibility, I'd like to suggest the [re]introduction of
mkmf
. Usingmkmf
for stuff other than C extensions is not perfect and I'll not try to defend it. But people seem to be doing it.The major flaw that I can see in this pull request is that the error message:
NotImplementedError: JRuby does not support native extensions
... is no longer present on stderr for the usual cases. It is replaced by the less explicit:
Check the mkmf.log file for more details.
TL;DR
People use
mkmf
for C code which is not a native ruby extension. The idea is to make this compatible with JRuby.