Support bundler 2 #494
Support bundler 2 #494
Conversation
deivid-rodriguez
force-pushed
the
remove_unnecessary_code
branch
from
a7355db to
41b3847
Compare
|
@deivid-rodriguez Really appreciate this input! |
|
@deivid-rodriguez I note that this project uninstalls Bundler and installs a Bundler < 2. I'll poke a little into that, see if a newer set of Bundler can be used, perhaps update the matrix to be about current Bundler & RubyGems. |
|
I did some work last week on getting warbler's CI green, I should be able to propose a separate PR for that soon 👍. |
deivid-rodriguez
force-pushed
the
remove_unnecessary_code
branch
from
41b3847 to
a38b0c6
Compare
|
I will try to support bundler 2 in this PR. |
|
Everything seems to be fine, and with the most recent bundler & rubygems a couple of hacks can be removed, apparently. I still want to I will see whether it's still worth keeping those hacks around in order to support old versions. Something problematic is that default gems are not correctly packaged inside the war file. That's why the CI was pinned to rubygems 2.7.11, because it's the most recent version that does not install bundler as a default gem. But this is an existing issue, unrelated to the bundler version in use. In this PR, I'm using a different workaround, namely, manually installing bundler as a regular gem so that it's correctly packaged. |
|
@deivid-rodriguez This is wonderful work, and I did merge the "Remove Rake 12 from matrix" change, too, would that take this PR closer to green? |
deivid-rodriguez
force-pushed
the
remove_unnecessary_code
branch
from
9696e05 to
b78a0f9
Compare
|
Yeah, PR should be green now, but I'm leaving it as a draft for now since I still want to verify a few things. |
|
@deivid-rodriguez Huh, it LOOKS like that actually fixed it. |
deivid-rodriguez
force-pushed
the
remove_unnecessary_code
branch
4 times, most recently
from
d712228 to
5e1518f
Compare
Unless using ruby 2.3 or older, or even of ruby 2.3 if a modern enough bundler is used, the bundler gemspec should never have its `loaded_from` attribute set to a missing file, so this whole hack shouldn't be necessary.
Currently warbler is unusable from jruby 9.2 during to this require failing (not sure to which extent though, but at least one spec is failing and there's an open issue about this). If we stop monkeypatching bundler, we don't need to require it, and thus the error disappears. Since doing that doesn't make any tests fail, I will assume the problems caused by not monkeypatching bundler are less important than the problems caused by doing it. So, I'm removing the code to fix the issue and get specs green.
Since warbler in action doesn't need bundler.
Fixes several failures like
```
10) Warbler::Jar with JBundler in a war project uses ENV['JBUNDLE_JARFILE'] if set
Failure/Error: let(:config) { drbclient.config(@extra_config) }
LoadError:
no such file to load -- jbundler/config
# (druby://127.0.0.1:7890) ./lib/warbler/traits/jbundler.rb:33:in `add_jbundler_jars'
# (druby://127.0.0.1:7890) ./lib/warbler/traits/jbundler.rb:29:in `after_configure'
# (druby://127.0.0.1:7890) ./lib/warbler/traits.rb:33:in `block in after_configure'
# (druby://127.0.0.1:7890) ./lib/warbler/traits.rb:33:in `after_configure'
# (druby://127.0.0.1:7890) ./lib/warbler/config.rb:217:in `initialize'
# (druby://127.0.0.1:7890) ./spec/drb_helper.rb:29:in `config'
# ./spec/warbler/jbundler_spec.rb:23:in `block in config'
# ./spec/warbler/jbundler_spec.rb:63:in `block in <main>'
```
`Bundler::Definition#specs_for` no longer behaves as before. Admittedly the previous behavior was more intuitive.
deivid-rodriguez
force-pushed
the
remove_unnecessary_code
branch
from
5e1518f to
e3b4c8d
Compare
|
Seems to have stalled since 2021? |
|
I actually rebased this a couple months ago and fixed one more issue, but it was still not ready. |
|
How are we now? Close to releaseable? |
|
I'm not actively working on this, nor planning to. Sorry. |
|
@deivid-rodriguez sorry to bother, I know it's been 2 years, but do you have a (draft) list of what remains to do here for this to be merged in? I'm using the code from this branch for some time and it kind of works for me. And if it's not something massive probably I can have a go at it. @headius without this, what is the recommended way to deploy new JRuby apps today? Especially if Rails 7 requires Ruby 3 which (I assume) requires Bundler 2. I thought about replacing the whole thing with a simple script but wanted to hear your thoughts on it first 🙏 |
|
@dolzenko I stopped working on this patch a while ago. I think I still had to get a few tests to pass. But if you're using this succesfully, I guess this is probably still an improvement even if some tests fail. Happy to give this a rebase if there's interest in merging it. |
|
@deivid-rodriguez I integrated the code in the midst of difficult release process in a hurry and I have this in the build script (kind of a gross hack where I just almost bruteforced through the different bundle invocation scenarios to find the one working) but now I definitely have the needed time to at least test it properly, or maybe even look into fixing the tests if you say its only that. |
|
Actually, a few comments above I claimed that CI was green, so maybe it was not that. Honestly, I forgot what the remaining issues were, sorry 😞. |
Unless using ruby 2.3 or older, or even on ruby 2.3 if a modern enough bundler is used, the bundler gemspec should never have its
loaded_fromattribute set to a missing file, so this whole hack shouldn't be necessary.Just checking CI for now.