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

Should we use the latest Bundler release or the Bundler shipped with that Ruby by default? #358

Closed
eregon opened this issue Jul 20, 2022 · 10 comments · Fixed by #362
Closed
Assignees

Comments

@eregon
Copy link
Member

eregon commented Jul 20, 2022

Currently it's the former but I'm thinking to change to the latter.

But it led to several issues already, including rubygems/rubygems#5691 and previous issues where a Bundler release was broken for some aspect (I'm too lazy to search but I'd think at least 2 other issues).
To be clear, every software has bugs, I think the Bundler is doing a good job, no problem there.
But I think this is no coincidence, the Bundler shipped with a Ruby is more tested (because it's tested in that Ruby's CI rather extensively and manually during that Ruby's development) and more stable (since it doesn't change that often).

Of course, such issues in Bundler still need to be fixed and some users might need a newer Bundler than what is shipped with the Ruby.
But exposing CI users more than in local development to this kind of issues does not seem ideal.

Because in local development, one doesn't gem install bundler before every bundle install.
In fact I rarely do gem install bundler but tend to use the bundler shipped with that Ruby (mostly to not add another variable part in my environment/repro).

Note we already use the "shipped" Bundler for -head versions, but for Ruby releases we currently gem install bundler.
So the change would be for Ruby releases which do ship with Bundler to just use that Bundler and no longer gem install bundler by default. Of course one could still configure they want latest Bundler with: bundler: latest.

@MSP-Greg @dentarg @deivid-rodriguez Any opinion on this?

@deivid-rodriguez
Copy link
Contributor

I of course prefer the latest version than the default version. And I disagree the default version is more tested, sometimes the default version is only cherry-picked by ruby-core a few days days before releases. Yes, we sometimes introduce regressions, but those regressions could also happen in the default version (and it wouldn't be the first time). If you change this, you'll get the opposite problem, people might run into issues already fixed in the latest version.

@eregon
Copy link
Member Author

eregon commented Jul 20, 2022

And I disagree the default version is more tested, sometimes the default version is only cherry-picked by ruby-core a few days days before releases.

Right, this is different for CRuby. For TruffleRuby I can definitely say the default version is more tested (e.g. I use/"test" it a lot more than "all newer Bundler versions"). And since it is a version used in a CRuby release, potential issues are typically well known or also seen in local development.

If you change this, you'll get the opposite problem, people might run into issues already fixed in the latest version.

True. At least they can easily use bundler: latest or bundler: SOME_VERSION for fixing that, and they'd need to document that for anyone working on the project too anyway (e.g. ruby-install/ruby-build won't install latest Bundler for you).

I think setup-ruby by default shouldn't introduce variability, but currently that's not true due to using the latest bundler, I feel that's the wrong default.
For instance if a user says ruby-version: 3.0.5 they get that and nothing else. Any other variability is explicitly introduced by their project (e.g., a Gemfile without a Gemfile.lock), so that much is expected.
But the Bundler version is currently also changing, and what's more it actually only changes for releases, i.e., the "stable versions".

FWIW setup-ruby used to only install bundler for Rubies without Bundler.
It changed in #47 / #38 (comment) I don't remember exactly why but probably because it was easiest to install "a Bundler 2" on Rubies without one, and it ensures the same Bundler 2 is used for all ruby versions.

@deivid-rodriguez
Copy link
Contributor

Well, although I disagree with the argument that "default is more stable than latest", I'm always in for CI stability, so I agree with your point about variability, and I agree using the default version is probably a better default, indeed.

@dentarg
Copy link

dentarg commented Jul 20, 2022

I can see it being 50/50 between the two options. Either you run into problems fixed in the latest version of Bundler or you run into some incompatibility with latest Bundler. 😅

I think setup-ruby by default shouldn't introduce variability, but currently that's not true due to using the latest bundler, I feel that's the wrong default.

I agree with this.

Not doing that gem install bundler would also make CI a bit quicker? Would be a good thing.

(The sped up may be defeated by the fact that a recent enough Bundler will switch (install the later version) to the version you specified in your lockfile? If you are keeping up with Bundler releases in your lockfile, that is.)

@eregon
Copy link
Member Author

eregon commented Jul 26, 2022

Great, it seems we all 3 agree the Bundler shipped with that Ruby should be used by default, for CI stability.

Not doing that gem install bundler would also make CI a bit quicker? Would be a good thing.

Yes, on CRuby it's just a couple seconds or so, but on other Rubies it's longer.

The sped up may be defeated by the fact that a recent enough Bundler will switch (install the later version) to the version you specified in your lockfile? If you are keeping up with Bundler releases in your lockfile, that is.

If there is a Gemfile.lock with a BUNDLED WITH section we use the Bundler version specified there, and that won't change. We do it in the action because we also want to do it for older Bundler versions which don't do that themselves.

eregon added a commit to eregon/setup-ruby that referenced this issue Jul 26, 2022
eregon added a commit that referenced this issue Jul 26, 2022
eregon added a commit to eregon/sinatra that referenced this issue Jul 26, 2022
No longer needed since ruby/setup-ruby#358
eregon added a commit to eregon/sinatra that referenced this issue Jul 26, 2022
No longer needed since ruby/setup-ruby#358
@eregon
Copy link
Member Author

eregon commented Jul 26, 2022

Mmh, this seems problematic in practice unfortunately :/ with lines like this in the Gemfile:

gem 'rack-test', github: 'rack/rack-test'

I tried it on Sinatra's CI (sinatra/sinatra#1803):

   /opt/hostedtoolcache/Ruby/2.6.10/x64/bin/bundle lock
  The git source `git://github.com/rack/rack-test.git` uses the `git` protocol, which transmits data without encryption. Disable this warning with `bundle config git.allow_insecure true`, or switch to the `https` protocol to keep your data secure.
  Fetching git://github.com/rack/rack-test.git
  fatal: unable to connect to github.com:
  github.com[0: 140.82.113.4]: errno=Connection timed out
  
  
  Retrying `git clone 'git://github.com/rack/rack-test.git' "/home/runner/work/sinatra/sinatra/vendor/bundle/ruby/2.6.0/cache/bundler/git/rack-test-5785c108954e778be48b7f03fe5a97b437dc741b" --bare --no-hardlinks --quiet` due to error (2/4): Bundler::Source::Git::GitCommandError Git error: command `git clone 'git://github.com/rack/rack-test.git' "/home/runner/work/sinatra/sinatra/vendor/bundle/ruby/2.6.0/cache/bundler/git/rack-test-5785c108954e778be48b7f03fe5a97b437dc741b" --bare --no-hardlinks --quiet` in directory /home/runner/work/sinatra/sinatra has failed.fatal: unable to connect to github.com:
  github.com[0: 140.82.114.3]: errno=Connection timed out
  
  
  Retrying `git clone 'git://github.com/rack/rack-test.git' "/home/runner/work/sinatra/sinatra/vendor/bundle/ruby/2.6.0/cache/bundler/git/rack-test-5785c108954e778be48b7f03fe5a97b437dc741b" --bare --no-hardlinks --quiet` due to error (3/4): Bundler::Source::Git::GitCommandError Git error: command `git clone 'git://github.com/rack/rack-test.git' "/home/runner/work/sinatra/sinatra/vendor/bundle/ruby/2.6.0/cache/bundler/git/rack-test-5785c108954e778be48b7f03fe5a97b437dc741b" --bare --no-hardlinks --quiet` in directory /home/runner/work/sinatra/sinatra has failed.fatal: unable to connect to github.com:
  github.com[0: 140.82.112.4]: errno=Connection timed out
  
  
  Retrying `git clone 'git://github.com/rack/rack-test.git' "/home/runner/work/sinatra/sinatra/vendor/bundle/ruby/2.6.0/cache/bundler/git/rack-test-5785c108954e778be48b7f03fe5a97b437dc741b" --bare --no-hardlinks --quiet` due to error (4/4): Bundler::Source::Git::GitCommandError Git error: command `git clone 'git://github.com/rack/rack-test.git' "/home/runner/work/sinatra/sinatra/vendor/bundle/ruby/2.6.0/cache/bundler/git/rack-test-5785c108954e778be48b7f03fe5a97b437dc741b" --bare --no-hardlinks --quiet` in directory /home/runner/work/sinatra/sinatra has failed.fatal: unable to connect to github.com:
  github.com[0: 140.82.112.3]: errno=Connection timed out
  
  
  Git error: command `git clone 'git://github.com/rack/rack-test.git'
  "/home/runner/work/sinatra/sinatra/vendor/bundle/ruby/2.6.0/cache/bundler/git/rack-test-5785c108954e778be48b7f03fe5a97b437dc741b"
  --bare --no-hardlinks --quiet` in directory /home/runner/work/sinatra/sinatra
  has failed.
  Took 525.30 seconds

Also it warns about insecure, but recent versions don't warn, since they actually use https for gem 'rack-test', github: 'rack/rack-test'.

  /opt/hostedtoolcache/Ruby/2.7.6/x64/bin/bundle lock
  Fetching https://github.com/rack/rack-test.git
  fatal: Needed a single revision
  Git error: command `git rev-parse --verify master` in directory
  /home/runner/work/sinatra/sinatra has failed.
  Revision master does not exist in the repository
  https://github.com/rack/rack-test.git. Maybe you misspelled it?
  If this error persists you could try removing the cache directory
  '/home/runner/work/sinatra/sinatra/vendor/bundle/ruby/2.7.0/cache/bundler/git/rack-test-80082fd7356f34028cb681263704bb2e8e9ef981'

On 2.6 I think we should probably keep installing Bundler 2 (latest Bundler 2 or maybe some known version of Bundler 2 but then which one?): #362 (comment)

On 2.7, maybe we consider the Bundler 2.1.4 it has "too old" and so install latest/more recent Bundler when Bundler < 2.2?
FWIW latest TruffleRuby and Ruby 3.0 ship with Bundler 2.2.32 (https://stdgems.org/3.0.4/).

@eregon
Copy link
Member Author

eregon commented Jul 26, 2022

#363 to use latest Bundler on CRuby 2.6 & 2.7, that passes the sinatra CI: https://github.com/eregon/sinatra/runs/7527978961?check_suite_focus=true

Not ideal, but I think we need something like this to not break a ton of CIs.
Potentially we could express it differently like a minimum Bundler 2 version (and possibly minumum Bundler 1 version if 1.17.3 works better than 1.17.2).

eregon added a commit to eregon/sinatra that referenced this issue Jul 26, 2022
No longer needed since ruby/setup-ruby#358
@eregon
Copy link
Member Author

eregon commented Jul 26, 2022

Actual changes with #363 merged:

  • Ruby 1.9-2.5: no change (latest Bundler)
  • Ruby 2.6: would use default Bundler 1.17.2, no change with Use latest Bundler on CRuby 2.6 and 2.7 as their default Bundler is too old #363 (latest Bundler)
  • Ruby 2.7: would use default Bundler 2.1.4, no change with Use latest Bundler on CRuby 2.6 and 2.7 as their default Bundler is too old #363 (latest Bundler)
  • Ruby 3.0: use default Bundler 2.2.33 instead of latest (2.3.18)
  • Ruby 3.1: use default Bundler 2.3.7 instead of latest (2.3.18)
  • JRuby < 9.3: no change (latest Bundler due to no default Bundler)
  • JRuby >= 9.3: default Bundler 2.2.29 instead of latest (2.3.18)
  • TruffleRuby < 21.0: default Bundler 1.17.2 instead of latest, but they are old releases, could use latest Bundler for those
  • TruffleRuby 21.0-21.3: default Bundler 2.1.4 instead of latest, but they are old releases, could use latest Bundler for those
  • TruffleRuby 22.0 - 22.1: default Bundler 2.2.22 instead of latest
  • TruffleRuby 22.2: default Bundler 2.2.32 instead of latest
  • head versions: no change, was already default bundler

So Bundler 2.2.22 is the minimum Bundler 2 version, older than that we install latest Bundler.

@eregon
Copy link
Member Author

eregon commented Jul 29, 2022

I'm adding a test for this, simply bundle install of a Gemfile with gem 'rack-test', github: 'rack/rack-test'.

That fails on Ruby 1.9 - Ruby 2.2, which all use the latest Bundler 1 (Bundler 2 requires Ruby 2.3+), specifically 1.17.3:
https://github.com/eregon/setup-ruby/runs/7577816666?check_suite_focus=true

 The git source `git://github.com/rack/rack-test.git` uses the `git` protocol, which transmits data without encryption. Disable this warning with `bundle config git.allow_insecure true`, or switch to the `https` protocol to keep your data secure.
Fetching git://github.com/rack/rack-test.git
fatal: unable to connect to github.com:
github.com[0: 140.82.114.4]: errno=Connection timed out

So Bundler 1 simply doesn't support gem github:, because it uses the git:// protocol and that is no longer supported by GitHub since 15 March 2022: https://github.blog/2021-09-01-improving-git-protocol-security-github/
So at this point I guess we should consider Bundler 1 unmaintained/won't be fixed (fine by me). It still works for some gemfiles, but not for gem github:.

In other words, we cannot do anything about it in setup-ruby, and I'll just exclude the test for these old Rubies.
Also, we never want to use a Bundler 1 default gem (if we can install Bundler 2), because it's not working well.

Ruby 2.6 and Ruby 2.7 fail as mentioned above with their default Bundler, we'll just use latest Bundler 2 for them.

@eregon
Copy link
Member Author

eregon commented Jul 29, 2022

To have a simple consistent rule, I changed it to:

  • The default Bundler gem is only used if >= 2.2, otherwise the latest Bundler is installed.

d5ee236

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

Successfully merging a pull request may close this issue.

3 participants