Skip to content
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

Merged
merged 1 commit into from
Aug 12, 2016

Conversation

joankaradimov
Copy link
Contributor

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 of mkmf 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. Using mkmf 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.

@headius
Copy link
Member

headius commented Jul 26, 2016

I agree. mkmf should work to generate makefiles, since they have other uses in gems than building C exts. Will review today.

@headius
Copy link
Member

headius commented Jul 26, 2016

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.

@joankaradimov
Copy link
Contributor Author

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 mkmf. But my approach was to take mkmf straight from Ruby 2.3.0 and touch nothing in it.

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?

@headius
Copy link
Member

headius commented Jul 26, 2016

@joankaradimov We do maintain a lightly-patched stdlib fork at https://github.com/jruby/ruby in the jruby- branches. Making a modification to fast-fail in mkmf would be fine, if there's a reliable way to do it (e.g. without rejecting non-ext extconfs).

@joankaradimov
Copy link
Contributor Author

I'll look into it.

@mkristian
Copy link
Member

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.

@headius
Copy link
Member

headius commented Jul 26, 2016

@mkristian You are just saying that non-ext C libs built with extconf/mkmf will fail when installing from inside a jar, yes?

@mkristian
Copy link
Member

@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.

@joankaradimov
Copy link
Contributor Author

@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 C:\dev\jruby\maven\jruby-complete\target\jruby-complete-9.1.3.0-SNAPSHOT.jar.

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 GEM_HOME is set, all works fine:

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

@mkristian
Copy link
Member

@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.

@joankaradimov
Copy link
Contributor Author

@headius About that warning message - I'm having some issues with finding a single place where to raise the NotImplementedError, "C extensions are not supported". I'll need to dig some more.

@joankaradimov joankaradimov force-pushed the capybara-webkit-compatibility branch from bda57ff to a4c78b9 Compare July 28, 2016 16:34
@joankaradimov
Copy link
Contributor Author

I've updated the PR with some changes. They are mostly in RbConfigLibrary.java.

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 have_(func|library|macro|header|*) functions.

What I did so far was test various gems with gem install xxx --platform=ruby. However, none of them pass the configuration phase on Windows. Each with a different problem. I tried fixing some of them, but the amount of incompatibilities is overwhelming. I use MSYS2 which is as good as it gets when it comes to POSIX compatibility.

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?

@enebo
Copy link
Member

enebo commented Aug 11, 2016

@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?

@mkristian
Copy link
Member

@enebo from my side this was OK

@headius
Copy link
Member

headius commented Aug 12, 2016

I think we can go with this as a nice incremental step for 9.1.3.0. 👍

@headius headius merged commit ce21a14 into jruby:master Aug 12, 2016
@headius headius added this to the JRuby 9.1.3.0 milestone Aug 12, 2016
@joankaradimov joankaradimov deleted the capybara-webkit-compatibility branch January 3, 2018 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants