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

Properly compile gems #312

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

camertron
Copy link

I ran into some issues when setting config.compile_gems = true in my Warbler config:

  1. When constructing jar packages, warbler does not compile git-based gems (i.e. any gems specified via the :git or :github options in the Gemfile).
  2. When constructing war packages, warbler does not compile any gems, including git-based ones.
  3. When constructing jar packages, there is no way to specify a Ruby version (i.e. no 1.9-mode support).

This pull request introduces a trait called CompiledGems that addresses 1 and 2 above (see comments in the code for details). For number 3, I had to add this to my warbler config (note that there's no way to specify 2.0 mode, even using this hack):

config.webxml = Warbler::Traits::War::WebxmlOpenStruct.new
config.webxml.jruby.compat.version = '1.9'

I'm mostly submitting this PR to share what I've learned with others who have run into similar issues (doesn't need to be merged). I think there will need to be a more holistic look at warbler (maybe when JRuby 9000 gets released?) to really fix things right. For now, this trait is just a hack to help in the interim.

@jkutner
Copy link
Member

jkutner commented Mar 17, 2015

Thanks for sharing this! I'm sorry you had so much trouble. This looks like a decent solution though, so I may end up merging it. I'll see if I can write up some tests to cover this trait.

You are correct that JRuby 9k may require some overhauling. But we'll definitely continue to support the 1.7.x line for some time.

@camertron
Copy link
Author

Thanks @jkutner! Also many thanks for warbler, it has been an indispensable tool to us :)

@Gauravshah
Copy link

@camertron any reason we are not merging ?

@camertron
Copy link
Author

@Gauravshah it's out of my hands since I don't have merge permissions. @jkutner any idea when this can be merged?

@Gauravshah
Copy link

thanks @camertron I am unable to figure out what people use for jruby on prod. Warbler is not being maintained actively. trinidad has no commits from over 2 years. puma is slow. Not sure what people use now.

@camertron
Copy link
Author

camertron commented Jan 17, 2018

@Gauravshah I think the app server of choice for JRuby is torquebox. AFAIK it's actively maintained by the guys at Red Hat. We used warbler + glassfish for our last JRuby project.

@jkutner
Copy link
Member

jkutner commented Jan 18, 2018

Is this still an issue with Warbler 2.0.4 (which support JRuby 9k)?

@camertron
Copy link
Author

@jkutner I'm not sure, perhaps the others can chime in?

@Gauravshah
Copy link

@jkutner it does not bundle git gems, verified on 2.0.4 version of warbler.

@jkutner
Copy link
Member

jkutner commented Jan 18, 2018

Since this trait doesn't affect anything if you don't use it, I can merge it. But can you confirm that it works? Ideally would like to have a test for it.

@kares
Copy link
Member

kares commented Jan 19, 2018

I think the app server of choice for JRuby is torquebox.
AFAIK it's actively maintained by the guys at Red Hat.

nope, unfortunately, its not the case anymore. its mostly just Puma or .war + Tomcat/Jetty these days ...

@Gauravshah
Copy link

@kares then how do we generate war since warble is not being actively maintained ?

@kares
Copy link
Member

kares commented Jan 19, 2018

@Gauravshah as far as I can tell .war generation works reasonably - have some folks that I am helping out and they're fine with Warbler as is (JRuby 9.1 + Rails 4.2). not 100% sure what exactly your issue is - git gems bundle fine. assuming you're having a problem with git + compiled gems than disable compilation.

I am sure a proper resolution (with tests) will be merged - as noted we do not know whether this piece of PR actually resolves your problem (again not exactly clear what issue you're having) ... if you believe so add a piece of test so we understand. otherwise the minimum you could do is a simple reproducer.

we shall not get into slightly off-topic discussions on open-source project support in a PR thread,
good practice that works for me is to expect 'nothing' from folks maintaining projects (for free) that I am benefiting from and if I run into issues I do try to be helpful or support the project anyway I can ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants