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

Redo: Run Bundler v1 helpers with explicit version #3223

Merged
merged 1 commit into from
Mar 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ ENV DEPENDABOT_NATIVE_HELPERS_PATH="/opt" \
RUN bash /opt/terraform/helpers/build /opt/terraform && \
bash /opt/python/helpers/build /opt/python && \
bash /opt/dep/helpers/build /opt/dep && \
bash /opt/bundler/helpers/build /opt/bundler && \
mkdir -p /opt/bundler/v1 && bash /opt/bundler/helpers/v1/build /opt/bundler/v1 && \
bash /opt/go_modules/helpers/build /opt/go_modules && \
bash /opt/npm_and_yarn/helpers/build /opt/npm_and_yarn && \
bash /opt/hex/helpers/build /opt/hex && \
Expand Down
2 changes: 2 additions & 0 deletions bundler/helpers/v1/.bundle/config
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
BUNDLE_PATH: ".bundle"
9 changes: 9 additions & 0 deletions bundler/helpers/v1/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/.bundle/*
!/.bundle/config
/.env
/tmp
/dependabot-*.gem
Gemfile.lock
spec/fixtures/projects/*/.bundle/
!spec/fixtures/projects/**/Gemfile.lock
!spec/fixtures/projects/**/vendor
7 changes: 7 additions & 0 deletions bundler/helpers/v1/Gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

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
6 changes: 6 additions & 0 deletions bundler/helpers/build → bundler/helpers/v1/build
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,15 @@ fi

helpers_dir="$(dirname "${BASH_SOURCE[0]}")"
cp -r \
"$helpers_dir/.bundle" \
"$helpers_dir/lib" \
"$helpers_dir/monkey_patches" \
"$helpers_dir/run.rb" \
"$helpers_dir/Gemfile" \
"$install_dir"

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
BUNDLER_VERSION=1 bundle install
File renamed without changes.
13 changes: 9 additions & 4 deletions bundler/lib/dependabot/bundler/file_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require "dependabot/file_parsers/base"
require "dependabot/bundler/file_updater/lockfile_updater"
require "dependabot/bundler/native_helpers"
require "dependabot/bundler/helpers"
require "dependabot/bundler/version"
require "dependabot/shared_helpers"
require "dependabot/errors"
Expand Down Expand Up @@ -129,8 +130,8 @@ def parsed_gemfile
repo_contents_path) do
write_temporary_dependency_files

SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "parsed_gemfile",
args: {
gemfile_name: gemfile.name,
Expand Down Expand Up @@ -159,8 +160,8 @@ def parsed_gemspec(file)
repo_contents_path) do
write_temporary_dependency_files

SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "parsed_gemspec",
args: {
gemspec_name: file.name,
Expand Down Expand Up @@ -298,6 +299,10 @@ def imported_ruby_files
select { |f| f.name.end_with?(".rb") }.
reject { |f| f.name == "gems.rb" }
end

def bundler_version
@bundler_version ||= Helpers.bundler_version(lockfile)
end
end
end
end
Expand Down
9 changes: 7 additions & 2 deletions bundler/lib/dependabot/bundler/file_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require "dependabot/file_updaters"
require "dependabot/file_updaters/base"
require "dependabot/bundler/native_helpers"
require "dependabot/bundler/helpers"
require "dependabot/file_updaters/vendor_updater"

module Dependabot
Expand Down Expand Up @@ -75,8 +76,8 @@ def vendor_cache_dir
return @vendor_cache_dir if defined?(@vendor_cache_dir)

@vendor_cache_dir =
SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "vendor_cache_dir",
args: {
dir: repo_contents_path
Expand Down Expand Up @@ -159,6 +160,10 @@ def top_level_gemspecs
select { |file| file.name.end_with?(".gemspec") }.
reject(&:support_file?)
end

def bundler_version
@bundler_version ||= Helpers.bundler_version(lockfile)
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
require "dependabot/errors"
require "dependabot/bundler/file_updater"
require "dependabot/bundler/native_helpers"
require "dependabot/bundler/helpers"

module Dependabot
module Bundler
Expand Down Expand Up @@ -64,8 +65,8 @@ def build_updated_lockfile
) do |tmp_dir|
write_temporary_dependency_files

SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "update_lockfile",
args: {
gemfile_name: gemfile.name,
Expand Down Expand Up @@ -301,6 +302,10 @@ def using_bundler2?

lockfile.content.match?(/BUNDLED WITH\s+2/m)
end

def bundler_version
@bundler_version ||= Helpers.bundler_version(lockfile)
end
end
end
end
Expand Down
16 changes: 16 additions & 0 deletions bundler/lib/dependabot/bundler/helpers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

module Dependabot
module Bundler
module Helpers
V1 = "1"
V2 = "2"

# TODO: Add support for bundler v2
# return "v2" if lockfile.content.match?(/BUNDLED WITH\s+2/m)
def self.bundler_version(_lockfile)
V1
end
end
end
end
29 changes: 27 additions & 2 deletions bundler/lib/dependabot/bundler/native_helpers.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,35 @@
# frozen_string_literal: true

require "bundler"
require "dependabot/shared_helpers"

module Dependabot
module Bundler
module NativeHelpers
def self.helper_path
"bundle exec ruby #{File.join(native_helpers_root, 'run.rb')}"
def self.run_bundler_subprocess(function:, args:, bundler_version:)
# Run helper suprocess with all bundler-related ENV variables removed
::Bundler.with_original_env do
SharedHelpers.run_helper_subprocess(
command: helper_path(bundler_version: bundler_version),
function: function,
args: args,
env: {
# Bundler will pick the matching installed major version
"BUNDLER_VERSION" => bundler_version,
"BUNDLE_GEMFILE" => File.join(versioned_helper_path(bundler_version: bundler_version), "Gemfile"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is required when running with bundler 2 installed alongside v1, without this it defaults to v2 during dry-runs.

"BUNDLE_PATH" => File.join(versioned_helper_path(bundler_version: bundler_version), ".bundle")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this to match BUNDLE_GEMFILE but not sure it's necessary, both seemed to work inside and outside the container with v1 and v2 installed

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed synchronously, I'm 👍🏻 on having versioned_helper_path(bundler_version: bundler_version) as a clean(ish) room bundler path for all things related to the native helper process.

}
)
end
end

def self.versioned_helper_path(bundler_version:)
native_helper_version = "v#{bundler_version}"
File.join(native_helpers_root, native_helper_version)
end

def self.helper_path(bundler_version:)
"ruby #{File.join(versioned_helper_path(bundler_version: bundler_version), 'run.rb')}"
end

def self.native_helpers_root
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require "dependabot/bundler/update_checker"
require "dependabot/bundler/native_helpers"
require "dependabot/bundler/helpers"
require "dependabot/shared_helpers"

module Dependabot
Expand All @@ -28,8 +29,8 @@ def initialize(dependency_files:, repo_contents_path:, credentials:)
# * requirement [String] the requirement on the target_dependency
def conflicting_dependencies(dependency:, target_version:)
in_a_native_bundler_context(error_handling: false) do |tmp_dir|
SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "conflicting_dependencies",
args: {
dir: tmp_dir,
Expand All @@ -42,6 +43,12 @@ def conflicting_dependencies(dependency:, target_version:)
)
end
end

private

def bundler_version
@bundler_version ||= Helpers.bundler_version(lockfile)
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require "dependabot/bundler/file_parser"
require "dependabot/bundler/file_updater/lockfile_updater"
require "dependabot/bundler/native_helpers"
require "dependabot/bundler/helpers"
require "dependabot/bundler/update_checker"
require "dependabot/bundler/update_checker/requirements_updater"
require "dependabot/errors"
Expand Down Expand Up @@ -43,8 +44,8 @@ def update_multiple_dependencies?

def force_update
in_a_native_bundler_context(error_handling: false) do |tmp_dir|
updated_deps, specs = SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
updated_deps, specs = NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "force_update",
args: {
dir: tmp_dir,
Expand Down Expand Up @@ -146,6 +147,10 @@ def using_bundler2?

lockfile.content.match?(/BUNDLED WITH\s+2/m)
end

def bundler_version
@bundler_version ||= Helpers.bundler_version(lockfile)
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# frozen_string_literal: true

require "dependabot/bundler/native_helpers"
require "dependabot/bundler/helpers"

module Dependabot
module Bundler
class UpdateChecker
Expand Down Expand Up @@ -53,8 +56,8 @@ def latest_git_version_details

SharedHelpers.with_git_configured(credentials: credentials) do
in_a_native_bundler_context do |tmp_dir|
SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "depencency_source_latest_git_version",
args: {
dir: tmp_dir,
Expand Down Expand Up @@ -98,8 +101,8 @@ def dependency_rubygems_uri
def private_registry_versions
@private_registry_versions ||=
in_a_native_bundler_context do |tmp_dir|
SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "private_registry_versions",
args: {
dir: tmp_dir,
Expand All @@ -118,8 +121,8 @@ def source_type
return @source_type = RUBYGEMS unless gemfile

@source_type = in_a_native_bundler_context do |tmp_dir|
SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "dependency_source_type",
args: {
dir: tmp_dir,
Expand All @@ -135,6 +138,15 @@ def gemfile
dependency_files.find { |f| f.name == "Gemfile" } ||
dependency_files.find { |f| f.name == "gems.rb" }
end

def lockfile
dependency_files.find { |f| f.name == "Gemfile.lock" } ||
dependency_files.find { |f| f.name == "gems.locked" }
end

def bundler_version
@bundler_version ||= Helpers.bundler_version(lockfile)
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

require "dependabot/bundler/update_checker"
require "dependabot/bundler/native_helpers"
require "dependabot/bundler/helpers"
require "dependabot/shared_helpers"
require "dependabot/errors"

Expand Down Expand Up @@ -163,8 +164,8 @@ def handle_bundler_errors(error)

def inaccessible_git_dependencies
in_a_native_bundler_context(error_handling: false) do |tmp_dir|
git_specs = SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
git_specs = NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "git_specs",
args: {
dir: tmp_dir,
Expand All @@ -187,8 +188,8 @@ def inaccessible_git_dependencies

def jfrog_source
in_a_native_bundler_context(error_handling: false) do |dir|
SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "jfrog_source",
args: {
dir: dir,
Expand Down Expand Up @@ -236,6 +237,10 @@ def using_bundler2?

lockfile.content.match?(/BUNDLED WITH\s+2/m)
end

def bundler_version
@bundler_version ||= Helpers.bundler_version(lockfile)
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require "excon"

require "dependabot/bundler/helpers"
require "dependabot/bundler/update_checker"
require "dependabot/bundler/file_updater/lockfile_updater"
require "dependabot/bundler/requirement"
Expand Down Expand Up @@ -75,8 +76,8 @@ def fetch_latest_resolvable_version_details
# some errors we want to handle specifically ourselves, including
# potentially retrying in the case of the Ruby version being locked
in_a_native_bundler_context(error_handling: false) do |tmp_dir|
details = SharedHelpers.run_helper_subprocess(
command: NativeHelpers.helper_path,
details = NativeHelpers.run_bundler_subprocess(
bundler_version: bundler_version,
function: "resolve_version",
args: {
dependency_name: dependency.name,
Expand Down Expand Up @@ -218,6 +219,10 @@ def using_bundler2?

lockfile.content.match?(/BUNDLED WITH\s+2/m)
end

def bundler_version
@bundler_version ||= Helpers.bundler_version(lockfile)
end
end
end
end
Expand Down
Loading