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

Bundler: Run v1 native helpers with bundler v1 #3211

Merged
merged 2 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,11 @@ jobs:
run: |
docker run --rm "$CORE_CI_IMAGE" bash -c "cd /opt/npm_and_yarn && npm run lint"
docker run --rm "$CORE_CI_IMAGE" bash -c "cd /opt/npm_and_yarn && npm test"
- name: Run bundler v1 native helper specs
if: matrix.suite == 'bundler'
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it's a big deal right for this PR, but I guess we should add bundler2 to the matrix rather than mess with this existing key?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, was also thinking we could run them in in the same suite a second step as I think they run quickly. Each matrix ends up building the entire image as it's set up currently.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call, that makes sense!

run: |
docker run --rm "$CORE_CI_IMAGE" bash -c \
"cd /home/dependabot/dependabot-core/bundler/helpers/v1 && BUNDLER_VERSION=1 bundle install && BUNDLER_VERSION=1 bundle exec rspec spec"
- name: Run ${{ matrix.suite }} tests with rspec
run: |
docker run --env "CI=true" --env "DEPENDABOT_TEST_ACCESS_TOKEN=$DEPENDABOT_TEST_ACCESS_TOKEN" --rm "$CORE_CI_IMAGE" bash -c \
Expand Down
11 changes: 8 additions & 3 deletions bundler/helpers/v1/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

source "https://rubygems.org"

# NOTE: This is intentionally left blank as it's currently only used to force
# bundler to use v1 when executing native helpers by pointing the BUNDLE_GEMFILE
# env to this Gemfile in Dependabot::Bundler::NativeHelpers
# NOTE: Used to run native helper specs
group :test do
gem "byebug", "11.1.3"
gem "rspec", "3.10.0"
gem "rspec-its", "1.3.0"
gem "vcr", "6.0.0"
gem "webmock", "3.12.1"
end
2 changes: 1 addition & 1 deletion bundler/helpers/v1/build
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,4 @@ cd "$install_dir"

# NOTE: Sets `BUNDLED WITH` to match the installed v1 version in Gemfile.lock
# forcing specs and native helpers to run with the same version
feelepxyz marked this conversation as resolved.
Show resolved Hide resolved
BUNDLER_VERSION=1 bundle install
BUNDLER_VERSION=1 bundle install --without test
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@
described_class.new(
dependency_name: dependency_name,
target_version: target_version,
lockfile_name: lockfile_name
lockfile_name: "Gemfile.lock"
)
end

let(:dependency_name) { "dummy-pkg-a" }
let(:target_version) { "2.0.0" }

let(:gemfile_fixture_name) { "blocked_by_subdep" }
let(:lockfile_fixture_name) { "blocked_by_subdep.lock" }
let(:project_name) { "blocked_by_subdep" }

describe "#conflicting_dependencies" do
subject(:conflicting_dependencies) do
Expand All @@ -37,8 +36,7 @@
end

context "for nested transitive dependencies" do
let(:gemfile_fixture_name) { "transitive_blocking" }
let(:lockfile_fixture_name) { "transitive_blocking.lock" }
let(:project_name) { "transitive_blocking" }
let(:dependency_name) { "activesupport" }
let(:target_version) { "6.0.0" }

Expand Down Expand Up @@ -96,8 +94,7 @@
let(:dependency_name) { "activesupport" }
let(:current_version) { "5.0.0" }
let(:target_version) { "6.0.0" }
let(:gemfile_fixture_name) { "multiple_blocking" }
let(:lockfile_fixture_name) { "multiple_blocking.lock" }
let(:project_name) { "multiple_blocking" }

it "returns all of the blocking dependencies" do
expect(conflicting_dependencies).to match_array(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@

let(:dependency_source) do
described_class.new(
gemfile_name: gemfile_name,
gemfile_name: "Gemfile",
dependency_name: dependency_name
)
end

let(:dependency_name) { "business" }

let(:gemfile_fixture_name) { "specified_source" }
let(:project_name) { "specified_source_no_lockfile" }
let(:registry_url) { "https://repo.fury.io/greysteil/" }
let(:gemfury_business_url) do
"https://repo.fury.io/greysteil/api/v1/dependencies?gems=business"
Expand Down Expand Up @@ -47,7 +47,7 @@
end

context "specified as the default source" do
let(:gemfile_fixture_name) { "specified_default_source" }
let(:project_name) { "specified_default_source_no_lockfile" }

it "returns all versions from the private source" do
is_expected.to eq([
Expand Down Expand Up @@ -152,7 +152,7 @@
end

context "that only implements the old Bundler index format..." do
let(:gemfile_fixture_name) { "sidekiq_pro" }
let(:project_name) { "sidekiq_pro" }
let(:dependency_name) { "sidekiq-pro" }
let(:registry_url) { "https://gems.contribsys.com/" }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@
)
end

let(:gemfile_fixture_name) { "Gemfile" }
let(:lockfile_fixture_name) { "Gemfile.lock" }
let(:project_name) { "gemfile" }

describe "#parsed_gemfile" do
subject(:parsed_gemfile) do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
require "native_spec_helper"
require "shared_contexts"

require "dependabot/dependency"

RSpec.describe Functions::VersionResolver do
include_context "in a temporary bundler directory"
include_context "stub rubygems compact index"
Expand All @@ -13,8 +11,8 @@
described_class.new(
dependency_name: dependency_name,
dependency_requirements: dependency_requirements,
gemfile_name: gemfile_name,
lockfile_name: lockfile_name
gemfile_name: "Gemfile",
lockfile_name: "Gemfile.lock"
)
end

Expand All @@ -37,8 +35,7 @@
in_tmp_folder { version_resolver.version_details }
end

let(:gemfile_fixture_name) { "Gemfile" }
let(:lockfile_fixture_name) { "Gemfile.lock" }
let(:project_name) { "gemfile" }
let(:requirement_string) { " >= 0" }

its([:version]) { is_expected.to eq(Gem::Version.new("1.4.0")) }
Expand All @@ -47,8 +44,7 @@
context "with a private gemserver source" do
include_context "stub rubygems compact index"

let(:gemfile_fixture_name) { "specified_source" }
let(:lockfile_fixture_name) { "specified_source.lock" }
let(:project_name) { "specified_source" }
let(:requirement_string) { ">= 0" }

before do
Expand All @@ -72,8 +68,7 @@
end

context "with a git source" do
let(:gemfile_fixture_name) { "git_source" }
let(:lockfile_fixture_name) { "git_source.lock" }
let(:project_name) { "git_source" }

its([:version]) { is_expected.to eq(Gem::Version.new("1.6.0")) }
its([:fetcher]) { is_expected.to be_nil }
Expand Down
49 changes: 49 additions & 0 deletions bundler/helpers/v1/spec/native_spec_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# frozen_string_literal: true

require "rspec/its"
require "webmock/rspec"
require "byebug"

$LOAD_PATH.unshift(File.expand_path("../lib", __dir__))
$LOAD_PATH.unshift(File.expand_path("../monkey_patches", __dir__))

# Bundler monkey patches
require "definition_ruby_version_patch"
require "definition_bundler_version_patch"
require "git_source_patch"

require "functions"

RSpec.configure do |config|
config.color = true
config.order = :rand
config.mock_with(:rspec) { |mocks| mocks.verify_partial_doubles = true }
config.raise_errors_for_deprecations!
end

# Duplicated in lib/dependabot/bundler/file_updater/lockfile_updater.rb
# TODO: Stop sanitizing the lockfile once we have bundler 2 installed
LOCKFILE_ENDING = /(?<ending>\s*(?:RUBY VERSION|BUNDLED WITH).*)/m.freeze

def project_dependency_files(project)
project_path = File.expand_path(File.join("../../spec/fixtures/projects/bundler1", project))
Dir.chdir(project_path) do
# NOTE: Include dotfiles (e.g. .npmrc)
files = Dir.glob("**/*", File::FNM_DOTMATCH)
files = files.select { |f| File.file?(f) }
files.map do |filename|
content = File.read(filename)
if filename == "Gemfile.lock"
content = content.gsub(LOCKFILE_ENDING, "")
end
{
name: filename,
content: content
}
end
end
end

def fixture(*name)
File.read(File.join("../../spec/fixtures", File.join(*name)))
end
59 changes: 59 additions & 0 deletions bundler/helpers/v1/spec/shared_contexts.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# frozen_string_literal: true

require "bundler/compact_index_client"
require "bundler/compact_index_client/updater"

TMP_DIR_PATH = File.expand_path("../tmp", __dir__)

RSpec.shared_context "in a temporary bundler directory" do
let(:project_name) { "gemfile" }

let(:tmp_path) do
Dir.mkdir(TMP_DIR_PATH) unless Dir.exist?(TMP_DIR_PATH)
dir = Dir.mktmpdir("native_helper_spec_", TMP_DIR_PATH)
Pathname.new(dir).expand_path
end

before do
project_dependency_files(project_name).each do |file|
File.write(File.join(tmp_path, file[:name]), file[:content])
end
end

def in_tmp_folder(&block)
Dir.chdir(tmp_path, &block)
end
end

RSpec.shared_context "without caching rubygems" do
before do
# Stub Bundler to stop it using a cached versions of Rubygems
allow_any_instance_of(Bundler::CompactIndexClient::Updater).
to receive(:etag_for).and_return("")
end
end

RSpec.shared_context "stub rubygems compact index" do
include_context "without caching rubygems"

before do
# Stub the Rubygems index
stub_request(:get, "https://index.rubygems.org/versions").
to_return(
status: 200,
body: fixture("ruby", "rubygems_responses", "index")
)

# Stub the Rubygems response for each dependency we have a fixture for
fixtures =
Dir[File.join("../../spec", "fixtures", "ruby", "rubygems_responses", "info-*")]
fixtures.each do |path|
dep_name = path.split("/").last.gsub("info-", "")
stub_request(:get, "https://index.rubygems.org/info/#{dep_name}").
to_return(
status: 200,
body: fixture("ruby", "rubygems_responses", "info-#{dep_name}")
)
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
source "https://rubygems.org"

gem "activesupport", "5.0.0"
gem "actionview", "5.0.0"
gem "actionmailer", "5.0.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
GEM
remote: https://rubygems.org/
specs:
actionmailer (5.0.0)
actionpack (= 5.0.0)
actionview (= 5.0.0)
activejob (= 5.0.0)
mail (~> 2.5, >= 2.5.4)
rails-dom-testing (~> 2.0)
actionpack (5.0.0)
actionview (= 5.0.0)
activesupport (= 5.0.0)
rack (~> 2.0)
rack-test (~> 0.6.3)
rails-dom-testing (~> 2.0)
rails-html-sanitizer (~> 1.0, >= 1.0.2)
actionview (5.0.0)
activesupport (= 5.0.0)
builder (~> 3.1)
erubis (~> 2.7.0)
rails-dom-testing (~> 2.0)
rails-html-sanitizer (~> 1.0, >= 1.0.2)
activejob (5.0.0)
activesupport (= 5.0.0)
globalid (>= 0.3.6)
activesupport (5.0.0)
concurrent-ruby (~> 1.0, >= 1.0.2)
i18n (~> 0.7)
minitest (~> 5.1)
tzinfo (~> 1.1)
builder (3.2.4)
concurrent-ruby (1.1.7)
crass (1.0.6)
erubis (2.7.0)
globalid (0.4.2)
activesupport (>= 4.2.0)
i18n (0.9.5)
concurrent-ruby (~> 1.0)
loofah (2.7.0)
crass (~> 1.0.2)
nokogiri (>= 1.5.9)
mail (2.7.1)
mini_mime (>= 0.1.1)
mini_mime (1.0.2)
mini_portile2 (2.4.0)
minitest (5.14.2)
nokogiri (1.10.10)
mini_portile2 (~> 2.4.0)
rack (2.2.3)
rack-test (0.6.3)
rack (>= 1.0)
rails-dom-testing (2.0.3)
activesupport (>= 4.2.0)
nokogiri (>= 1.6)
rails-html-sanitizer (1.3.0)
loofah (~> 2.3)
thread_safe (0.3.6)
tzinfo (1.2.7)
thread_safe (~> 0.1)

PLATFORMS
ruby

DEPENDENCIES
actionmailer (= 5.0.0)
actionview (= 5.0.0)
activesupport (= 5.0.0)

BUNDLED WITH
2.1.4
5 changes: 5 additions & 0 deletions bundler/spec/fixtures/projects/bundler1/sidekiq_pro/Gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
source 'https://rubygems.org'

source "https://username:[email protected]/" do
gem 'sidekiq-pro'
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
GEM
remote: https://rubygems.org/
remote: https://username:[email protected]/
specs:
concurrent-ruby (1.0.5)
connection_pool (2.2.1)
rack (1.4.7)
rack-protection (1.5.3)
rack
redis (3.3.3)
sidekiq (4.2.9)
concurrent-ruby (~> 1.0)
connection_pool (~> 2.2, >= 2.2.0)
rack-protection (>= 1.5.0)
redis (~> 3.2, >= 3.2.1)
sidekiq-pro (3.4.4)
sidekiq (>= 4.1.5)

PLATFORMS
ruby

DEPENDENCIES
sidekiq-pro!

BUNDLED WITH
1.15.3
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
source 'https://[email protected]/greysteil/'

gem 'business'
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
source 'https://rubygems.org'

gem 'statesman'

source 'https://[email protected]/greysteil/' do
gem 'business'
end
Loading