Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print warning when running Bundler on potentially problematic RubyGems & Ruby combinations #5177

Merged
merged 3 commits into from
Dec 20, 2021

Conversation

deivid-rodriguez
Copy link
Member

@deivid-rodriguez deivid-rodriguez commented Dec 19, 2021

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

Users should move on and stop using old rubies, but we shouldn't abruptly drop support in bundler. Although doing this is generally fine to do for gems on minor versions, bundler is a bit special, because many scripts include gem install bundler commands, and old rubygems versions (which old rubies use by default) didn't have the ability to figure out that the latest bundler does not support the current ruby, so those gem install bundler commands will start failing as soon as a version of bundler no longer supporting old rubies is released.

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

In order to make sure people get ready for this, start by printing a warning when bundler is run on old rubies, and explain which steps users should be taking.

Make sure the following tasks are checked

bundler/exe/bundle Outdated Show resolved Hide resolved
@deivid-rodriguez deivid-rodriguez force-pushed the old_rubies_warnings branch 2 times, most recently from 8071b11 to 5b865a4 Compare December 19, 2021 21:40
@deivid-rodriguez
Copy link
Member Author

Marking as ready, pending the specific copy of the warning to be decided.

@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review December 19, 2021 23:05
bundler/exe/bundle Outdated Show resolved Hide resolved
bundler/exe/bundle Outdated Show resolved Hide resolved
Copy link
Member

@indirect indirect left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for setting this up! I think this will reduce future support by a lot. 👍🏻

bundler/exe/bundle Outdated Show resolved Hide resolved
bundler/exe/bundle Outdated Show resolved Hide resolved
deivid-rodriguez and others added 3 commits December 20, 2021 20:58
The `BUNDLE_` prefix should be reserved to first class settings that
should be listed when running `bundle config`. This one is just a hacky
environment variable that has not corresponding documented setting.
@deivid-rodriguez
Copy link
Member Author

@indirect Good to go now? Feel free to enable auto merge if you are satisfied with this.

@deivid-rodriguez deivid-rodriguez changed the title Print warnings when running bundler on old rubies Print warning when running Bundler on potentially problematic RubyGems & Ruby combinations Dec 20, 2021
@indirect indirect enabled auto-merge December 20, 2021 20:23
@indirect indirect merged commit 0743b5a into master Dec 20, 2021
@indirect indirect deleted the old_rubies_warnings branch December 20, 2021 21:48
@deivid-rodriguez
Copy link
Member Author

I'm reading the gem update --system code now, and I'm pretty sure it still suffers from the same issue already fixed for unconstrained gem install :(

Luckily we recommended a constrained gem update --system 3.2.3 in the warning text, and unconstrained gem update --system is much less common that gem install bundler in scripts/CI anyways.

Providing a nice upgrade path for this other issue feels much harder.

One idea that comes to mind is to update the warning text to also mention that gem update --system commands need to be updated to be constrained, and to add some docs/blog post about this to get people ready for next year.

Another idea would be to keep support for old rubies in rubygems for now, while dropping it in bundler.

And of course, the issue needs to be fixed, but the problem is to propagate it to old rubies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants