Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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... 🤷see lib/warbler/templates/jar.erb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems there is an alternate approach at #572 FWIW
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a lot of digging, I've made an alternate approach at #575 which
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
config.webxml.jruby.rack.env.gem_path
set tofalse
or some path value for jruby-rack to work with before warbler boots9.3
will usually be fine.