-
Notifications
You must be signed in to change notification settings - Fork 206
Fixes to update Warbler to modern libs and JRuby #559
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
Conversation
Safe mode is long dead, and other args should be in kwargs. Fixes part of jruby#558
|
Fixes here will try to resolve #558. |
This hack appears to have been added around the same time we started to support bundling dependencies from source repos, and may no longer be necessary. My reading of the code is that it was done to work around a bug in bundler when loading itself from a source repo. Removing this hack allows many more tests to pass on JRuby 10 and Bundler 2.6.3.
|
I removed a hack by @nicksieger that apparently worked around some issues with gem specifications' "loaded from" attribute when loading bundler. The logic presented does not properly handle the case where the running bundler is a default gem (as it is in any recent release of JRuby) and I'm not sure if it's necessary anymore in any case. |
|
jbundler-related failures are going to need an update to that library. I've started the process here: jruby/jbundler#104 cc @mkristian I need to move that repo over to jruby org and get push rights with you get a chance. Thank you! |
|
maven-tools 1.2.3 has been released which included a few unreleased fixes. This eliminates some build failures due to it referencing the defunct torquebox gem proxy URL. |
|
With an updated jbundler (only available as a local build) and maven-tools I have failures down to four: The first failure seems to be an issue with the subprocess launch of drb, perhaps using some old pattern for launching processes The second failure is due to The third failure and fourth failure need investigation. |
Don't change the java.specification.version!
JRuby 10 only supports the Java 21 bytecode version "0x41" because there's no later LTS and because 21 is the minimum Java supported. I modify the test here to match those limitations.
ostruct will be moved out of default gems in 3.5, so loading it from stdlib without a gem dependency causes warnings. Those warnings lead to a test failure due to extraneous output.
|
I've dealt with the ostruct issue by adding it as a dependency and using a newer version of rake that doesn't also use it blindly. The bytecode version failure was due to JRuby 10 always using bytecode versions higher than 21, since that's the minimum version it supports. I patched the test to handle that situation. I also modified the way this code tries to specify a specific bytecode version; tweaking This is using jbundler 0.9.5 which is unreleased, so it will just fail hard until that's pushed. |
|
Thanks for this work and for the excellent updates along the way. Easy to learn from, super helpful. |
I'm not positive whether this is a kosher change, given that the spec seems to want bundler's lib files to also be included in the jar, but it may be excluding those files because it is a default gem. In any case, the exe files are included so we'll expect that they are there.
|
The DRbServer used for running some tests remains a little flaky, but with latest changes this should be green on JRuby 10. jbundler 0.9.5 has been released and that project moved to JRuby org control so CI should also be green (or soon). |
|
@headius Perhaps a step "just run 1 version of Bundler" could simplify the look of the CI matrix just a little. That is: deleting "2.6.3" from the string "21, jruby-head, 2.6.3". |
Letting this be open-ended may use a newer jruby-jars than the current JRuby, which breaks testing if that version requires a newer version of Java.
Using a head build breaks because the maven tooling warbler uses cannot handle a snapshot version.
|
@olleolleolle Yeah good idea. No really compelling reason to test against several versions anymore. |
Might be useful later but doesn't add anything during this upgrade.
I'm not sure how this is getting into setup-ruby but because it was installed with a different version, RubyGems complains. Those complaints appear to be causing specs to fail.
|
I ran into issues by using current master to warble a Rails app and also a minimal Sinatra app (with a local snapshot build from jruby-rack 1.3.0) and deploy it to a Tomcat. @headius: I also noticed that CI seems to be green, but failed because |
|
@mjansing Yeah I realize there's still some work needed here. I'll be circling back to this once we have JRuby 10.0.1.0 out. I also see that some of the integration tests are not really passing, so that will get fixed too. We're getting there! If you see small things you can fix, please submit a PR. |
|
@headius Thanks a lot for your changes! Since #561 makes it difficult to test Warbler directly from a Git dependency (you need to build and install the gem manually without using bundler), we published a temporary gem to simplify testing. |
|
Slightly unrelated to this and #554, but as you've probably seen I have been doing some work on jruby-rack to try and get things working with newer rails versions etc, resurrect all the testing etc - although still big gaps with the Rails regression testing to resolve. Warbler is a blind spot for me since the jruby-rack app I maintain (GoCD) uses embedded Jetty and assembles its war files manually itself rather than using warbler. However, if I can help with digging into anything caused by jruby-rack problems or needing jruby-rack support, feel free to ping me and I'll see what I can do. Given the adjacency of the tools, some of the CI challenges here are possibly ones I've had to address earlier for jruby-rack or have some kind of approach to workaround. |
This PR will encompass fixes to get Warbler updated for modern JRuby, modern Ruby, and modern versions of necessary libraries.
Fixes #558.