Avoid vendored thor gem polluting the global namespace

[takmar]

What was the end-user or developer problem that led to this PR?

When I ran specs in the Thor gem using the Bundler 2.4.22, I got the following warnings.

$ bundler -v
Bundler version 2.4.22
$ bundle exec thor spec
/thor/lib/thor/shell/lcs_diff.rb:11: warning: method redefined; discarding old show_diff
/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/bundler-2.4.22/lib/bundler/vendor/thor/lib/thor/shell/lcs_diff.rb:8: warning: previous definition of show_diff was here
/thor/lib/thor/shell/lcs_diff.rb:26: warning: method redefined; discarding old output_diff_line
/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/bundler-2.4.22/lib/bundler/vendor/thor/lib/thor/shell/lcs_diff.rb:23: warning: previous definition of output_diff_line was here
/thor/lib/thor/shell/lcs_diff.rb:42: warning: method redefined; discarding old diff_lcs_loaded?
/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/bundler-2.4.22/lib/bundler/vendor/thor/lib/thor/shell/lcs_diff.rb:39: warning: previous definition of diff_lcs_loaded? was here

This is because when the Thor gem in this repo was updated to v1.3, the LCSDiff module was added as a top-level module in the PR as shown below. As both the LCSDiff in the Bundler and the Thor define the same name method, those warnings are shown.

rubygems/bundler/lib/bundler/vendor/thor/lib/thor/shell/lcs_diff.rb

Line 1 in 558f516

module LCSDiff

The Thor gem has a spec that checks there is no warning in $VERBOSE mode, so the test is broken when I use a Bundler that has the Thor v1.3.

What is your fix for the problem, implemented in this PR?

I added the Bundler namespace to the LCSDiff module as other vendor gems do.

Make sure the following tasks are checked

• Describe the problem / feature
• Write tests for features and bug fixes
• Write code to solve the problem
• Make sure you follow the current code style and write meaningful commit messages without tags

[welcome]

Thanks for opening a pull request and helping make RubyGems and Bundler better! Someone from the RubyGems team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use GitHub Actions to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of GitHub Actions in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #rubygems or #bundler channel on Slack.

For more information about contributing to the RubyGems project feel free to review our CONTRIBUTING guide

[deivid-rodriguez]

Good catch! We may need to tweak our vendoring scripts thought, since #7285 should catch this now as "invalid vendoring".

[segiddins]

I dont think this actually works, since Thor is re-opening the existing LCSDiff module, it's not complete on its own

[deivid-rodriguez]

Good point, alternatively, we may want to skip this module from our vendored copy altogether, since Bundler::LCSDiff will never be available and we don't want our vendored copy to cause global side effects?

[takmar]

Thank you for your feedback!

I removed LCSDiff module entirely from vendored Thor with the task introduced #7285. It's an awesome feature.

[deivid-rodriguez]

Thanks, that feature is indeed cool. As per CI, you also need to remove the explicit requires of the file being removed.

[takmar]

I’m sorry I removed require_relative "lcs_diff" and also removed the file from Manifest.txt.

[deivid-rodriguez]

Looks great, thanks so much!

[p8]

Hi 👋
This fix is available in Bundler 2.5 which requires Ruby 3.0 or higher.
Could this be backported to Bundler 2.4.x ?
It would help make the Thor CI green for Ruby 2.6 and 2.7.

[deivid-rodriguez]

We generally don't do backports to versions older than current minor series (2.5.x in this case). @hsbt sometimes manages backports to old branches if the bugs fixed are important enough, but in this case since it affects Thor tests only, I would advice to let this be workaround in Thor's side.

Actually, if Thor's ruby version support was aligned with Bundler that would be ideal for us, since that would simplify our lives when vendoring Thor.

[p8]

@deivid-rodriguez Thanks. I'll see what I can do in Thor.