Skip to content

Bump bundler from 2.3.8 to 2.3.9 and automate it #4884

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.

Sign up for GitHub

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

Merged
landongrindheim merged 1 commit into from
Mar 31, 2022
Merged

Bump bundler from 2.3.8 to 2.3.9 and automate it #4884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ RUN apt-add-repository ppa:brightbox/ruby-ng \
&& apt-get install -y --no-install-recommends ruby2.7 ruby2.7-dev \
&& gem update --system 3.2.20 \
&& gem install bundler -v 1.17.3 --no-document \
&& gem install bundler -v 2.3.8 --no-document \
&& gem install bundler -v 2.3.9 --no-document \
&& rm -rf /var/lib/gems/2.7.0/cache/* \
&& rm -rf /var/lib/apt/lists/*

Expand Down
26 changes: 26 additions & 0 deletions bin/update-bundler.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#!/usr/bin/env ruby
# frozen_string_literal: true

# This script bumps the bundler version used, since we reference it in a few
# different places.

require "excon"
require "json"

LATEST_VERSION = JSON.parse(Excon.get("https://rubygems.org/api/v1/gems/bundler.json").body)["version"]
CURRENT_VERSION = File.read("Dockerfile").match(/gem install bundler -v (2.\d+\.\d+)/)[1]

def update_file(filename)
File.open(filename, "r+") do |f|
contents = f.read
f.rewind
f.write(contents.gsub(CURRENT_VERSION, LATEST_VERSION))
end
end

update_file("Dockerfile")
update_file("bundler/helpers/v2/build")
update_file("bundler/lib/dependabot/bundler/helpers.rb")
update_file("bundler/spec/dependabot/bundler/helper_spec.rb")
update_file("bundler/script/ci-test")
update_file("bundler/spec/spec_helper.rb")
6 changes: 3 additions & 3 deletions bundler/helpers/v2/build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@ cd "$install_dir"

# NOTE: Sets `BUNDLED WITH` to match the installed v2 version in Gemfile.lock
# forcing specs and native helpers to run with the same version
BUNDLER_VERSION=2.3.8 bundle config --local path ".bundle"
BUNDLER_VERSION=2.3.8 bundle config --local without "test"
BUNDLER_VERSION=2.3.8 bundle install
BUNDLER_VERSION=2.3.9 bundle config --local path ".bundle"
BUNDLER_VERSION=2.3.9 bundle config --local without "test"
BUNDLER_VERSION=2.3.9 bundle install
2 changes: 1 addition & 1 deletion bundler/lib/dependabot/bundler/helpers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Dependabot
module Bundler
module Helpers
V1 = "1.17.3"
V2 = "2.3.8"
V2 = "2.3.9"
Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez Mar 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The Bundler version selection behavior implemented in this file should be the default as long as there's single v1 and v2 versions installed in the running environment. If that's the case I think it should be possible to completely remove this logic (don't pass BUNDLER_VERSION at all to native helpers) and let RubyGems do the right thing. But it seems something to consider for the future, definitely not for this PR.

Copy link
Copy Markdown
Member Author

@jurre jurre Mar 21, 2022

Choose a reason for hiding this comment

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

Aye, yeah we should definitely try that and see if it doesn't break anything!

# If we are updating a project with no Gemfile.lock, we default to the
# newest version we support
DEFAULT = V2
Expand Down
4 changes: 2 additions & 2 deletions bundler/script/ci-test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fi

if [[ "$SUITE_NAME" == "bundler2" ]]; then
cd helpers/v2 \
&& BUNDLER_VERSION=2.3.8 bundle install \
&& BUNDLER_VERSION=2.3.8 bundle exec rspec spec \
&& BUNDLER_VERSION=2.3.9 bundle install \
&& BUNDLER_VERSION=2.3.9 bundle exec rspec spec \
&& cd -
fi
2 changes: 1 addition & 1 deletion bundler/spec/dependabot/bundler/helper_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
end

let(:v1) { "1.17.3" }
let(:v2) { "2.3.8" }
let(:v2) { "2.3.9" }
Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez Mar 21, 2022

Choose a reason for hiding this comment

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

I think this spec could be relaxed to not hardcode exact versions, but instead just check the major version selected.

Copy link
Copy Markdown
Member Author

@jurre jurre Mar 22, 2022

Choose a reason for hiding this comment

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

I'm not sure, right now I think the exact version is relevant, if we do this then we can maybe relax it?

Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez Mar 22, 2022

Choose a reason for hiding this comment

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

These specs are unit tests for the Bundler::Helpers.bundler_version method, which selects an exact bundler version according to the lockfile contents, using the following criteria:

  • No lockfile -> The exact v2 version dependabot has available.
  • Lockfile with no BUNDLED WITH section -> The exact v2 version dependabot has available.
  • Lockfile with BUNDLED WITH 1.x section -> The exact v1 version dependabot has available.
  • Lockfie with BUNDLED WITH 2.x section or higher -> The exact v2 version dependabot has available.

Namely, a unit test for this method

dependabot-core/bundler/lib/dependabot/bundler/helpers.rb

Lines 18 to 26 in ea8fbaf

def self.bundler_version(lockfile)
return DEFAULT unless lockfile
if (matches = lockfile.content.match(BUNDLER_MAJOR_VERSION_REGEX))
matches[:version].to_i >= 2 ? V2 : V1
else
FAILOVER
end
end

In my opinion, testing that the result is a valid 3 component version number, and that the first segment is correct (1.x or 2.x) is enough. The exact version itself that dependabot is using is "configuration" and should not need a test, because it's not useful for detecting bugs and it means essentially testing the specific value of a constant, which defeats one of the points of using constants (keep things DRY).

Copy link
Copy Markdown
Contributor

@deivid-rodriguez deivid-rodriguez Mar 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

That said that just my personal opinion but it's no big deal, and indeed if #4884 (comment) works, all this code will go away, so these tests will go too anways.


describe "#bundler_version" do
def described_method(lockfile)
Expand Down
2 changes: 1 addition & 1 deletion bundler/spec/spec_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def self.use_bundler_2?
end

def self.bundler_version
use_bundler_2? ? "2.3.8" : "1.17.3"
use_bundler_2? ? "2.3.9" : "1.17.3"
end

def self.bundler_major_version
Expand Down