rubygems / rfcs

RubyGems + Bundler RFCs
45 stars 40 forks source link

RFC submission for override_gem_dependency_versions #13

Closed br3nt closed 2 years ago

br3nt commented 6 years ago

This is the RFC that has been requested by @indirect in bundler#4552.

Summary

Enable the version constraints of gem dependencies to be explicitly overridden for gems listed in a Gemfile. The purpose of this RFC is to get feedback on use cases, format/syntax/DSL, and feature options.

Motivation

This feature has been requested many times since 2011.

Project authors want the ability to specify newer versions of gems in their Gemfile even if the version is being constrained by another gem.

There several causes for this issue:

The common approach to this problem is to fork the problematic gem, relax the dependencies specified in the gemspec, and reference the fork in the Gemfile. This is a very heavyweight and time intensive solution to a problem that could be easily solved by a well-designed solution to override dependencies from within the Gemfile itself.

Obviously, overriding version constraints of gems would be AT YOUR OWN RISK, however, a properly defined DSL and output messages should mitigate and address these issues.

Most of the time, this kind of override should NOT be a permanent solution, and as such, SHOULD be accompanied with some type of message that is additionally displayed to indicate why the override has occurred.

Guide-level explanation

Say you want to use the latest version of the json gem (currently 2.1.0), however, another gem you are using constrains the version to s.add_dependency 'json', '>= 1.7.7', '<= 2'. To override this version constraint so that version 2.1.0 is installed:

gem 'json'
gem 'other_gem' do |s|
  s.override_runtime_dependency('json', '>= 1.7.7', '<3')
end

This will allow bundler to install the 2.1.0 version of the json gem.

The output when running bundle install will look like the following:

Installing json 2.1.0 (forced version, conflicts with other_gem)
Installing other_gem 1.0.0

Most of the time, this should only be a temporary fix as we expect the gem author to release a new version with updated dependency constraints.

To provide additional information a note can be included:

gem 'json'
gem 'other_gem' do |s|
  s.override_runtime_dependency('json', '>= 1.7.7', '<3', 'waiting on pull request #5')
end

The output when running bundle install will look like the following:

Installing json 2.1.0 (forced version, conflicts with other_gem: waiting on pull request #5)
Installing other_gem 1.0.0
segiddins commented 6 years ago

Have you seen https://github.com/bundler/bundler/pull/5670, by any chance?

br3nt commented 6 years ago

@segiddins I hadn't seen your PR, but I had seen people suggest the :force_version syntax in some issues. Because of that, I do make reference to that syntax in the RFC. To be honest, I do knock it a bit as my thinking on the matter is that it glosses over the finer details of what exactly is being overridden, whereas the syntax suggested in the RFC clearly documents the offending gem and what we are overriding and gives a mechanism to provide additional comments in outputted messages. But that's the whole point of RFCs right? To get comments from all sides and implement a solution as decided by the community?

segiddins commented 6 years ago

Definitely! I just wanted to make sure you had seen a (mostly) working implementation of an alternative, especially since it might be a good jumping-off point for playing around with the different DSL options

br3nt commented 6 years ago

@segiddins it's great that there is a working implementation. What exactly isn't working the way you would expect?

Also, did I read correctly that an exact version must be supplied? Whats the rationale behind this over using version constraints and negotiating with other gems?

segiddins commented 6 years ago

Also, did I read correctly that an exact version must be supplied? Whats the rationale behind this over using version constraints and negotiating with other gems?

I think my thinking at the time was that it would help keep resolution more predictable -- by restricting it to only exact versions, you would never end up with a conflict on that particular gem. It also requires the override to be very explicit (I need this version), versus just changing the requirements around (which could end up matching different versions over time, with no change in the Gemfile). Basically, I wanted this to be a very precise hammer to break out of the normal behavior of Bundler, and I wanted it to be targeted, so there would be minimal room for surprising (or edge-case-y behavior)

br3nt commented 6 years ago

@segiddins yeah sure that makes sense.

colby-swandale commented 6 years ago

I understand the use case for this but at the same time this scares me completely 😱

br3nt commented 4 years ago

@colby-swandale it shouldn't :) It should be a perfectly reasonable thing to do.

Just imagine that this was implemented and the user could override a gem dependency version. When performing a gem install or gem update there are two possibilities:

  1. A gem dependency version constraint is getting overridden and the chosen version is outside the bounds of the gem authors contstraint. Some text in warning colours could be displayed underneath the gem.
Installing json 2.1.0 (forced version, conflicts with other_gem)
Installing other_gem 1.0.0
  Overriding json version constraint from `'>= 1.7.7', '<= 2'` to `'>= 1.7.7', '<=3'`

Or 2. A gem dependency version constraint is getting overridden and the chosen version is within the bounds of the gem authors constraint. A friendly text can be displayed telling the user to remove the override.

Installing json 2.1.0
Installing other_gem 1.0.0
  Remove `override_runtime_dependency` for json on line 3 of Gemfile

This is much safer and easier than the current hack of forking repos just to fix stupid version constraints imposed by gem authors. When the user forks a repo, they need to manually check the original gem for updates. They could potentially miss out on security updates or what not if they aren't diligent. However, being able to add an override, the user gets nice messages indicating whether the override can be removed or not. A simple gem update will check for new versions and if the gem author finally fixes their gem dependency version constraints, the user will automatically get the fix, and a nice error will be shown telling them to remove the override!

It couldn't be simpler or more intuitive if you tried ;)

lukaso commented 3 years ago

@br3nt I've added a bunch of comments and a working implementation. I can't PR your PR but feel free to incorporate my feedback into the content of the RFC, especially the fact this is a common capability in package managers. Not exactly sure how this moves forward, but hopefully a fully functioning implementation helps!

lukaso commented 3 years ago

Having spoken with the maintainers of Bundler they have proposed an alternative workflow for dealing with out of date gems. Here is documentation of that workflow:

How to override gem dependencies

Note This attempts to document that recommended approach for overriding gem dependencies which are not being updated by gem maintainers. This documentation is based on what has been recommended by the core Bundler maintainers as preferable to adopting this RFC (or the attached implementation https://github.com/rubygems/rubygems/pull/4178).

When a gem has dependencies that are not being updated (or really any changes that should be made) in a timely manner, the community should attempt to move forward by proposing the changes.

The best way to do this is to

Note This documentation only seeks to provide a solution for (Github)[https://www.github.com] (or at least git) based gems. Any other gems would require a similar approach, but there is not attempt to document here.

Steps

  1. For the relevant gem, find where its source code is maintained. Typically on https://www.github.com.

    1_find_gem_github_1 1_find_gem_github_2
  2. Search Pull Requests (PRs) to find one that meets your needs.

    2_find_github_PR_1 2_find_github_PR_2 2_find_github_PR_3 2_find_github_PR_4 2_find_github_PR_5
  3. If you can't find a PR that meets your needs, skip to "If one is not available"

  4. Extract the information you will need for your Gemfile. This is the name of the account for the PR, the name of the repo and the name of the branch.

    2_find_github_PR_6
  5. Skip to "Generate the URL for your Gemfile"

  6. If one is not available, fork the repo and create a PR that meets your needs (by updating the relevant dependencies or making the relevant changes).

  7. Generate the gem line for your Gemfile. For example, with https://github.com/lukaso/FuelSDK-Ruby.git, the gem line will be gem 'sfmc-fuelsdk-ruby' git: 'https://github.com/lukaso/FuelSDK-Ruby', branch: 'patch-1'. Here is the Bundler documentation.

    3_grab_github_url
  8. Place this line in your Gemfile

This will provide you with a patched version of the gem which can then be replaced when the PR is eventually incorporated into the original gem.

unicornrainbow commented 3 years ago

Suggestion for DSL:

gem "aardvark" do
  drop_dependency "my_other_car_is_a_hamburger"
end

gem "sparkle" do
  change_dependency "jiffy", ">= 8.5.2"
end

This syntax could co-exist with yielding a decorated spec for more elaborate tinkering.

simi commented 2 years ago

This RFC was AFAIK widely rejected by various RubyGems maintainers at various places (Slack, Issues, ...). Would it be ok to close this RFC and mark as rejected?

lukaso commented 2 years ago

This RFC was AFAIK widely rejected by various RubyGems maintainers at various places (Slack, Issues, ...). Would it be ok to close this RFC and mark as rejected?

I'm OK with it being closed, but I think it would be good to incorporate the documentation I wrote here into the official documentation.

simi commented 2 years ago

I'm OK with it being closed, but I think it would be good to incorporate the documentation I wrote here into the official documentation.

I don't think this should be part of official documentation. We should suggest to reach the project and ask for update or takeover over the maintenance (you can use also new adoption center at rubygems.org). Forking the project and using custom version is really last option here and it should be done only temporarily (not as a final solution) meanwhile the project is revived and maintained again or until you remove this unmaintained dependency from your project.

lukaso commented 2 years ago

I don't think this should be part of official documentation. We should suggest to reach the project and ask for update or takeover over the maintenance (you can use also new adoption center at rubygems.org). Forking the project and using custom version is really last option here and it should be done only temporarily (not as a final solution) meanwhile the project is revived and maintained again or until you remove this unmaintained dependency from your project.

The problem always was the second level dependency. Not the first level dependency. I'm not familiar with the adoption center so maybe that really is the answer. Because as it stood, we were forking loads of gems to just update dependencies.

Anyways, all this was discussed at length at the time.

br3nt commented 2 years ago

Seriously, why would anyone want to reject and close this issue??

This is a real problem that will only ever increase as more gems become abandoned.

Of course current gem authors don’t agree with this… they can’t imagine a time when their gem will hit the graveyard.

But sure enough it’s highly likely that at some point their gem will become abandoned and the poor saps who have a dependency on those gems will want a way to override the crazy overly restrictive constraints that gem authors often impose.

Unless someone has come up with a better way of solving this real problem, I propose the community work together and push this solution to completion.

At the very least, the community needs this feature, or a feature like it, to get past all the security bugs from old abandoned gems that are still useful.

Prove me wrong or link to a better option.

br3nt commented 2 years ago

Just as an add on to this… forks, merge requests, and any other such hacks are not viable solutions to get around dependency constraints from poorly maintained or abandoned gems.

Not only does this add extra development burden to every project using the abandoned or poorly maintained gem.

It also fragments support! If everyone followed this advice, there could be 1000s of forks for a single gem all making the same change to its constraints.

Just crazy!

It’s wishful thinking that an abandoned or poorly maintained gem will get a new maintainer.

The community needs a simple easy way to override dependencies for the brief period required to replace the problematic gem.

br3nt commented 2 years ago

I seriously don’t understand what all the fuss is about. There hasn’t been a good argument against this proposal.

br3nt commented 2 years ago

I just read about the Bundler teams recommendations documented in a comment by @lukaso on 1 Feb 2021.

This is seriously bad advice and bad development practices for the reasons I stated above. Just because the Bundler team support and endorse this practice doesn’t make it good or right. Just because it works doesn’t make it a good solution. How many times do you think an abandoned and poorly maintained gem will be forked just so it’s dependencies can be fixed? Max of once for every app using the gem. Now what happens when people use one of these forks so they don’t have to fork it them selves? What happens when that fork becomes abandoned or poorly maintained? Let’s call this scenario forking hell.

I’ll say it again… it’s terrible advice and bad practice.

We have a far superior and versatile solution in this proposal. The community just needs to push it to completion.

deivid-rodriguez commented 2 years ago

Our take is that encouraging fixing this upstream (for everyone) and not just for yourself actually saves work for everyone. Yes, the first person that proposes a solution upstream has to do some extra work, but once there's a working PR, the changes needed for every other user are about the same either with a new feature, or with existing features.

Basically,

gem 'other_gem' do |s|
   s.override_runtime_dependency('json', '>= 1.7.7', '<3')
 end

vs

gem 'other_gem', github: "https://github.com/<other_gem_org>/<other_gem_name>/pull/<pull_number>"

I worked with ruby libraries for many years and in real life there's no such issue as "fork hell", there's hardly ever competing PRs proposing the same dependency changes, and when that happens they get unified quickly.

Let's close this and focus on other things, thanks for the discussion!

br3nt commented 2 years ago

In regards to this vs force_version, the latter is like a sledgehammer, whereas, this is like a paint brush.

This recommendation works with the constraints system following the same rules and behaviours.

That is, the constraint of the dependency is overridden with a new constraint.

I think this would be more useful for gem authors who want to slowly ween off an abandoned gem dependency. Imagine if a gem author used force_version and now your dependencies were locked to the exact version they specified. Yikes!

By using this approach, a looser constraint can be applied as an override. This would allow the underlying dependency to continue to develop and the end app will get the newer versions. Perfect!

arcreative commented 2 years ago

The notion that this is closed as unnecessary when it's supported in virtually every other package manager, is quite frankly, bonkers. Many of us use this exact feature in other package managers, and it works perfectly without the feared explosions and mayhem alluded to here. It's absolutely brilliant to be able to add a resolutions element to my package.json file and solve 9 security issues in 5 minutes and go on with my life without hurting anyone in the process.

At this point, I am much closer to using a forked version of bundler with @lukaso's PR than I am a fork of every problematic dependency/sub-dependency in my life.

You guys, the work on https://github.com/rubygems/rubygems/pull/4178 appears nearly finished, either as-is or with a bit more discussion. I can understand maintainers' apprehension about a breaking change, but again, this is an opt-in change that no one is being forced to use, but so many would benefit from.

Please consider reopening this, I've spent dozens of hours trying to find workarounds for this, only to find my own comments gathering dust, posted at 3-year intervals. I'm even happy to be a part of the process myself if it would help move it along.

indirect commented 2 years ago
  1. Posting an angry, combative comment does not make this more likely to happen.
  2. Threatening to use a fork of Bundler does not make this more likely to happen. This is open source! You are supposed to use a forked version of Bundler if that makes your life better.
  3. Repeating previously-mentioned talking points does not make this more likely to happen.
  4. Insisting we should feel obligated to do work that you want, for free, does not make this more likely to happen.
  5. Offering to consider contributing to Bundler if, and only if, we agree up front to do what you want... does not make this more likely to happen.

Your underlying points are fairly reasonable, but the way you have chosen to present them is so off-putting that I, for one, have lost all interest in trying to engage.

If there are any future possible contributors reading this, the existing team is not actively against anything like this feature. As discussed in this ticket and elsewhere, it is the effort needed to develop this feature and support it when it causes issues that means it is not a high priority at this time.