Replace Unix launcher with simple wrapper that executes jruby.sh #48
Conversation
|
I pushed to master a CI fix you can rebase. |
|
I tested this out by pulling your branch and making a gem from it:
The following sequence demonstrates the new launcher working properly, and supporting flags the original does not like This is looking very good so far, but it seems like some change in the makefile has triggered additional verbose output we need to clean up. |
|
False alarm on that verbose output... running the jruby-launcher install again does NOT output the verbose build stuff. It may be something to do with the old launcher not setting up JRuby properly. |
Only works on Unix because it depends on the sh launcher script
mrnoname1000
force-pushed
the
simple-unix-launcher
branch
from
82e9360 to
964a5a1
Compare
|
@mrnoname1000 I see there are some failures to find the jruby.sh file now that tests are running. Other failures are possibly behavior changes in jruby.sh that were never reflected in the old launcher. So one big thing I think might simplify this would be that the skinny launcher should only run the jruby.sh file that's right next to it in JRuby's bin dir. Perhaps that would help make sure it can find it? Perhaps the build can verify that the jruby.sh file is there? |
|
Are these tests that live in the main JRuby repo? It should just be resolving the path to itself, then executing the sibling jruby.sh but if it doesn't exist it'll error |
|
@mrnoname1000 I believe they are all tests that live in this repository. Have a look at the failed jobs (and I will as well). |
|
Looks like a number of these are failing because the test harness expects some native launcher-only debug flags to be used: jruby_launcher_args passes I'll try to get more of these passing locally. |
|
Aha... and many of them expect to be able to run the launcher in the absence of JRuby! Obviously that won't work when it has to have the shell script. I will also try to make those work better by putting the shell script near the executable. |
|
Yeah we will need to fully reexamine these specs... I'm not sure any of them are valid or supportable directly with the new code. I'm still figuring out where all the pieces are but basically this sets up a dummy JRuby home and then runs the launcher several ways without actually starting JRuby. So there's a jruby.jar, but it's a dummy, and it's in a lib dir but none of the standard library is present. It's only trying to validate that the launcher finds the JRuby jar and builds the proper command lines to exec... which is what the shell launcher does for us now. So for the skinny launcher, the only things we really need to test is that it can locate the sibling jruby.sh file and launch it. We may also want to add some of the native launcher flags like -Xversion for the version of the launcher and -Xcommand to print out the command it is about to exec. The specs are still valid for the Windows launcher, and may be needed for the Rust launcher when that's ready. |
|
FWIW I ran some of our larger JRuby test suites with this new launcher and saw no failures related to the command line. |
|
I have pushed this version as a preview gem, jruby-launcher 2.0.0.pr1, installable by |
|
This patch excludes the specs when not running on Windows. No specs run, but none of them are really valid with the skinny launcher anyway. diff --git a/spec/launcher_spec.rb b/spec/launcher_spec.rb
index 79573c1..1478292 100644
--- a/spec/launcher_spec.rb
+++ b/spec/launcher_spec.rb
@@ -2,7 +2,7 @@ require 'tmpdir'
require File.expand_path('../spec_helper.rb', __FILE__)
load File.expand_path('../../lib/jruby-launcher.rb', __FILE__)
-describe "JRuby native launcher" do
+describe "JRuby native launcher", if: /Windows/.match?(ENV_JAVA['os.name']) do
it "should run org.jruby.Main" do
jruby_launcher_args("").last.should == "org/jruby/Main"
end |
|
I gave it a shot on 9.4 and after installing it it broke my rbenv jruby install: I wanted to test jruby-10-snapshot first but I can't install it there because it needs the fixed launcher for that I think (bad file description error) |
|
@Earlopain Thanks for the report! Can you confirm that the file it claims is missing exists? It really should be there. |
|
Nah, it's definitly gone https://github.com/rbenv/ruby-build/blob/2334633d60b78124fc99dedb32728605f44a4e56/bin/ruby-build#L938 That code has been there since forever rbenv/ruby-build#1. Looks like that PR was the one to add the very first jruby version to ruby-build. |
|
Ugh, why is it deleting the shell script? That basically makes it possible to go back to the script and serves no purpose I can see. It's not a Windows file either so I'm not sure why it's deleted in this function. They need to stop doing that, but the damage is done for anyone with a current installation. I'm going to have to figure out a workaround. |
I'm not sure why this file was being deleted but this code dates way back! Unfortunately deleting this interferes with a new version of the native JRuby launcher we want to release which is dependent on the shell script being present. This PR removes jruby.sh from the `remove_windows_files` function so it can be used by the new launcher. See jruby/jruby-launcher#48 (comment)
|
I just pushed rbenv/ruby-build#2517 to stop deleting the jruby.sh file. That logic is old enough that I would not be surprised if I younger, more naive version of me told them it was okay to do. In any case, it needs to stop. I have also proposed in the past that this launcher gem ship the shell script as well, so that users can update both the native and the shell launchers if a blocking issue is found. We may need to revisit that idea. |
|
@Earlopain perhaps you can manually apply my rbenv patch and confirm it fixes the issue with the skinny launcher? And comment on my pr? |
|
Yeah, that seems to work OK. I'll comment on the PR |
|
I'm going to merge and make the additional changes. |
This assumes
jrubyis located in$JRUBY_HOME, and if it is, itexec's jruby.sh. This is mainly to support shebangs with jruby as the target on non-Linux systems.Fixes #47, but only on Unix.