Try to fix Gem.paths #564
Try to fix Gem.paths #564
Conversation
|
Ping @jlahtinen |
| end | ||
| end | ||
|
|
||
| Gem.paths = ENV |
There was a problem hiding this comment.
while this might work, not sure this is the proper place (or actually the proper fix).
Gem.paths should have been properly adjusted, otherwise we're kind of in a mess anyway... 🤷
There was a problem hiding this comment.
You're absolutely right.
We’ve looked into this quite extensively, and honestly, we couldn’t figure out why this issue occurs in some projects but not in others — even when the dependencies and environment are nearly identical.
It would definitely be better to understand and fix the root cause instead of working around it. Unfortunately, we haven’t been able to pinpoint it. Given that monkey patches are already present in the relevant issue comments — and much of this file is already composed of other monkey patches (example here) — we’ve put together this PR to help unblock affected users.
That said, a proper fix to ensure Gem.paths is set up correctly would definitely be preferable. We'd be happy to adjust or close this PR if someone can point us in the right direction.
There was a problem hiding this comment.
Seems there is an alternate approach at #572 FWIW
There was a problem hiding this comment.
not sure about that one either given the if defined?(Gem) && Gem.respond_to?(:paths=) part, which would have the intent of not always applying... my first question would be why 🤷
jruby-rack has ways user can control the ENV and wout looking into Warbler in details I can not tell which one is aligned with that.
also Bunlder supports a bunch of ENV variables and these days RubyGems and Bundler are tightly integrated, so that should be taken into account as well, in terms of isolation from the actual environment.
both PRs achieve something but they feel like quick hacks, would rather not be involved with these with my already limited time for OSS.
There was a problem hiding this comment.
Fair enough.
It probably is quite sensitive to how the given servlet container is propagating env from surrounding environment (standalone tomcat et al, vs embedded etc)
Which then makes it a design decision about how warbler should (or should not) respect them and what is most idiomatic. I had to address a related issue with jruby-rack when reinstating the tests for modern bundler/rubygems (with and without mise/rvm) and adjusting paths to load the vendored rack so I might have a clue or another....
There was a problem hiding this comment.
After a lot of digging, I've made an alternate approach at #575 which
- includes cleaning up the old hacks which confuse things here
- makes the change closer to the appropriate place (it's not really specific to bundler usage)
- includes analysis of the root cause of change in behaviour in JRuby 9.4
The core problem and attempt to fix it here is valid IMHO - just not in quite the right place or in an ideal state.
As to why it occurs in some places, but not others It is probably sensitive to
- whether the GEM_HOME and GEM_PATH are already set or propagated to the ENV before webserver loads
- whether you have
config.webxml.jruby.rack.env.gem_pathset tofalseor some path value for jruby-rack to work with before warbler boots - jruby version,
9.3will usually be fine. - jruby-rack version (I have only really evaluated with 1.2.x)
|
I think But I agree with @kares that this "fix" can be a wrong way to fix the "issue". |
|
@jlahtinen @tillsc The approach in #575 seems like the right way... can you have a look at that and see if it addresses issues for you? |
|
@headius I’m AFK for a bit, sorry. This fix has always been more of a hack than a proper solution, so I don’t see a reason to keep this open now that there’s a real approach to address the problem. I’ll give it a try when I’m back. Feel free to close this PR in the meantime. |
|
Yeah, your fix is equivalent to mine anyway - both end up calling Only diff is that mine collates the change in the two places that are (intentionally) mutating the paths (not just bundler, theoretically it can break the rare non bundler app) and gives the background explanation of why it broke on 9.4. It is semantically identical so will work the same. |
|
Replaced by #575 (now merged) - thanks again for the work here - it really helped narrow things down (and I've been using your pre-release gem for a couple of months to test jruby-rack before digging in here) |
See #539 and #537.
This fix is based on comments in the respective tickets. Since the entire file appears to be a hack or monkey patch, I’m not sure whether this is the “correct” solution. However, it works for us with JRuby 9.4, the current master of Warbler, and Bundler 2.6.3, running on Tomcat 9 under macOS and Windows.