jruby / warbler

Warbler chirpily constructs .war files of your Ruby applications.
Other
883 stars 204 forks source link

Fix helper class used for config.webxml values #513

Closed JesseChavez closed 2 years ago

JesseChavez commented 2 years ago

This fixes the issue reported in theJRuby repo:

https://github.com/jruby/jruby/issues/7160

Basically the issue was caused due 2 changes in the ostruct repo:

change 1:

commit d39c4e3feab2852f45cff791cf8cd684960c07a1
Refs:
Author:     nobu <nobu@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>
AuthorDate: Mon Jan 5 01:57:26 2015 +0000
Commit:     nobu <nobu@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>
CommitDate: Mon Jan 5 01:57:26 2015 +0000

    ostruct.rb: append suffixes to protected methods

    * lib/ostruct.rb (modifiable?, new_ostruct_member!, table!):
      append suffixes to protected methods so that they will not clash
      with assigned members.  [Fix GH-806]

    git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@49145 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

change 2:

commit 6dbc902092249ae2c75546fd121ba7618d3645c7
Refs: v0.1.0-33-g6dbc902
Author:     Marc-Andre Lafortune <github@marc-andre.ca>
AuthorDate: Mon Sep 14 14:06:49 2020 -0400
Commit:     Marc-Andre Lafortune <github@marc-andre.ca>
CommitDate: Mon Sep 14 16:13:45 2020 -0400

    method_missing is private
olleolleolle commented 2 years ago

@JesseChavez 👋 Hi! This is super-clear, you added good context information in the commit message, good to have links to background information!

I saw that jruby-head tests were failing on something minor, so I started a PR to see if we could work around that for jruby-head. https://github.com/jruby/warbler/pull/514 Edit: Now merged.

olleolleolle commented 2 years ago

Also, the relevant change - adding the ! methods - was made five years ago, I think we should see it as supported.

Edit: So, if you rebase, perhaps we can see this run green on jruby-head, too?

enebo commented 2 years ago

@olleolleolle This is a weird failure. How is the rubygems being run so old if we are running a JRuby 3.1.x codebase which has its own rubygems?

olleolleolle commented 2 years ago

@olleolleolle This is a weird failure. How is the rubygems being run so old if we are running a JRuby 3.1.x codebase which has its own rubygems?

Because the steps that are installing RubyGems were installing a very specific version. I changed it in #514.

headius commented 2 years ago

Good work all around, folks!

headius commented 2 years ago

Because the steps that are installing RubyGems were installing a very specific version. I changed it in #514.

@JesseChavez Could you rebase on current warbler master so we can see that it is green across the board?

JesseChavez commented 2 years ago

@headius , I've rebased it

enebo commented 2 years ago

All green!

olleolleolle commented 2 years ago

Exquisite! I went ahead and merged this change. 🎉 Thanks, @JesseChavez! And thanks all who chimed in!

johnnyb commented 2 years ago

I'm trying to use this (gem installed from git to main repo), but it is now saying, "TypeError: nil is not a string" when trying to evaluate config.webxml.jruby.min.runtimes = 1 in "lib/warbler/traits/rails.rb". new_ostruct_member!("runtimes") is returning nil apparently.

olleolleolle commented 2 years ago

@headius Oh, hi, this last comment looks a lot like the issue you and others have been fighting in OpenStruct. Can you offer any insight?

olleolleolle commented 2 years ago

@johnnyb To make debugging this easier, can you say what jruby that is, and what version of OpenStruct is in use (recently made into a separate gem)?

sebastianguarin commented 2 years ago

@olleolleolle is this change in any version? don't want to force master for Prod.

olleolleolle commented 2 years ago

@sebastianguarin Answer: not yet released. See https://github.com/jruby/warbler/issues/498 for what is left.

nbekirov commented 2 years ago

@olleolleolle Any chance of this getting a minor release instead of waiting for the 2.1.0?

joerixaop commented 2 years ago

This fix is still broken on 9.3.6.0. The #modifiable method is no longer part of ostruct in that release. The result of that is the following confusing behaviour:

irb(main):036:0> c = Warbler::Traits::War::WebxmlOpenStruct.new
=> #<Warbler::Traits::War::WebxmlOpenStruct>
irb(main):037:0> c.rails.env = 'production'
=> "production"
irb(main):038:0> c.rails.env
=> #<Warbler::Traits::War::WebxmlOpenStruct>
irb(main):039:0> c.rails.env = 'production'
=> "production"
irb(main):040:0> c.rails.env
=> "production"
irb(main):041:0> c
=> #<Warbler::Traits::War::WebxmlOpenStruct rails=#<Warbler::Traits::War::WebxmlOpenStruct modifiable=#<Warbler::Traits::War::WebxmlOpenStruct name=#<Warbler::Traits::War::WebxmlOpenStruct>, No value for 'name' found="production">, env="production">>

The fix for this is "simple": instead of the line modifiable[new_ostruct_member!(mname)] = args[0] it should be set_ostruct_member_value!(mname, args[0]), the catch is that that won't work on JRuby 9.2.

JesseChavez commented 2 years ago

Looking deeper in the open struct code the current method_missing, the implementation of in warbler is copy and paste from and older version of open struct.

https://github.com/ruby/ostruct/blob/1ea1438e592b7407654d0ead49f3a20f813a44cc/lib/ostruct.rb#L175