Re-enable specs to run via JRuby-provided bundler

[chadlwilson]

This is work-in-progress to get the tests running/passing again against latest JRuby.

Getting tests/specs passing again

• Resolve the issue in 477502c
  • Root problem seems to be that running the Rake ruby task ended up trying to execute an incorrect Ruby binary via ENV['RUBY] when running inside JRuby, and also was trying to run system ruby when run with FULL_BIN_PATH, and thus dependent on the environment around it (rather than how Maven was configured with jruby jars).
• Resolve errors/warnings with specs on modern Rubies
• Remove unnecessary/outddated cruft from the Maven build process
• Update dependencies where possible
• Fix other test failures
  • Moves Rack back to 2.2.9 as the last known validated-in-real-world version (?) which passes the tests.
  • Drops rest for Rubygems-based "magic comment" install which seems difficult to get to work now that rubygems is automatically loaded during init on modern Rubies

Changes in non-test functionality

• Change jruby-core dependency back to provided scope, to allow for users why may be using jruby-complete. BYO JRuby, basically.
• Remove Log4JLogger (1.x) which is EOL, insecure and should not be used
• Drops loadpath hacks/workarounds for misbehaving J2EE servers in booter.rb which causes problems with getting tests to pass and seems extremely legacy now.

[headius]

I'll have a look at the Input failures soon.

[headius]

I suppose that's what I get for not getting tests running before making changes. Here's the patch for JRuby::Rack::Input:

diff --git a/src/main/java/org/jruby/rack/ext/Input.java b/src/main/java/org/jruby/rack/ext/Input.java
index e680e79..2cdd8e3 100644
--- a/src/main/java/org/jruby/rack/ext/Input.java
+++ b/src/main/java/org/jruby/rack/ext/Input.java
@@ -172,7 +172,7 @@ public class Input extends RubyObject {
                 if ( buffer != null ) {
                     buffer.clear();
                     try {
-                        int unused = (int) CONCAT_WITH_CODERANGE.invokeExact(new ByteList(bytes, false), StringSupport.CR_UNKNOWN);
+                        int unused = (int) CONCAT_WITH_CODERANGE.invokeExact(buffer, new ByteList(bytes, false), StringSupport.CR_UNKNOWN);
                     } catch (Throwable t) {
                         Helpers.throwException(t);
                     }

[headius]

@jlahtinen The patch above should work properly... I botched it before 🤦.

[headius]

@chadlwilson I merged that fix to master so if you rebase you'll pick it up.

[chadlwilson]

Hmm, there's a few suspicious bits around the place like the below, so perhaps these specs requiring a new rack version to be dynamically installed or use of JRuby home in tmpdir weren't working for a long time - not sure how long since TravisCI was passing :D

jruby-rack/.travis.yml

Lines 38 to 39 in 5c060ae

	# NOTE: RubyGems 2.4.x fails us at application_spec's gem_install_rack_unless_installed !
	- (jruby --1.9 -S gem update --system 2.2.3) || true

jruby-rack/src/main/ruby/jruby/rack/booter.rb

Lines 176 to 192 in d3ee8f0

	def adjust_load_path
	require 'jruby'
	# http://kenai.com/jira/browse/JRUBY_RACK-8 If some containers do
	# not allow proper detection of jruby.home, fall back to this
	tmpdir = ENV_JAVA['java.io.tmpdir']
	if JRuby.runtime.instance_config.jruby_home == tmpdir
	ruby_paths = # mirroring org.jruby.runtime.load.LoadService#init
	if JRUBY_VERSION >= '1.7.0'
	# 2.0 is 1.9 as well and uses the same setup as 1.9 currently ...
	%w{ site_ruby shared } << ( JRuby.runtime.is1_9 ? '1.9' : '1.8' )
	else # <= JRuby 1.6.8
	if JRuby.runtime.is1_9
	%w{ site_ruby/1.9 site_ruby/shared site_ruby/1.8 1.9 }
	else
	%w{ site_ruby/1.8 site_ruby/shared 1.8 }
	end
	end

[enebo]

@chadlwilson that first loaderror is pretty odd. If I try and load it I can see it:

java -jar jruby-complete-9.4.7.0.jar -r jruby -e 'p $LOADED_FEATURES'
["java.rb", "jruby/util.rb", "enumerator.rb", "rational.rb", "complex.rb", "thread.rb", "fiber.rb", "ruby2_keywords.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/unicode_normalize.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rbconfig.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/compatibility.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/defaults.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/deprecate.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/errors.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/unknown_command_spell_checker.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/exceptions.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/basic_specification.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/stub_specification.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/platform.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/util/list.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/version.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/requirement.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/specification.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/jars/post_install_hook.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/jar_install_post_install_hook.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/defaults/jruby.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/defaults/operating_system.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/util.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/dependency.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/core_ext/kernel_gem.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/monitor.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/core_ext/kernel_warn.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/rubygems/path_support.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/version.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/core_ext/name_error.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/levenshtein.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/jaro_winkler.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/spell_checker.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/spell_checkers/name_error_checkers/class_name_checker.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/spell_checkers/name_error_checkers/variable_name_checker.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/spell_checkers/name_error_checkers.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/spell_checkers/method_name_checker.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/spell_checkers/key_error_checker.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/spell_checkers/null_checker.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/tree_spell_checker.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/spell_checkers/require_path_checker.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/spell_checkers/pattern_key_name_checker.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean/formatter.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/did_you_mean.rb", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib/jruby.rb"]

If I had to guess perhaps something is changing $LOAD_PATH?

java -jar jruby-complete-9.4.7.0.jar -r jruby -e 'p $LOAD_PATH'
["uri:classloader:/META-INF/jruby.home/lib/ruby/3.1/site_ruby", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib"]

[enebo]

Unrelated to jruby not loading but all that code after that point assumes we are either Ruby 1.8 or Ruby 1.9 implementation but we have be a 2+ impl for like a decade so I think that code is going to be broken. In my previous comment if you see site_ruby it is is in ruby/${ruby_major}/site_ruby and not shared. I think there will be an opportunity to delete a lot of that method.

[enebo]

@chadlwilson I am also guessing the booter does not remove the include directory so require jruby.rb probably would normall work but this spec is doing something special. Random thoughts:

1. no point in making require 'jruby' be in the method. It is so likely to get called by something else in this particular library it may as well just be at top of file.
2. Remove all the version checking which only leaves the jruby_home call (which is too bad because that is what will still require jruby as a require.
3. What is the appropriate paths to index on? In this case:

uri:classloader:/META-INF/jruby.home/lib/ruby/3.1/site_ruby", "uri:classloader:/META-INF/jruby.home/lib/ruby/stdlib"

it is ``["3.1/site_ruby", "stdlib"]`. This may need rbconfig to get version so this is more future proof.

4. Does it make sense to even have this method any more? @kares might know if newer web containers are well-behaved or perhaps whether we can just back off and see if a websphere user exists and needs it fixed again. My opinion is this code is ancient and it is a good opportunity to try and delete some stuff. This method has not worked for anyone using this since we supported Ruby 1.9 (I think this because we changed lib/ruby layout during Ruby 2).

Anyways some thoughts to try and help. This work is incredible and we need to get jruby-rack back into shape.

[chadlwilson]

Thanks, yeah. I tried changing the paths after your message but it still seemed to be failing in the same way.

Actually after reading your message it reminded me that I am trying to do this on master which is actually missing years of fixes to 1.1 right now currently pending in #253

My thinking was to try and get the tests passing to give more confidence in merging that PR, but given 1.1 is probably more stable than 1.2/master (right now) and may indeed have test fixes, I might have that around the wrong way and should at least look at that branch.

It's a separate topic to this specific case (where deleting the old code seems most advisable) but perhaps worth keeping in mind for some of the remaining failures.

[chadlwilson]

The last remaining test failing is

jruby-rack/src/spec/ruby/rack/application_spec.rb

Lines 313 to 328 in cb35667

	it "loads specified version of rack", :lib => :stub do
	gem_install_unless_installed 'rack', '2.1.4'
	set_config 'jruby.runtime.env', 'false'
	script = "# rack.version: ~>2.1.0\n Proc.new { 'proc-rack-app' }"
	app_factory.setRackupScript script
	@runtime = app_factory.newRuntime
	@runtime.evalScriptlet "ENV['GEM_HOME'] = #{ENV['GEM_HOME'].inspect}"
	@runtime.evalScriptlet "ENV['GEM_PATH'] = #{ENV['GEM_PATH'].inspect}"
	app_factory.checkAndSetRackVersion(@runtime)
	@runtime.evalScriptlet "require 'rack'"
	should_eval_as_eql_to "Rack.release if defined? Rack.release", '2.1'
	should_eval_as_eql_to "Gem.loaded_specs['rack'].version.to_s", '2.1.4'
	end

I am not a Ruby expert, but I wonder if the underlying functionality is no longer required now.

This functionality appears to allow jruby-rack to dynamically install/load/require a specific Rack version from config.ru using Rubygems alone, using a magic comment - without Gemfile/bundler being available. This was introduced in 32b531f

There is also a bundled version of rack which I don't fully understand interactions of. Not quite sure how to proceed on this one, but suffice it to say that after this logic is executed in test setup without error, rack can still not be found

jruby-rack/src/spec/ruby/spec_helper.rb

Lines 96 to 100 in 34f390c

	unless found
	require 'rubygems/dependency_installer'
	installer = Gem::DependencyInstaller.new
	installer.install name, version
	end

...leading to...

[INFO]   1) Java::OrgJrubyRack::DefaultRackApplicationFactory initialized newRuntime loads specified version of rack
[INFO]      Failure/Error: Unable to find org.jruby.rack.DefaultRackApplicationFactory.checkAndSetRackVersion(org/jruby/rack/DefaultRackApplicationFactory.java to read failed line
[INFO]
[INFO]      Java::JavaLang::RuntimeException:
[INFO]        org.jruby.exceptions.LoadError: (MissingSpecError) Gem::MissingSpecError

when running

jruby-rack/src/main/java/org/jruby/rack/DefaultRackApplicationFactory.java

Line 430 in e492263

runtime.evalScriptlet("gem 'rack', '"+ rackVersion +"' if defined? gem");

[enebo]

@chadlwilson sorry I missed your last comments. I am not sure if this is still valid or not and I do not see any obvious rack documentation on this (although to be honest I only looked a for a few minutes). It is definitely possible to run JRuby without bundler but has rack just decided people need bundler? Does the magic comments still work? Maybe @kares knows.

A possible way to test this would be to make a simple config.ru and trying it with C Ruby and see if it works with the magic comment. The fact this is just embedded code in Java does not change that this could just be invoked from Ruby. If it works then we probably need to change how we require the gem (like something is missing or gem api changed over time and we are not calling something). If it doesn't then it can be nuked from orbit.

[chadlwilson]

No worries @enebo

I think the magic comment is a jruby-rack invention, perhaps specific to a Jruby context due jruby-rack bundling an opinionated rack version: https://github.com/jruby/jruby-rack/blob/master/History.md#1112-281112

In the README, and

with a plain Rack app when config.ru is evaluated it already assumes Rack to be loaded hovewer it's common practice to bundle exec rackup such scripts while Bundler loads a version of Rack that might be different than JRuby-Rack's bundled Rack version.

thus for now we support a magic comment :
# rack.version = ~>1.3.6
...
with "special" support for Bundler as well :
# encoding: UTF-8
# rack.version = bundler

I'm a little bit lost here as I don't know rackup (the app I use uses init.rb style), but it seems that the intent here is to allow an arbitrary rack version with an external bundler rackup, but then use the magic comment to run whatever the jruby-rack bundled version is.

So as long as rackup is still useful, and jruby-rack still bundles a rack version, it seems conceptually possible that the functionality is needed, but I can't for the life of me figure out how to empirically test the various cases and work backwards to how the test should behave 😅 (definitely a skill issue, rack is not something I've put much time into understanding)

[enebo]

@chadlwilson I decided to look a little bit. In trying to just run rake instead of full maven run I had to tweak:

diff --git a/Rakefile b/Rakefile
index 599ce7b..19f50a2 100644
--- a/Rakefile
+++ b/Rakefile
@@ -110,7 +110,8 @@ task :speconly => [ :resources, :test_resources ] do
     spec = ENV['SPEC'] || File.join(Dir.getwd, "src/spec/ruby/**/*_spec.rb")
     opts = opts.split(' ').push *FileList[spec].to_a
 
-    args = ENV['RUBY'].split(' ') + %w{-Isrc/spec/ruby -rbundler/setup -S rspec}
+    ruby = ENV['RUBY'] || 'jruby'
+    args = ruby.split(' ') + %w{-Isrc/spec/ruby -rbundler/setup -S rspec}
     sh *args, *opts
   end
 end

Then rake test runs but I get 3 errors all of which is failing to find servlet class so servlet_30? fails to work right. I guess this may be a stretch goal but it would be nice to just run rake for local dev. Maven does repro locally for me so something with maven it properly loading the servlet API before running rake.

[enebo]

Ah I see ... this will generate a full Java CMD and not use jruby itself. This also explains why servlets classes are not found since it builds up a classpath while in maven. Ignore my previous comment. It would be nice just to type rake but this was not designed around being able to do that

[chadlwilson]

Ah I see ... this will generate a full Java CMD and not use jruby itself. This also explains why servlets classes are not found since it builds up a classpath while in maven. Ignore my previous comment. It would be nice just to type rake but this was not designed around being able to do that

Yeah, I agree. The feedback loop is pretty painful right now. I was just going the path of least resistance since things were not in a working state, and I had no access to prior builds to even know when the tests last passed, against which versions of things and on which branches :-) This branch also does not have a whole lot of fixes from https://github.com/jruby/jruby-rack/tree/1.1-stable just yet, so it's possible that will change things again.

It's also possible that a change somewhere on that branch not merged to master did something to address the tests here, or even allow them to work with rake, so I should probably pause and take a look at that later.

[chadlwilson]

diff --git a/Rakefile b/Rakefile
index 599ce7b..19f50a2 100644
--- a/Rakefile
+++ b/Rakefile
@@ -110,7 +110,8 @@ task :speconly => [ :resources, :test_resources ] do
     spec = ENV['SPEC'] || File.join(Dir.getwd, "src/spec/ruby/**/*_spec.rb")
     opts = opts.split(' ').push *FileList[spec].to_a
 
-    args = ENV['RUBY'].split(' ') + %w{-Isrc/spec/ruby -rbundler/setup -S rspec}
+    ruby = ENV['RUBY'] || 'jruby'
+    args = ruby.split(' ') + %w{-Isrc/spec/ruby -rbundler/setup -S rspec}
     sh *args, *opts
   end
 end

Thx, I've merged this - that's better.

Then rake test runs but I get 3 errors all of which is failing to find servlet class so servlet_30? fails to work right. I guess this may be a stretch goal but it would be nice to just run rake for local dev. Maven does repro locally for me so something with maven it properly loading the servlet API before running rake.

Interesting, this provides a hint I guess to why the other test is failing, as it must be to do with the wrapping environment mvn -> rake -> mvn or something I changed here. I'll take another look tomorrow perhaps with this in mind...

[enebo]

The ENV['RUBY'] when executed from maven is:

/home/enebo/Applications/jdks/jdk8u402-b06/jre/bin/java -cp /home/enebo/.m2/repository/org/jruby/jruby-complete/9.4.8.0-SNAPSHOT/jruby-complete-9.4.8.0-SNAPSHOT.jar:/home/enebo/work/gems/jruby-rack/target/test-classes:/home/enebo/work/gems/jruby-rack/target/classes:/home/enebo/.m2/repository/org/jruby/jruby-core/9.4.8.0-SNAPSHOT/jruby-core-9.4.8.0-SNAPSHOT.jar:/home/enebo/.m2/repository/org/apache/geronimo/specs/geronimo-servlet_3.0_spec/1.0/geronimo-servlet_3.0_spec-1.0.jar:/home/enebo/.m2/repository/org/apache/geronimo/specs/geronimo-jsp_2.2_spec/1.2/geronimo-jsp_2.2_spec-1.2.jar:/home/enebo/.m2/repository/org/apache/geronimo/specs/geronimo-el_2.2_spec/1.0.4/geronimo-el_2.2_spec-1.0.4.jar:/home/enebo/.m2/repository/org/apache/geronimo/specs/geronimo-jms_1.1_spec/1.1.1/geronimo-jms_1.1_spec-1.1.1.jar:/home/enebo/.m2/repository/commons-logging/commons-logging-api/1.1/commons-logging-api-1.1.jar:/home/enebo/.m2/repository/org/slf4j/slf4j-api/1.7.2/slf4j-api-1.7.2.jar:/home/enebo/.m2/repository/log4j/log4j/1.2.17/log4j-1.2.17.jar:/home/enebo/.m2/repository/org/springframework/spring-core/3.2.10.RELEASE/spring-core-3.2.10.RELEASE.jar:/home/enebo/.m2/repository/commons-logging/commons-logging/1.1.3/commons-logging-1.1.3.jar:/home/enebo/.m2/repository/javax/activation/activation/1.1/activation-1.1.jar:/home/enebo/.m2/repository/org/jruby/jruby-complete/9.4.8.0-SNAPSHOT/jruby-complete-9.4.8.0-SNAPSHOT.jar org.jruby.Main

So it knows how to acculmulate all those jar files into a java command. My patch makes things work until you need one of these jars (since just calling jruby by itself is not adding them). It is fascinating that only three things fail though. If whatever jar which includes javax.servlet.AsyncContext then MAYBE things will work:

  def servlet_30?
    return @@servlet_30 unless @@servlet_30.nil?
    @@servlet_30 = !! ( Java::JavaClass.for_name('javax.servlet.AsyncContext') rescue nil )
  end

So the specs could load the jar(s) via require...but then we would have this odd data replication since we specify deps in pom.xml and we would also have requires elsewhere. I might have opened a can of worms :)

[chadlwilson]

The ENV['RUBY'] when executed from maven is:

/home/enebo/Applications/jdks/jdk8u402-b06/jre/bin/java -cp /home/enebo/.m2/repository/org/jruby/jruby-complete/9.4.8.0-SNAPSHOT/jruby-complete-9.4.8.0-SNAPSHOT.jar:/home/enebo/work/gems/jruby-rack/target/test-classes:/home/enebo/work/gems/jruby-rack/target/classes:/home/enebo/.m2/repository/org/jruby/jruby-core/9.4.8.0-SNAPSHOT/jruby-core-9.4.8.0-SNAPSHOT.jar:/home/enebo/.m2/repository/org/apache/geronimo/specs/geronimo-servlet_3.0_spec/1.0/geronimo-servlet_3.0_spec-1.0.jar:/home/enebo/.m2/repository/org/apache/geronimo/specs/geronimo-jsp_2.2_spec/1.2/geronimo-jsp_2.2_spec-1.2.jar:/home/enebo/.m2/repository/org/apache/geronimo/specs/geronimo-el_2.2_spec/1.0.4/geronimo-el_2.2_spec-1.0.4.jar:/home/enebo/.m2/repository/org/apache/geronimo/specs/geronimo-jms_1.1_spec/1.1.1/geronimo-jms_1.1_spec-1.1.1.jar:/home/enebo/.m2/repository/commons-logging/commons-logging-api/1.1/commons-logging-api-1.1.jar:/home/enebo/.m2/repository/org/slf4j/slf4j-api/1.7.2/slf4j-api-1.7.2.jar:/home/enebo/.m2/repository/log4j/log4j/1.2.17/log4j-1.2.17.jar:/home/enebo/.m2/repository/org/springframework/spring-core/3.2.10.RELEASE/spring-core-3.2.10.RELEASE.jar:/home/enebo/.m2/repository/commons-logging/commons-logging/1.1.3/commons-logging-1.1.3.jar:/home/enebo/.m2/repository/javax/activation/activation/1.1/activation-1.1.jar:/home/enebo/.m2/repository/org/jruby/jruby-complete/9.4.8.0-SNAPSHOT/jruby-complete-9.4.8.0-SNAPSHOT.jar org.jruby.Main

So it knows how to acculmulate all those jar files into a java command. My patch makes things work until you need one of these jars (since just calling jruby by itself is not adding them). It is fascinating that only three things fail though. If whatever jar which includes javax.servlet.AsyncContext then MAYBE things will work:

Yeah, on current deps it will be /home/enebo/.m2/repository/org/apache/geronimo/specs/geronimo-servlet_3.0_spec/1.0/geronimo-servlet_3.0_spec-1.0.jar but given all the other provided dependencies, it's a bit surprising there aren't more things actually being used. I think perhaps for the examples or appraisals or whatnot which I haven't looked at.

So the specs could load the jar(s) via require...but then we would have this odd data replication since we specify deps in pom.xml and we would also have requires elsewhere. I might have opened a can of worms :)

Since rake already calls some mvn tasks to set things up, probably just as easy to get it to to dump a classpath for the test lifecycle somehow and then use that in the Rakefile. But ya, probably a can of worms :-)

[chadlwilson]

The problem with the last failing test when run via mvn test (compared to bundle exec rake test where it is OK) seems to be that gems are being loaded from a different place than they have been earlier installed to by the test code.

When running via Maven, the test installs the "override" rack version to a place like GEM_HOME=/Users/chad/Projects/community/jruby-rack/target/rubygems inside the maven build.directory.

Before requiring rubygems as part of initializating rack, the code then tries to override GEM_HOME within the newly created "under-test" jruby runtime, however it is unsuccessful in altering the Gem.dir which remains uri:classloader:/META-INF/jruby.home/lib/ruby/gems/shared inside the jruby-core jar.

jruby-rack/src/spec/ruby/rack/application_spec.rb

Lines 320 to 324 in 39cdba5

	@runtime.evalScriptlet "ENV['GEM_HOME'] = #{ENV['GEM_HOME'].inspect}"
	@runtime.evalScriptlet "ENV['GEM_PATH'] = #{ENV['GEM_PATH'].inspect}"
	app_factory.checkAndSetRackVersion(@runtime)
	@runtime.evalScriptlet "require 'rack'"

These first two lines dont seem to have the intended effect of changing where the runtime will load Gems from to match what rake is running with. So it tries to load from a different location than the Gem was earlier installed, and thus you get MissingSpecError.

By contrast, when running tests directly via rake the GEM_HOME/Gem.dir ends up ones external jruby installation (e.g /Users/chad/.local/share/mise/installs/ruby/jruby-9.4.7.0/lib/ruby/gems/shared) and since it matches defaults it works OK. For some reason, this problem doesn't seem to affect something similar done in a Bundler test, but haven't dug into why that one works.

@enebo or @headius - once against excusing my JRuby ignorance, does this ring a bell to you, as to why the below would be ineffective in actually changing Gem.dir and affecting where RubyGems try to load from? Possibly it has already been loaded or this logic only worked on earlier Ruby/JRuby versions.

@runtime.evalScriptlet "ENV['GEM_HOME'] = #{ENV['GEM_HOME'].inspect}"

[enebo]

@chadlwilson I realize after reading this that rubygems is run as part of initialization and this used to not be the case (we changed to this whenever C Ruby made the change). This spec starts a new Ruby instances which loads rubygems then it sets GEM_HOME after gems have already loaded. as part of the instance coming up. This explains why it is not seeing the gems.

We can probably make this work by setting java system property jruby.cli.rubygems.enable to false before creating the instance but I am questioning the value of this test in a world where rubygems is just loaded by default. All this test is really doing is verifying Ruby gems can change its home location. I think it would be safe to just remove this test and if there is some future issue about people not being able to customize GEM_HOME we maybe start again.

[enebo]

@chadlwilson to be more explicit I mean this line will load rubygems:

jruby-rack/src/spec/ruby/rack/application_spec.rb

Line 319 in 39cdba5

@runtime = app_factory.newRuntime

[chadlwilson]

Right, that's what I thought might be happening.

The test is really trying to check you can load a custom rack version using the magic comments and needs to change the GEM_HOME to do so between tests. Alternatively it could change it for all tests prior to starting and have the same effect rather than trying to swap it in between tests.

If there is no way to change the Gem.dir after rubygems is loaded what would be the right way to have the GEM_HOME or additional GEM_PATHs set prior to the runtime creation when doing so programmatically like in these tests so it's picked up by the initialization?

[enebo]

@chadlwilson oh yeah I forgot! You can try something like this before the newRuntime call (but you should remove it after the test as well):

java.lang.System.properties.put("jruby.cli.rubygems.enable", "false")

This should prevent it from loading rubygems by default...which is unclear how gems ends up being loaded by the test but it must do it somewhere.

[chadlwilson]

Sadly I cannot find a place to set this (or any related things!) that seems to work. The runtimes are reused between tests, but since it is all happening within one JVM instance launched by mvn test --> rake I actually dont quite understand why the GEM_HOME originally set for the wrapping process doesn't auto-magically propagate through to how JRuby is configured when it is required and/or runtimes created. I tried setting --disable-gems and this System property from rake as well, but I guess something else is loading rubygems earlier than that.

I also tried appending to Gem.path manually via Gem.path.unshift, which shows as updated by the time of the require 'rack' but is not effective in allowing the Gem to load.

Due to my own inexperience, I am leaning towards just deleting this test and leaving it untested, relying solely on the bundler test and assuming that any sane individual trying to do this magic comment stuff is doing so via Bundler/Gemfile rather than relying on a naked Gem install.

[enebo]

@chadlwilson I think excluding and working towards the longer goal of getting jruby-rack back up to date and in shape. We can always figure this out later (pend the spec).

[chadlwilson]

OK, I have removed this one test, which gets things passing now. Ready for review whenever you folks are able.

Next I'll try and tackle getting all the 1.1 fixes/changes merged back onto master with the tests in a known state.

[enebo]

Everything looked so straightforward I just merged this without other reviews (which I am not normally a jruby-rack person but these were all sensible changes). @chadlwilson Thanks. You are well on your way to untangling this thing. We look forward to the next step.