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

Remove the BundlerVersionFinder / "Bundler version switcher" #3879

Closed
eregon opened this issue Aug 6, 2020 · 17 comments
Closed

Remove the BundlerVersionFinder / "Bundler version switcher" #3879

eregon opened this issue Aug 6, 2020 · 17 comments

Comments

@eregon
Copy link
Contributor

eregon commented Aug 6, 2020

I think we've discussed this several times and agreed on it, but I couldn't find an issue on the tracker.

Bundler 1 seems no longer maintained, the last release of Bundler 1 was in 2018.
Therefore it seems important, notably for security reasons and to increase Bundler 2 adoption, that Bundler 2 is used for all cases if installed, even if the Gemfile.lock still has BUNDLED WITH 1.x.y.

There are many reasons why a Gemfile.lock might still have BUNDLED WITH 1.x.y, either it wasn't updated in some time, or maintainers didn't want to force people using Bundler 2, or the I think the main reason: due to BundlerVersionFinder, if the Gemfile.lock was BUNDLED WITH 1.x.y it would always remain so, because BundlerVersionFinder would force using Bundler 1 even if you did not want to.

Also I think the general Bundler version autoswitcher mechanism has confused more than helped.
bundler would again act like a regular gem in that if you run the executable, you use the latest version again.
That's intuitive, and it's good for security and to get the latest release's bug fixes.

Also the current situation makes it hard or potentially annoying to deprecate APIs that Bundler 1 might still use:
https://github.com/rubygems/rubygems/pull/3817/files#r453293031

I don't know what's the current status regarding unification of RubyGems and Bundler, but it seems for now Bundler is still a separate gem? Any place to read the vision and progress on that?

Regarding multiple lockfile parsers, etc, my understand is Bundler 2's Gemfile.lock syntax is a strict superset of Bundler 1's Gemfile.lock syntax. They both use lockfile v1 according to https://github.com/rubygems/rfcs/blob/master/text/0000-lockfile-versions.md.
So I think there is no need for versioning the parser yet, and it doesn't seem a prerequisite to remove the BundlerVersionFinder.

Relates to https://github.com/rubygems/rfcs/blob/master/text/0000-lockfile-versions.md
Related to #3275.

cc @indirect @deivid-rodriguez @colby-swandale @schneems @duckinator

@colby-swandale
Copy link
Member

The current issue/PR is at #3073

@hsbt
Copy link
Member

hsbt commented Aug 6, 2020

I wonder why you didn't use bundle update --bundler. I always update the bundler version with it under the BUNDLED WITH.

@eregon
Copy link
Contributor Author

eregon commented Aug 6, 2020

I wonder why you didn't use bundle update --bundler. I always update the bundler version with it under the BUNDLED WITH.

There are arguments for that in the RFC and blog post.
Some reasons I remember from those:

  • If I contribute to a project and the Gemfile.lock is BUNDLED WITH 1, I definitely don't want to include that change in a PR (not my decision as a contributor). But I also don't want to install an old unmaintained Bundler version. And keeping a local diff is annoying.
  • And for the maintainer of a gem, if they switch to BUNDLED WITH 2 they will immediately force every user of the project to use Bundler 2, which might be unwanted (e.g., if they still want to support Ruby 2.2 or Rails 4). And it's all sad because Bundler 1 would still work fine without BundlerVersionFinder.

@eregon
Copy link
Contributor Author

eregon commented Aug 6, 2020

@colby-swandale Thanks, I searched issues and not PRs so I missed it.

Let's wait for @deivid-rodriguez to back and see what he thinks.
Probably we just need to rebase that PR and merge it?
Let me know if I can do anything to get this PR merged.

@hsbt
Copy link
Member

hsbt commented Aug 6, 2020

  • But I also don't want to install an old unmaintained Bundler version

Interesting. I also faced the old verion of rails, nokogiri, bundler and other stuffs everydays. But I did install them if I hope to contribute the some project. Why You avoid to install only bundler?

@eregon
Copy link
Contributor Author

eregon commented Aug 6, 2020

In practice I still install Bundler 1 since I have no choice while the BundlerVersionFinder is still there.

My concern specifically here is Bundler 1 seems unmaintained (including security-wise).
For other gems older versions might still be maintained, which is different. And other gems basically never prevent you to use a newer version, especially if it's compatible. BundlerVersionFinder prevents to use a newer version (or makes it inconvenient and requires changing the Gemfile.lock with an extra command).

@eregon
Copy link
Contributor Author

eregon commented Aug 6, 2020

A concrete example: https://github.com/ruby/setup-ruby (and I would think other CIs) have pretty complicated logic, all due to the BundlerVersionFinder. For instance, it needs to read BUNDLED WITH to find a version of Bundler that BundlerVersionFinder will accept. And it would probably make a lot more sense to just always use Bundler 2 on Ruby >= 2.3, but BundlerVersionFinder prevents this.
And clearly there are some bugs in Bundler 1 that will never be fixed, so Bundler 2+ is inevitable longer term.

There is no major incompatibility in Bundler 2, so it should IMHO require no special action from the user, and specifically not need to change the Gemfile.lock with an extra command.

Another typical annoyance I often run into: while testing gems I run the tests on CRuby and TruffleRuby. CRuby might have Bundler 2 installed and so will create a Gemfile.lock BUNDLED WITH 2. TruffleRuby comes with Bundler 1 to match what CRuby 2.6 does (and due to BundlerVersionFinder shipping only Bundler 2 would be incompatible). So now I cannot bundle install on TruffleRuby, even though there is no fundamental reason for it (and removing the BUNDLED WITH section actually works around this, but feels like a dirty hack).

@eregon
Copy link
Contributor Author

eregon commented Aug 6, 2020

To be honest I don't know why I detail this so much here. Maybe to illustrate the urgency of removing it?
The Bundler team already agreed in the RFC to remove BundlerVersionFinder, and it solves so many issues.

@hsbt
Copy link
Member

hsbt commented Aug 6, 2020

Got it. But In my understanding, After Bundler 1.17 didn't have an security issues. Because we didn't release the new version of Bundler 1. The critical vulnerability was found in Bundler, We may release the new version of Bundler 1.

In my experience, I sometimes got the frustration of bundler version. But I did install the each bundler version and share them each Ruby versions, I leave from the issues of BundlerVersionFinder.

The Bundler team already agreed in the RFC to remove BundlerVersionFinder, and it solves so many issues.

Really? I didn't get it.

#3073 (comment)

I am planning to prepare a RFC for the new version switcher I plan to implement inside bundler.

We still wait above proposal.

@eregon
Copy link
Contributor Author

eregon commented Aug 6, 2020

We still wait above proposal.

https://github.com/rubygems/rfcs/blob/master/text/0000-lockfile-versions.md has all the details isn't it?

Specifically https://github.com/rubygems/rfcs/blob/master/text/0000-lockfile-versions.md#reference-level-explanation
Step 1 is done. Step 2 is this issue.

But I really think we should do this first and quick, because this makes bundler very user unfriendly for no good reason (no lockfile format changes between v1 and v2, nor major incompatibilites)

I agree, it causes pain and it gains basically nothing. There is only one lockfile version right now, so we don't need any kind of lockfile version handling yet. And anyway since it would be all about the lockfile version, there should never be a need to use a older Bundler version or automatically switch versions or any magic.
Always use latest Bundler, and latest Bundler supports previous lockfile versions. Can't be simpler than that.
If users want a specific Bundler version (e.g. to repro a bug) they can bundle _x.y.z_ ... but that should be very rarely needed.

@hsbt
Copy link
Member

hsbt commented Aug 6, 2020

https://github.com/rubygems/rfcs/blob/master/text/0000-lockfile-versions.md has all the details isn't it?

Thanks. I missed it. We need to how move to step.3 before doing step.2.

@deivid-rodriguez
Copy link
Member

I'm sorry I never commented on the RFC. In my opinion, the RFC and the idea of versioning lockfiles is nice, but it solves a "future problem" (there's no current lockfile format incompatibilities accross versions) and I'm not sure it justifies removing the BundlerVersionFinder.

Even though I proposed #3073 at the time, and said what was quoted, I don't think I want to remove the BundlerVersionFinder after all. I really like the idea of the BundlerVersionFinder, which is that all the users, CI, and production environment of an application run the exact same bundler code. The problem with the BundlerVersionFinder in my opinion, is that it's badly implemented, not the idea of it. For example, running bundle install should not fail if the right major version of bundler is not installed, but instead it should use the latest version of bundler available and install the one specified as BUNDLED WITH, so that future bundle exec invokations use the right version.

In general, I agree with most things @hsbt has said in this thread, and I would like to focus on making the BundlerVersionFinder user friendly rather than removing it. That would also provide a better compatibility path improving the experience for everyone, while not pissing off any users happily using this feature.

Like @hsbt said, sometimes we install lockfiles with old versions of gems, and that's fine. The right way to fix that is to encourage applications to upgrade, not to introduce tricks to version managers to opt-out of version locking. I don't think bundler should be treated differently. I understand that there can be different opinions about the particular case of the version manager itself, but we've had this feature for years now, so I don't want to start the adventure of removing it, honestly.

@eregon
Copy link
Contributor Author

eregon commented Aug 30, 2020

If we don't remove it can we agree we at least agree we need to fix it?

So that using a more recent major Bundler version is always allowed, and BundlerVersionFinder should never get in the way of that. If the Gemfile.lock contains BUNDLED WITH 1.x.y, I want to be able to use Bundler 2 (not have to install Bundler 1 additionally), and without having to change the Gemfile.lock (so I don't have to keep a dirty git status, make decisions like updating it that are none of my concern but the project's, etc). Many more arguments about that above and in the RFC.

Nobody wants to maintain Bundler 1, so people should eventually use Bundler 2+, without first forcing every project to manually update their Gemfile (notably some projects want to keep compat with EOL Rubies so they cannot update).

Having to install 2 Bundler versions (and more in the future) is a hassle.
The logic for "which version of Bundler should I use in CI" is incredibly complicated:

  • It is the reason most CIs broke when Bundler 2 was released, together with Bundler 2 refusing to work with BUNDLED WITH 1.
  • there are so many ugly hacks in .travis.yml for that and the TravisCI documentation is still so confusing trying to make sense of that mess
  • and basically every CI needs to recreate the logic of BundlerVersionFinder just to make BundlerVersionFinder happy (e.g., ruby/setup-ruby).

The answer should be, as long as you use a non-EOL Ruby, "the latest release of Bundler, which works for all previous Gemfile.lock". With a given Gemfile.lock, which gems to install is very clear, there is no ambiguity and so I think there is no point to install an older version.

If we have that, IMHO it doesn't make sense to ever use Bundler 1 (even if installed), if Bundler 2 is installed.
In other words, the BundlerVersionFinder should only check if your Bundler is new enough.
There is no need for BundlerVersionFinder for that, Bundler can just check itself if its major version is >= BUNDLED WITH.
In other words, BundlerVersionFinder is pointless and needlessly complicated and confusing.
Is there any real reason with a practical example to keep it?

Also from experience it's very clear that any Bundler-specific in RubyGems is total nightmare to maintain because it depends on the RubyGems version but really it needs to be customized by Bundler based on how Bundler deals with backward/forward compat so it can be fixed without updating RubyGems (remember when BundlerVersionFinder was too picky and wanted the exact same Bundler x.y.z?).
That all results in counter-intuitive behavior, and which bundler gets activated is "dark magic" vs "latest installed release" like every other single gem. This expectation of command xyz uses the latest version of gem xyz is what people expect, and I see no valid reason to break it and confuse everyone.

Ideally, if it's the same lockfile version, then using an older major version should be allowed too. This would be nice when e.g. a new Bundler version with the same lockfile version just comes out, yet not everyone might want to update at once. Basically it avoid needlessly breaking things when there is no need. But I think it's less important.

I really like the idea of the BundlerVersionFinder, which is that all the users, CI, and production environment of an application run the exact same bundler code.

I think most people don't care about that, and we shouldn't force them to. As said above, if there is a Gemfile.lock, no matter which Bundler version those exact gems and versions will be installed. If there is no Gemfile.lock, then the argument is moot (no Gemfile.lock, so no BUNDLED WITH).
If people need a specific version of Bundler for an app, then I think they should encode that explicitly (maybe as gem "bundler", "~> x.y" in the Gemfile or some other way?). That seems a small minority though.

What people want is a way to install gems and run with those gems. RubyGems just works, no matter the version, why should Bundler need N installed versions to be usable and pick a seemingly random different version for each project?

@eregon
Copy link
Contributor Author

eregon commented Aug 30, 2020

but we've had this feature for years now, so I don't want to start the adventure of removing it, honestly.

Which feature? BundlerVersionFinder? That's visible only since Bundler 2.
If what it takes is me making a PR to remove it so in Ruby 3 we have a much more intuitive behavior for Bundler, I'm happy to do that.

@schneems
Copy link
Contributor

This conversation is a bit muddy for me. I want to get context over where we're at.

I spoke with Andre and Coby at RubyConf last year and my understanding was that going forwards the existing failure/error behavior was going to be removed and that any incompatibility in the lock file would be solved by maintaining multiple lock file parsers internally in Bundler. I.e. bundler version 9000 that ships with lock file v9, all would have a lock file parser that could understand v1-v8.

This buys us (platform providers) the ability to move back to only providing one bundler version that is compatible with a range of projects.

Is what I described above still the active plan?

@duckinator
Copy link
Member

duckinator commented Sep 1, 2020

I talked with @deivid-rodriguez, @indirect, and @segiddins and I think I now have enough information to clear things up here:

  1. Further discussion has clarified that the lockfile versioning and BundlerVersionFinder things are distinct problems, as evidenced by the behavior of the same lockfiles sometimes changing between Bundler versions. For the purpose of this discussion, let's set everything related to lockfile versioning aside.
  2. At least myself, @deivid-rodriguez, and @indirect are in favor of the BundlerVersionFinder (the version switcher) being removed.
  3. If it's removed: we'll replace it with a different system that takes into account everything we've learned from the problems with the version switcher. The details of such a replacement are still being ironed out.

There's still some internal discussion ongoing, but that's a summary of the state of things. 🙂

EDIT: Accidentally implied more of a consensus than there is. Sorry about that. 😅

@deivid-rodriguez
Copy link
Member

We're taking several steps to improve the ability to lock the bundler version, so I don't think it makes sense to keep this issue open. Let's discuss things in other tickets.

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

Successfully merging a pull request may close this issue.

6 participants