ruby / psych

A libyaml wrapper for Ruby
MIT License
566 stars 205 forks source link

Psych 5.1.1 fails to load on JRuby 9.x #655

Closed jsvd closed 1 year ago

jsvd commented 1 year ago

[EDIT] the issue also happens with 9.3.11.0 and the latest daily snapshot of 9.4.4.0

The new psych gem version 5.1.1 doesn't work correctly when installed in JRuby , I've tried only with 9.4.2.0 so far:

/tmp ❯ tar -zxf ~/Downloads/jruby-dist-9.4.2.0-bin.tar.gz

/tmp ❯ cd jruby-9.4.2.0 

/tmp/jruby-9.4.2.0 ❯ bin/jruby -v
jruby 9.4.2.0 (3.1.0) 2023-03-08 90d2913fda OpenJDK 64-Bit Server VM 17.0.4.1+1 on 17.0.4.1+1 +jit [arm64-darwin]

/tmp/jruby-9.4.2.0 ❯ bin/jruby -S gem install psych
Fetching psych-5.1.1-java.gem
  jar dependencies for psych-5.1.1-java.gemspec . . .
Installing gem 'ruby-maven' . . .
Fetching ruby-maven-3.3.13.gem
Fetching ruby-maven-libs-3.3.9.gem
Successfully installed ruby-maven-libs-3.3.9
Successfully installed ruby-maven-3.3.13
Parsing documentation for ruby-maven-libs-3.3.9
Installing ri documentation for ruby-maven-libs-3.3.9
Parsing documentation for ruby-maven-3.3.13
Installing ri documentation for ruby-maven-3.3.13
Done installing documentation for ruby-maven-libs, ruby-maven after 0 seconds
Parsing documentation for ruby-maven-3.3.13
Parsing documentation for ruby-maven-libs-3.3.9
Done installing documentation for ruby-maven, ruby-maven-libs after 0 seconds

using maven for the first time results in maven
downloading all its default plugin and can take time.
as those plugins get cached on disk and further execution
of maven is much faster then the first time.

2023-10-11T14:39:15.073+01:00 [main] WARN FilenoUtil : Native subprocess control requires open access to the JDK IO subsystem
Pass '--add-opens java.base/sun.nio.ch=ALL-UNNAMED --add-opens java.base/java.io=ALL-UNNAMED' to enable.
      org.snakeyaml:snakeyaml-engine:2.7:compile
Successfully installed psych-5.1.1-java
Parsing documentation for psych-5.1.1-java
Installing ri documentation for psych-5.1.1-java
Done installing documentation for psych after 0 seconds
1 gem installed

/tmp/jruby-9.4.2.0 ❯ bin/jruby -S gem install psych
NameError: cannot load (ext) (org.jruby.ext.psych.PsychLibrary)
          load_ext at org/jruby/ext/jruby/JRubyUtilLibrary.java:216
            <main> at /private/tmp/jruby-9.4.2.0/lib/ruby/gems/shared/gems/psych-5.1.1-java/lib/psych.rb:7
           require at org/jruby/RubyKernel.java:1057
           require at /private/tmp/jruby-9.4.2.0/lib/ruby/stdlib/rubygems/core_ext/kernel_require.rb:85
         load_yaml at /private/tmp/jruby-9.4.2.0/lib/ruby/stdlib/rubygems.rb:610
         load_file at /private/tmp/jruby-9.4.2.0/lib/ruby/stdlib/rubygems/config_file.rb:346
        initialize at /private/tmp/jruby-9.4.2.0/lib/ruby/stdlib/rubygems/config_file.rb:189
               new at org/jruby/RubyClass.java:904
  do_configuration at /private/tmp/jruby-9.4.2.0/lib/ruby/stdlib/rubygems/gem_runner.rb:71
               run at /private/tmp/jruby-9.4.2.0/lib/ruby/stdlib/rubygems/gem_runner.rb:33
            <main> at /private/tmp/jruby-9.4.2.0/bin/jgem:21
              load at org/jruby/RubyKernel.java:1091
            <main> at /private/tmp/jruby-9.4.2.0/bin/gem:4
jsvd commented 1 year ago

@headius sorry for the ping here, just wanted to raise awareness of the impact of the changes between 5.1.0 and 5.1.1.

dometto commented 1 year ago

It looks like 5.1.1 was released to fix an issue on JRuby in 5.1.0, but that the Java artifacts were not rebuilt. PsychLibrary.java is the exact same file on both versions of psych:

$ md5sum ~/.rvm/gems/jruby-9.4.0.0@gollum/gems/psych-5.1.0-java/ext/java/org/jruby/ext/psych/PsychLibrary.java
445fce33df726a26317e2bbab2b1bee3
md5sum  ~/.rvm/gems/jruby-9.4.0.0/gems/psych-5.1.1-java/ext/java/org/jruby/ext/psych/PsychLibrary.java 
445fce33df726a26317e2bbab2b1bee

(This occurred to me because I remembered it happening before: https://github.com/ruby/psych/issues/598)

headius commented 1 year ago

@dometto Oh interesting! This makes some sense; if the Java bits were not updated, then they are not referencing the new path to the properties file. That would explain why my stepping through the code was off by several lines; it was the wrong code!

@hsbt I think we need a new release that includes updated JRuby bits. They did not get rebuilt for 5.1.1 so they are broken in that release. Rebuilding them requires JRuby 9.4.1.0. I updated CI to use 9.4.1.0 for testing until we can fully drop support for the older Region API in Joni.

headius commented 1 year ago

@hsbt Perhaps we can also chat about how to make this easier in the future, perhaps with a CI job to cleanly build all the gem targets every push.

hsbt commented 1 year ago

@headius I built psych-5.1.1 with JRuby 9.4.3.0. Should I rebuild it with JRuby 9.4.1.0? I'm not sure what's the problem of this case.

headius commented 1 year ago

@hsbt Perhaps you were not on master HEAD? Looking at comments above, PsychParser.java did not change between the two releases. I am confused as well.

hsbt commented 1 year ago

I'm also not sure why they are same.

I'll release 5.1.1.1 with this:

$ md5sum ~/.local/share/gem/gems/psych-5.1.1-java/ext/java/org/jruby/ext/psych/PsychParser.java
1b689b170835a983f5fe3cc5da1e0d09  /Users/hsbt/.local/share/gem/gems/psych-5.1.1-java/ext/java/org/jruby/ext/psych/PsychParser.java
hsbt commented 1 year ago

Released https://github.com/ruby/psych/releases/tag/v5.1.1.1

headius commented 1 year ago

@hsbt @jsvd @dometto @olleolleolle The new gem appears to work correctly! Please verify!

olleolleolle commented 1 year ago

Confirmed that I could run my test suite, and release https://rubygems.org/gems/gemstash/versions/2.7.1-java with it. Thanks!