Make JRuby compatible with capybara-webkit (and possibly other gems) #4031

Merged
merged 1 commit into from Aug 12, 2016

Conversation

Projects
None yet
4 participants
@joankaradimov
Contributor

joankaradimov commented Jul 26, 2016

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

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jul 26, 2016

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jul 26, 2016

Member

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.

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

This comment has been minimized.

Show comment
Hide comment
@joankaradimov

joankaradimov Jul 26, 2016

Contributor

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?

Contributor

joankaradimov commented Jul 26, 2016

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

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jul 26, 2016

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@joankaradimov

joankaradimov Jul 26, 2016

Contributor

I'll look into it.

Contributor

joankaradimov commented Jul 26, 2016

I'll look into it.

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Jul 26, 2016

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.

Member

mkristian commented Jul 26, 2016

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

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jul 26, 2016

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Jul 26, 2016

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.

Member

mkristian commented Jul 26, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@joankaradimov

joankaradimov Jul 27, 2016

Contributor

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

joankaradimov commented Jul 27, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Jul 27, 2016

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.

Member

mkristian commented Jul 27, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@joankaradimov

joankaradimov Jul 27, 2016

Contributor

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

Contributor

joankaradimov commented Jul 27, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@joankaradimov

joankaradimov Jul 28, 2016

Contributor

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?

Contributor

joankaradimov commented Jul 28, 2016

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

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo Aug 11, 2016

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Aug 11, 2016

Member

@enebo from my side this was OK

Member

mkristian commented Aug 11, 2016

@enebo from my side this was OK

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Aug 12, 2016

Member

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

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

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details

@headius headius added this to the JRuby 9.1.3.0 milestone Aug 12, 2016

@headius headius added the JRuby 9000 label Aug 12, 2016

@headius headius referenced this pull request in thoughtbot/capybara-webkit Aug 12, 2016

Closed

install error under jruby 1.7.18 #725

@joankaradimov joankaradimov deleted the joankaradimov:capybara-webkit-compatibility branch Jan 3, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment