From 35f22a4ad0e4fec86f9ac7a5ff63d62c575d294d Mon Sep 17 00:00:00 2001 From: jurre Date: Wed, 28 Oct 2020 13:01:43 +0100 Subject: [PATCH 01/13] Raise descriptive error when update is not possible Currently we are actually raising a `can't modify frozen NilClass` error because we attempt to `instance_variable_set` on `nil`, as we try to find `dependency` in the Gemfile. This is a small step towards figuring out what dependency is blocking for an update. --- bundler/helpers/lib/functions/force_updater.rb | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/bundler/helpers/lib/functions/force_updater.rb b/bundler/helpers/lib/functions/force_updater.rb index 6e262fc9fa..34bf3e2bba 100644 --- a/bundler/helpers/lib/functions/force_updater.rb +++ b/bundler/helpers/lib/functions/force_updater.rb @@ -1,5 +1,7 @@ module Functions class ForceUpdater + class TransitiveDependencyError < StandardError; end + def initialize(dependency_name:, target_version:, gemfile_name:, lockfile_name:, update_multiple_dependencies:) @dependency_name = dependency_name @@ -113,12 +115,15 @@ def build_definition(other_updates:) # Set the requirement for the gem we're forcing an update of new_req = Gem::Requirement.create("= #{target_version}") - definition.dependencies. - find { |d| d.name == dependency_name }. - tap do |dep| - dep.instance_variable_set(:@requirement, new_req) - dep.source = nil if dep.source.is_a?(Bundler::Source::Git) - end + dep = definition.dependencies. + find { |d| d.name == dependency_name } + + # If the dependency is not found in the Gemfile it means this is a + # transitive dependency that we can't force update. + raise TransitiveDependencyError unless dep + + dep.instance_variable_set(:@requirement, new_req) + dep.source = nil if dep.source.is_a?(Bundler::Source::Git) definition end From 01499e82229f7472334911850d388553c989c6f6 Mon Sep 17 00:00:00 2001 From: jurre Date: Mon, 2 Nov 2020 16:48:37 +0100 Subject: [PATCH 02/13] [SPIKE] explain why a bundler update was blocked This returns a message from ForceUpdater that explains why a certain update was not possible, by traversing the dependency tree and finding all parent dependencies that are not satisfied by the target version. Few things to improve: - The message is now formatted in the native helper, I would prefer to return raw data from there, and leave any formatting to whatever drives dependabot. - Because we currently depend on the native helper returning an error, it is hard to pass data from the native function to dependabot-core. - We need to call `latest_version_resolvable_with_full_unlock?` in order to hit the error that sets this new message. I'd prefer to be able to just call a method on the UpdateChecker instead. - I dislike having to string compare against the native errors `error_type`, it would be nice to have more granular errors that we can return from there. --- .../helpers/lib/functions/force_updater.rb | 27 ++++++++++++++++--- .../lib/dependabot/bundler/update_checker.rb | 7 ++++- .../dependabot/bundler/update_checker_spec.rb | 9 +++++++ 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/bundler/helpers/lib/functions/force_updater.rb b/bundler/helpers/lib/functions/force_updater.rb index 34bf3e2bba..90e75efc33 100644 --- a/bundler/helpers/lib/functions/force_updater.rb +++ b/bundler/helpers/lib/functions/force_updater.rb @@ -26,11 +26,11 @@ def run definition.resolve_remotely! specs = definition.resolve updates = [{ name: dependency_name }] + - other_updates.map { |dep| { name: dep.name } } + other_updates.map { |dep| { name: dep.name } } specs = specs.map do |dep| { name: dep.name, - version: dep.version, + version: dep.version } end [updates, specs] @@ -56,7 +56,7 @@ def run attr_reader :dependency_name, :target_version, :gemfile_name, :lockfile_name, :credentials, :update_multiple_dependencies - alias :update_multiple_dependencies? :update_multiple_dependencies + alias update_multiple_dependencies? update_multiple_dependencies def new_dependencies_to_unlock_from(error:, already_unlocked:) potentials_deps = @@ -120,7 +120,7 @@ def build_definition(other_updates:) # If the dependency is not found in the Gemfile it means this is a # transitive dependency that we can't force update. - raise TransitiveDependencyError unless dep + raise TransitiveDependencyError, locked_by_parent_msg unless dep dep.instance_variable_set(:@requirement, new_req) dep.source = nil if dep.source.is_a?(Bundler::Source::Git) @@ -153,6 +153,25 @@ def subdependencies all_deps - top_level end + def locked_by_parent_msg + version = Gem::Version.new(target_version) + specs = Bundler::LockfileParser.new(lockfile).specs.filter do |spec| + spec.dependencies.any? do |sub_dep| + sub_dep.name == dependency_name && + !sub_dep.requirement.satisfied_by?(version) + end + end + + locking = specs.map do |spec| + req = spec.dependencies.find { |bd| bd.name == dependency_name } + "#{spec.name} (#{spec.version}) which requires #{dependency_name} "\ + "#{req.requirement}" + end + + "#{dependency_name} cannot be updated to #{target_version} because it "\ + "is locked by #{locking.join(', ')}" + end + def unlock_gem(definition:, gem_name:) dep = definition.dependencies.find { |d| d.name == gem_name } version = definition.locked_gems.specs. diff --git a/bundler/lib/dependabot/bundler/update_checker.rb b/bundler/lib/dependabot/bundler/update_checker.rb index a925abc1c2..80cb6be2fd 100644 --- a/bundler/lib/dependabot/bundler/update_checker.rb +++ b/bundler/lib/dependabot/bundler/update_checker.rb @@ -107,6 +107,8 @@ def requirements_update_strategy dependency.version.nil? ? :bump_versions_if_necessary : :bump_versions end + attr_reader :blocked_by_parent_msg + private def latest_version_resolvable_with_full_unlock? @@ -157,7 +159,10 @@ def resolvable?(version) update_multiple_dependencies: false ).updated_dependencies true - rescue Dependabot::DependencyFileNotResolvable + rescue Dependabot::DependencyFileNotResolvable => e + if e.error_class.end_with?("TransitiveDependencyError") + @blocked_by_parent_msg = e.msg + end false end end diff --git a/bundler/spec/dependabot/bundler/update_checker_spec.rb b/bundler/spec/dependabot/bundler/update_checker_spec.rb index 4df583f69f..41db9857bd 100644 --- a/bundler/spec/dependabot/bundler/update_checker_spec.rb +++ b/bundler/spec/dependabot/bundler/update_checker_spec.rb @@ -473,6 +473,15 @@ end it { is_expected.to be_truthy } + + it "explains why the update was not possible" do + subject + expect(checker.blocked_by_parent_msg).to eq( + "dummy-pkg-a cannot be "\ + "updated to 2.0.0 because it is locked by dummy-pkg-b (1.0.0) which "\ + "requires dummy-pkg-a < 2.0.0" + ) + end end end end From 5eb95716464891a478a641eeacba870d9ae4638e Mon Sep 17 00:00:00 2001 From: jurre Date: Tue, 3 Nov 2020 16:42:53 +0100 Subject: [PATCH 03/13] Extract new native helper to detect blocking parent --- bundler/helpers/lib/functions.rb | 31 ++++++--- .../helpers/lib/functions/force_updater.rb | 21 +----- .../functions/parent_dependency_resolver.rb | 53 +++++++++++++++ .../lib/dependabot/bundler/update_checker.rb | 17 +++-- .../parent_dependency_resolver.rb | 64 +++++++++++++++++++ .../dependabot/bundler/update_checker_spec.rb | 56 +++++++++++++--- .../parent_dependency_resolver_spec.rb | 44 +++++++++++++ common/lib/dependabot/update_checkers/base.rb | 4 ++ 8 files changed, 247 insertions(+), 43 deletions(-) create mode 100644 bundler/helpers/lib/functions/parent_dependency_resolver.rb create mode 100644 bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb create mode 100644 bundler/spec/native_helpers/functions/parent_dependency_resolver_spec.rb diff --git a/bundler/helpers/lib/functions.rb b/bundler/helpers/lib/functions.rb index 4475148438..0e656fcd2a 100644 --- a/bundler/helpers/lib/functions.rb +++ b/bundler/helpers/lib/functions.rb @@ -3,6 +3,7 @@ require "functions/lockfile_updater" require "functions/dependency_source" require "functions/version_resolver" +require "functions/parent_dependency_resolver" module Functions def self.parsed_gemfile(lockfile_name:, gemfile_name:, dir:) @@ -32,7 +33,7 @@ def self.update_lockfile(dir:, gemfile_name:, lockfile_name:, using_bundler_2:, LockfileUpdater.new( gemfile_name: gemfile_name, lockfile_name: lockfile_name, - dependencies: dependencies, + dependencies: dependencies ).run end @@ -46,7 +47,7 @@ def self.force_update(dir:, dependency_name:, target_version:, gemfile_name:, target_version: target_version, gemfile_name: gemfile_name, lockfile_name: lockfile_name, - update_multiple_dependencies: update_multiple_dependencies, + update_multiple_dependencies: update_multiple_dependencies ).run end @@ -57,7 +58,7 @@ def self.dependency_source_type(gemfile_name:, dependency_name:, dir:, DependencySource.new( gemfile_name: gemfile_name, - dependency_name: dependency_name, + dependency_name: dependency_name ).type end @@ -69,7 +70,7 @@ def self.depencency_source_latest_git_version(gemfile_name:, dependency_name:, using_bundler_2: false) DependencySource.new( gemfile_name: gemfile_name, - dependency_name: dependency_name, + dependency_name: dependency_name ).latest_git_version( dependency_source_url: dependency_source_url, dependency_source_branch: dependency_source_branch @@ -83,7 +84,7 @@ def self.private_registry_versions(gemfile_name:, dependency_name:, dir:, DependencySource.new( gemfile_name: gemfile_name, - dependency_name: dependency_name, + dependency_name: dependency_name ).private_registry_versions end @@ -96,7 +97,7 @@ def self.resolve_version(dependency_name:, dependency_requirements:, dependency_name: dependency_name, dependency_requirements: dependency_requirements, gemfile_name: gemfile_name, - lockfile_name: lockfile_name, + lockfile_name: lockfile_name ).version_details end @@ -117,9 +118,9 @@ def self.git_specs(dir:, gemfile_name:, credentials:, using_bundler_2:) using_bundler_2: using_bundler_2) git_specs = Bundler::Definition.build(gemfile_name, nil, {}).dependencies. - select do |spec| - spec.source.is_a?(Bundler::Source::Git) - end + select do |spec| + spec.source.is_a?(Bundler::Source::Git) + end git_specs.map do |spec| # Piggy-back off some private Bundler methods to configure the # URI with auth details in the same way Bundler does. @@ -161,4 +162,16 @@ def self.set_bundler_flags_and_credentials(dir:, credentials:, Bundler.settings.set_command_option("github.https", "true") end end + + def self.blocking_parent_dependencies(dir:, dependency_name:, target_version:, + lockfile_name:, using_bundler_2:, + credentials:) + set_bundler_flags_and_credentials(dir: dir, credentials: credentials, + using_bundler_2: using_bundler_2) + ParentDependencyResolver.new( + dependency_name: dependency_name, + target_version: target_version, + lockfile_name: lockfile_name + ).blocking_parent_dependencies + end end diff --git a/bundler/helpers/lib/functions/force_updater.rb b/bundler/helpers/lib/functions/force_updater.rb index 90e75efc33..5a726cd6a4 100644 --- a/bundler/helpers/lib/functions/force_updater.rb +++ b/bundler/helpers/lib/functions/force_updater.rb @@ -120,7 +120,7 @@ def build_definition(other_updates:) # If the dependency is not found in the Gemfile it means this is a # transitive dependency that we can't force update. - raise TransitiveDependencyError, locked_by_parent_msg unless dep + raise TransitiveDependencyError unless dep dep.instance_variable_set(:@requirement, new_req) dep.source = nil if dep.source.is_a?(Bundler::Source::Git) @@ -153,25 +153,6 @@ def subdependencies all_deps - top_level end - def locked_by_parent_msg - version = Gem::Version.new(target_version) - specs = Bundler::LockfileParser.new(lockfile).specs.filter do |spec| - spec.dependencies.any? do |sub_dep| - sub_dep.name == dependency_name && - !sub_dep.requirement.satisfied_by?(version) - end - end - - locking = specs.map do |spec| - req = spec.dependencies.find { |bd| bd.name == dependency_name } - "#{spec.name} (#{spec.version}) which requires #{dependency_name} "\ - "#{req.requirement}" - end - - "#{dependency_name} cannot be updated to #{target_version} because it "\ - "is locked by #{locking.join(', ')}" - end - def unlock_gem(definition:, gem_name:) dep = definition.dependencies.find { |d| d.name == gem_name } version = definition.locked_gems.specs. diff --git a/bundler/helpers/lib/functions/parent_dependency_resolver.rb b/bundler/helpers/lib/functions/parent_dependency_resolver.rb new file mode 100644 index 0000000000..37626cfb30 --- /dev/null +++ b/bundler/helpers/lib/functions/parent_dependency_resolver.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module Functions + class ParentDependencyResolver + def initialize(dependency_name:, target_version:, lockfile_name:) + @dependency_name = dependency_name + @target_version = target_version + @lockfile_name = lockfile_name + + Bundler.settings.set_command_option("only_update_to_newer_versions", true) + end + + # @return [Array String}] + # :name the blocking dependencies name + # :version the version of the blocking dependency + # :requirement the requirement on the target_dependency + def blocking_parent_dependencies + parent_specs.map do |spec| + req = spec.dependencies.find { |bd| bd.name == dependency_name } + { + name: spec.name, + version: spec.version.to_s, + requirement: req.requirement.to_s + } + end + end + + private + + attr_reader :dependency_name, :target_version, :lockfile_name + + def parent_specs + version = Gem::Version.new(target_version) + Bundler::LockfileParser.new(lockfile).specs.filter do |spec| + spec.dependencies.any? do |sub_dep| + sub_dep.name == dependency_name && + !sub_dep.requirement.satisfied_by?(version) + end + end + end + + def lockfile + return @lockfile if defined?(@lockfile) + + @lockfile = + begin + return unless lockfile_name && File.exist?(lockfile_name) + + File.read(lockfile_name) + end + end + end +end diff --git a/bundler/lib/dependabot/bundler/update_checker.rb b/bundler/lib/dependabot/bundler/update_checker.rb index 80cb6be2fd..47bac396d4 100644 --- a/bundler/lib/dependabot/bundler/update_checker.rb +++ b/bundler/lib/dependabot/bundler/update_checker.rb @@ -13,6 +13,7 @@ class UpdateChecker < Dependabot::UpdateCheckers::Base require_relative "update_checker/requirements_updater" require_relative "update_checker/version_resolver" require_relative "update_checker/latest_version_finder" + require_relative "update_checker/parent_dependency_resolver" def latest_version return latest_version_for_git_dependency if git_dependency? @@ -107,7 +108,16 @@ def requirements_update_strategy dependency.version.nil? ? :bump_versions_if_necessary : :bump_versions end - attr_reader :blocked_by_parent_msg + def blocking_parent_dependencies + ParentDependencyResolver.new( + dependency_files: dependency_files, + repo_contents_path: repo_contents_path, + credentials: credentials + ).blocking_parent_dependencies( + dependency: dependency, + target_version: lowest_security_fix_version + ) + end private @@ -159,10 +169,7 @@ def resolvable?(version) update_multiple_dependencies: false ).updated_dependencies true - rescue Dependabot::DependencyFileNotResolvable => e - if e.error_class.end_with?("TransitiveDependencyError") - @blocked_by_parent_msg = e.msg - end + rescue Dependabot::DependencyFileNotResolvable false end end diff --git a/bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb b/bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb new file mode 100644 index 0000000000..dc60aa82a9 --- /dev/null +++ b/bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require "dependabot/bundler/native_helpers" +require "dependabot/shared_helpers" + +module Dependabot + module Bundler + class UpdateChecker + class ParentDependencyResolver + require_relative "shared_bundler_helpers" + include SharedBundlerHelpers + + def initialize(dependency_files:, repo_contents_path:, credentials:) + @dependency_files = dependency_files + @repo_contents_path = repo_contents_path + @credentials = credentials + end + + def blocking_parent_dependencies(dependency:, target_version:) + in_a_native_bundler_context(error_handling: false) do |tmp_dir| + SharedHelpers.run_helper_subprocess( + command: NativeHelpers.helper_path, + function: "blocking_parent_dependencies", + args: { + dir: tmp_dir, + dependency_name: dependency.name, + target_version: target_version, + credentials: relevant_credentials, + lockfile_name: lockfile.name, + using_bundler_2: using_bundler_2? + } + ) + end + end + + private + + attr_reader :dependency_files, :repo_contents_path, :credentials + + def lockfile + (dependency_files.find { |f| f.name == "Gemfile.lock" } || + dependency_files.find { |f| f.name == "gems.locked" }) + end + + def relevant_credentials + credentials. + select { |cred| cred["password"] || cred["token"] }. + select do |cred| + next true if cred["type"] == "git_source" + next true if cred["type"] == "rubygems_server" + + false + end + end + + def using_bundler_2? + return unless lockfile + + lockfile.content.match?(/BUNDLED WITH\s+2/m) + end + end + end + end +end diff --git a/bundler/spec/dependabot/bundler/update_checker_spec.rb b/bundler/spec/dependabot/bundler/update_checker_spec.rb index 41db9857bd..2c3430d600 100644 --- a/bundler/spec/dependabot/bundler/update_checker_spec.rb +++ b/bundler/spec/dependabot/bundler/update_checker_spec.rb @@ -473,15 +473,6 @@ end it { is_expected.to be_truthy } - - it "explains why the update was not possible" do - subject - expect(checker.blocked_by_parent_msg).to eq( - "dummy-pkg-a cannot be "\ - "updated to 2.0.0 because it is locked by dummy-pkg-b (1.0.0) which "\ - "requires dummy-pkg-a < 2.0.0" - ) - end end end end @@ -602,6 +593,53 @@ end end + describe "#blocking_parent_dependencies" do + include_context "stub rubygems compact index" + include_context "stub rubygems versions api" + + subject { checker.blocking_parent_dependencies } + + let(:gemfile_fixture_name) { "subdep_blocked_by_subdep" } + let(:lockfile_fixture_name) { "subdep_blocked_by_subdep.lock" } + let(:target_version) { "2.0.0" } + let(:dependency_name) { "dummy-pkg-a" } + let(:requirements) do + [{ + file: "Gemfile", + requirement: "~> 1.0.0", + groups: [], + source: nil + }] + end + + let(:requirements) { [] } + let(:security_advisories) do + [ + Dependabot::SecurityAdvisory.new( + dependency_name: dependency_name, + package_manager: "bundler", + vulnerable_versions: ["< 2.0.0"] + ) + ] + end + + before do + allow(checker). + to receive(:lowest_security_fix_version). + and_return(target_version) + end + + it do + is_expected.to eq( + [{ + "name" => "dummy-pkg-b", + "version" => "1.0.0", + "requirement" => "< 2.0.0" + }] + ) + end + end + describe "#latest_resolvable_version" do include_context "stub rubygems compact index" include_context "stub rubygems versions api" diff --git a/bundler/spec/native_helpers/functions/parent_dependency_resolver_spec.rb b/bundler/spec/native_helpers/functions/parent_dependency_resolver_spec.rb new file mode 100644 index 0000000000..0c049a3b4f --- /dev/null +++ b/bundler/spec/native_helpers/functions/parent_dependency_resolver_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require "native_spec_helper" +require "shared_contexts" + +RSpec.describe Functions::ParentDependencyResolver do + include_context "in a temporary bundler directory" + + let(:parent_dependency_resolver) do + described_class.new( + dependency_name: dependency_name, + target_version: target_version, + lockfile_name: lockfile_name + ) + 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" } + + describe "#blocking_parent_dependencies" do + subject(:blocking_parent_dependencies) do + in_tmp_folder { parent_dependency_resolver.blocking_parent_dependencies } + end + + it "returns a list of dependencies that block the update" do + expect(blocking_parent_dependencies).to eq( + [ + { name: "dummy-pkg-b", version: "1.0.0", requirement: "< 2.0.0" } + ] + ) + end + + context "without any blocking dependencies" do + let(:target_version) { "1.0.0" } + + it "returns an empty list" do + expect(blocking_parent_dependencies).to eq([]) + end + end + end +end diff --git a/common/lib/dependabot/update_checkers/base.rb b/common/lib/dependabot/update_checkers/base.rb index c1130a1050..6cf9d66755 100644 --- a/common/lib/dependabot/update_checkers/base.rb +++ b/common/lib/dependabot/update_checkers/base.rb @@ -93,6 +93,10 @@ def latest_resolvable_version_with_no_unlock raise NotImplementedError end + def blocking_parent_dependencies + raise NotImplementedError + end + def latest_resolvable_previous_version(_updated_version) dependency.version end From d048df2e6327bc3c290803b6b7fca8969fe14f03 Mon Sep 17 00:00:00 2001 From: jurre Date: Wed, 4 Nov 2020 14:22:55 +0100 Subject: [PATCH 04/13] Output blocking parents in dry-run --- bin/dry-run.rb | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/bin/dry-run.rb b/bin/dry-run.rb index 69f9f82d75..b44f494003 100755 --- a/bin/dry-run.rb +++ b/bin/dry-run.rb @@ -102,7 +102,7 @@ commit: nil, updater_options: {}, security_advisories: [], - security_updates_only: false, + security_updates_only: false } unless ENV["LOCAL_GITHUB_ACCESS_TOKEN"].to_s.strip.empty? @@ -424,7 +424,7 @@ def file_updater_for(dependencies) dependency_files: $files, repo_contents_path: $repo_contents_path, credentials: $options[:credentials], - options: $options[:updater_options], + options: $options[:updater_options] ) end @@ -482,11 +482,23 @@ def security_fix?(dependency) end end - latest_allowed_version = checker.vulnerable? ? - checker.lowest_resolvable_security_fix_version : - checker.latest_resolvable_version + latest_allowed_version = if checker.vulnerable? + checker.lowest_resolvable_security_fix_version + else + checker.latest_resolvable_version + end puts " => latest allowed version is #{latest_allowed_version || dep.version}" + parents = checker.blocking_parent_dependencies + if parents.any? + puts " => The update is not possible because of the following conflicting "\ + "dependencies:" + end + parents.each do |parent| + puts " #{parent['name']} (#{parent['version']}) which requires:" + puts " #{dep.name} #{parent['requirement']}" + end + if checker.up_to_date? puts " (no update needed as it's already up-to-date)" next From f2926d1404c96659d999671a21430109ac88bb48 Mon Sep 17 00:00:00 2001 From: jurre Date: Wed, 4 Nov 2020 14:23:04 +0100 Subject: [PATCH 05/13] Clean up Bundler parent dependency resolving and improve test coverage --- .../functions/parent_dependency_resolver.rb | 16 +-- .../parent_dependency_resolver.rb | 38 ++---- .../parent_dependency_resolver_spec.rb | 109 ++++++++++++++++++ .../fixtures/ruby/gemfiles/multiple_blocking | 5 + .../ruby/lockfiles/multiple_blocking.lock | 70 +++++++++++ common/lib/dependabot/update_checkers/base.rb | 8 +- 6 files changed, 211 insertions(+), 35 deletions(-) create mode 100644 bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb create mode 100644 bundler/spec/fixtures/ruby/gemfiles/multiple_blocking create mode 100644 bundler/spec/fixtures/ruby/lockfiles/multiple_blocking.lock diff --git a/bundler/helpers/lib/functions/parent_dependency_resolver.rb b/bundler/helpers/lib/functions/parent_dependency_resolver.rb index 37626cfb30..9f16c960bc 100644 --- a/bundler/helpers/lib/functions/parent_dependency_resolver.rb +++ b/bundler/helpers/lib/functions/parent_dependency_resolver.rb @@ -10,17 +10,19 @@ def initialize(dependency_name:, target_version:, lockfile_name:) Bundler.settings.set_command_option("only_update_to_newer_versions", true) end - # @return [Array String}] - # :name the blocking dependencies name - # :version the version of the blocking dependency - # :requirement the requirement on the target_dependency + # Finds any dependencies in the lockfile that have a subdependency on the + # given dependency that does not satisfly the target_version. + # @return [Array String}] + # * name [String] the blocking dependencies name + # * version [String] the version of the blocking dependency + # * requirement [String] the requirement on the target_dependency def blocking_parent_dependencies parent_specs.map do |spec| req = spec.dependencies.find { |bd| bd.name == dependency_name } { - name: spec.name, - version: spec.version.to_s, - requirement: req.requirement.to_s + "name" => spec.name, + "version" => spec.version.to_s, + "requirement" => req.requirement.to_s } end end diff --git a/bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb b/bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb index dc60aa82a9..8bcb0fc270 100644 --- a/bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb +++ b/bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true +require "dependabot/bundler/update_checker" require "dependabot/bundler/native_helpers" require "dependabot/shared_helpers" module Dependabot module Bundler - class UpdateChecker + class UpdateChecker < UpdateCheckers::Base class ParentDependencyResolver require_relative "shared_bundler_helpers" include SharedBundlerHelpers @@ -16,6 +17,15 @@ def initialize(dependency_files:, repo_contents_path:, credentials:) @credentials = credentials end + # Finds any dependencies in the lockfile that have a subdependency on + # the given dependency that does not satisfly the target_version. + # + # @param dependency [Dependabot::Dependency] the dependency to check + # @param target_version [String] the version to check + # @return [Array String}] + # * name [String] the blocking dependencies name + # * version [String] the version of the blocking dependency + # * requirement [String] the requirement on the target_dependency def blocking_parent_dependencies(dependency:, target_version:) in_a_native_bundler_context(error_handling: false) do |tmp_dir| SharedHelpers.run_helper_subprocess( @@ -32,32 +42,6 @@ def blocking_parent_dependencies(dependency:, target_version:) ) end end - - private - - attr_reader :dependency_files, :repo_contents_path, :credentials - - def lockfile - (dependency_files.find { |f| f.name == "Gemfile.lock" } || - dependency_files.find { |f| f.name == "gems.locked" }) - end - - def relevant_credentials - credentials. - select { |cred| cred["password"] || cred["token"] }. - select do |cred| - next true if cred["type"] == "git_source" - next true if cred["type"] == "rubygems_server" - - false - end - end - - def using_bundler_2? - return unless lockfile - - lockfile.content.match?(/BUNDLED WITH\s+2/m) - end end end end diff --git a/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb b/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb new file mode 100644 index 0000000000..eacc9dd4df --- /dev/null +++ b/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +require "spec_helper" +require "shared_contexts" +require "dependabot/dependency" +require "dependabot/dependency_file" +require "dependabot/bundler/update_checker/parent_dependency_resolver" + +RSpec.describe Dependabot::Bundler::UpdateChecker::ParentDependencyResolver do + include_context "stub rubygems compact index" + + let(:resolver) do + described_class.new( + dependency_files: dependency_files, + repo_contents_path: repo_contents_path, + credentials: [{ + "type" => "git_source", + "host" => "github.com", + "username" => "x-access-token", + "password" => "token" + }] + ) + end + let(:dependency_files) { [gemfile, lockfile] } + let(:repo_contents_path) { nil } + + let(:dependency) do + Dependabot::Dependency.new( + name: dependency_name, + version: current_version, + requirements: [], + package_manager: "bundler" + ) + end + let(:dependency_name) { "dummy-pkg-a" } + let(:current_version) { "1.0.1" } + let(:target_version) { "2.0.0" } + + let(:gemfile) do + Dependabot::DependencyFile.new(content: gemfile_body, name: "Gemfile") + end + let(:lockfile) do + Dependabot::DependencyFile.new(content: lockfile_body, name: "Gemfile.lock") + end + let(:gemfile_body) { fixture("ruby", "gemfiles", "subdep_blocked_by_subdep") } + let(:lockfile_body) do + fixture("ruby", "lockfiles", "subdep_blocked_by_subdep.lock") + end + + describe "#blocking_parent_dependencies" do + subject(:blocking_parent_dependencies) do + resolver.blocking_parent_dependencies( + dependency: dependency, + target_version: target_version + ) + end + + it "returns the right array of updated dependencies" do + expect(blocking_parent_dependencies).to match_array( + [ + { + "name" => "dummy-pkg-b", + "version" => "1.0.0", + "requirement" => "< 2.0.0" + } + ] + ) + end + + context "with no blocking dependencies" do + let(:target_version) { "1.5.0" } + it "returns an empty array" do + expect(blocking_parent_dependencies).to match_array([]) + end + end + + context "with multiple blocking dependencies" do + let(:dependency_name) { "activesupport" } + let(:current_version) { "5.0.0" } + let(:target_version) { "6.0.0" } + let(:gemfile_body) { fixture("ruby", "gemfiles", "multiple_blocking") } + let(:lockfile_body) do + fixture("ruby", "lockfiles", "multiple_blocking.lock") + end + + it "returns all of the blocking dependencies" do + expect(blocking_parent_dependencies).to match_array( + [ + { + "name" => "actionpack", + "version" => "5.0.0", + "requirement" => "= 5.0.0" + }, + { + "name" => "actionview", + "version" => "5.0.0", + "requirement" => "= 5.0.0" + }, + { + "name" => "activejob", + "version" => "5.0.0", + "requirement" => "= 5.0.0" + } + ] + ) + end + end + end +end diff --git a/bundler/spec/fixtures/ruby/gemfiles/multiple_blocking b/bundler/spec/fixtures/ruby/gemfiles/multiple_blocking new file mode 100644 index 0000000000..eb7111e1ad --- /dev/null +++ b/bundler/spec/fixtures/ruby/gemfiles/multiple_blocking @@ -0,0 +1,5 @@ +source "https://rubygems.org" + +gem "activesupport", "5.0.0" +gem "actionview", "5.0.0" +gem "actionmailer", "5.0.0" diff --git a/bundler/spec/fixtures/ruby/lockfiles/multiple_blocking.lock b/bundler/spec/fixtures/ruby/lockfiles/multiple_blocking.lock new file mode 100644 index 0000000000..2555bab90b --- /dev/null +++ b/bundler/spec/fixtures/ruby/lockfiles/multiple_blocking.lock @@ -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 diff --git a/common/lib/dependabot/update_checkers/base.rb b/common/lib/dependabot/update_checkers/base.rb index 6cf9d66755..b73ca2c3fc 100644 --- a/common/lib/dependabot/update_checkers/base.rb +++ b/common/lib/dependabot/update_checkers/base.rb @@ -93,8 +93,14 @@ def latest_resolvable_version_with_no_unlock raise NotImplementedError end + # Finds any dependencies in the lockfile that have a subdependency on the + # given dependency that do not satisfly the target_version. + # @return [Array String}] + # name [String] the blocking dependencies name + # version [String] the version of the blocking dependency + # requirement [String] the requirement on the target_dependency def blocking_parent_dependencies - raise NotImplementedError + [] # return an empty array for ecosystems that don't support this yet end def latest_resolvable_previous_version(_updated_version) From 31bb936f0de979ab057bfecd93198207ee952b74 Mon Sep 17 00:00:00 2001 From: Jurre Date: Wed, 4 Nov 2020 15:38:29 +0100 Subject: [PATCH 06/13] Fix typo in common/lib/dependabot/update_checkers/base.rb Co-authored-by: Pete Wagner --- common/lib/dependabot/update_checkers/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/lib/dependabot/update_checkers/base.rb b/common/lib/dependabot/update_checkers/base.rb index b73ca2c3fc..1491decc3c 100644 --- a/common/lib/dependabot/update_checkers/base.rb +++ b/common/lib/dependabot/update_checkers/base.rb @@ -94,7 +94,7 @@ def latest_resolvable_version_with_no_unlock end # Finds any dependencies in the lockfile that have a subdependency on the - # given dependency that do not satisfly the target_version. + # given dependency that do not satisfy the target_version. # @return [Array String}] # name [String] the blocking dependencies name # version [String] the version of the blocking dependency From f808848d4a4ec0781af66d475da0fdede11e74bd Mon Sep 17 00:00:00 2001 From: Jurre Date: Wed, 4 Nov 2020 15:38:45 +0100 Subject: [PATCH 07/13] Fix typo in bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb Co-authored-by: Pete Wagner --- .../bundler/update_checker/parent_dependency_resolver_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb b/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb index eacc9dd4df..e0c35edb65 100644 --- a/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb +++ b/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb @@ -55,7 +55,7 @@ ) end - it "returns the right array of updated dependencies" do + it "returns the right array of blocking dependencies" do expect(blocking_parent_dependencies).to match_array( [ { From e93d7b56c50895b19923a7820e044d5cf1873af1 Mon Sep 17 00:00:00 2001 From: jurre Date: Wed, 4 Nov 2020 15:35:39 +0100 Subject: [PATCH 08/13] Don't modify static state from constructor --- bundler/helpers/lib/functions/parent_dependency_resolver.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bundler/helpers/lib/functions/parent_dependency_resolver.rb b/bundler/helpers/lib/functions/parent_dependency_resolver.rb index 9f16c960bc..c1978cb597 100644 --- a/bundler/helpers/lib/functions/parent_dependency_resolver.rb +++ b/bundler/helpers/lib/functions/parent_dependency_resolver.rb @@ -6,8 +6,6 @@ def initialize(dependency_name:, target_version:, lockfile_name:) @dependency_name = dependency_name @target_version = target_version @lockfile_name = lockfile_name - - Bundler.settings.set_command_option("only_update_to_newer_versions", true) end # Finds any dependencies in the lockfile that have a subdependency on the @@ -17,6 +15,8 @@ def initialize(dependency_name:, target_version:, lockfile_name:) # * version [String] the version of the blocking dependency # * requirement [String] the requirement on the target_dependency def blocking_parent_dependencies + Bundler.settings.set_command_option("only_update_to_newer_versions", true) + parent_specs.map do |spec| req = spec.dependencies.find { |bd| bd.name == dependency_name } { From 7735d847c7ce95c78d62525d5e954153426562a2 Mon Sep 17 00:00:00 2001 From: jurre Date: Wed, 4 Nov 2020 15:36:22 +0100 Subject: [PATCH 09/13] Move local variable closer to where it's used --- bundler/helpers/lib/functions/force_updater.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bundler/helpers/lib/functions/force_updater.rb b/bundler/helpers/lib/functions/force_updater.rb index 5a726cd6a4..dad2343ac0 100644 --- a/bundler/helpers/lib/functions/force_updater.rb +++ b/bundler/helpers/lib/functions/force_updater.rb @@ -113,8 +113,6 @@ def build_definition(other_updates:) unlock_gem(definition: definition, gem_name: gem_name) end - # Set the requirement for the gem we're forcing an update of - new_req = Gem::Requirement.create("= #{target_version}") dep = definition.dependencies. find { |d| d.name == dependency_name } @@ -122,6 +120,8 @@ def build_definition(other_updates:) # transitive dependency that we can't force update. raise TransitiveDependencyError unless dep + # Set the requirement for the gem we're forcing an update of + new_req = Gem::Requirement.create("= #{target_version}") dep.instance_variable_set(:@requirement, new_req) dep.source = nil if dep.source.is_a?(Bundler::Source::Git) From 0b3aafcba8ecac74a7d45e81fc90ef19fd037ad8 Mon Sep 17 00:00:00 2001 From: jurre Date: Wed, 4 Nov 2020 15:39:39 +0100 Subject: [PATCH 10/13] Remove unused let --- .../bundler/update_checker/parent_dependency_resolver_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb b/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb index e0c35edb65..feade3e776 100644 --- a/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb +++ b/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb @@ -12,7 +12,7 @@ let(:resolver) do described_class.new( dependency_files: dependency_files, - repo_contents_path: repo_contents_path, + repo_contents_path: nil, credentials: [{ "type" => "git_source", "host" => "github.com", @@ -22,7 +22,6 @@ ) end let(:dependency_files) { [gemfile, lockfile] } - let(:repo_contents_path) { nil } let(:dependency) do Dependabot::Dependency.new( From 3197f2f36eb56e993fe1777683ed2fdf5d61f2a8 Mon Sep 17 00:00:00 2001 From: jurre Date: Wed, 4 Nov 2020 15:42:03 +0100 Subject: [PATCH 11/13] Rename blocking_parent_dependencies to conflicting_dependencies --- bin/dry-run.rb | 2 +- bundler/helpers/lib/functions.rb | 11 +++++------ ...r.rb => conflicting_dependency_resolver.rb} | 4 ++-- .../lib/dependabot/bundler/update_checker.rb | 8 ++++---- ...r.rb => conflicting_dependency_resolver.rb} | 6 +++--- .../parent_dependency_resolver_spec.rb | 18 ++++++++++-------- .../dependabot/bundler/update_checker_spec.rb | 4 ++-- ...=> conflicting_dependency_resolver_spec.rb} | 14 +++++++------- common/lib/dependabot/update_checkers/base.rb | 2 +- 9 files changed, 35 insertions(+), 34 deletions(-) rename bundler/helpers/lib/functions/{parent_dependency_resolver.rb => conflicting_dependency_resolver.rb} (95%) rename bundler/lib/dependabot/bundler/update_checker/{parent_dependency_resolver.rb => conflicting_dependency_resolver.rb} (91%) rename bundler/spec/native_helpers/functions/{parent_dependency_resolver_spec.rb => conflicting_dependency_resolver_spec.rb} (69%) diff --git a/bin/dry-run.rb b/bin/dry-run.rb index b44f494003..e31b2fb18e 100755 --- a/bin/dry-run.rb +++ b/bin/dry-run.rb @@ -489,7 +489,7 @@ def security_fix?(dependency) end puts " => latest allowed version is #{latest_allowed_version || dep.version}" - parents = checker.blocking_parent_dependencies + parents = checker.conflicting_dependencies if parents.any? puts " => The update is not possible because of the following conflicting "\ "dependencies:" diff --git a/bundler/helpers/lib/functions.rb b/bundler/helpers/lib/functions.rb index 0e656fcd2a..f1e199639f 100644 --- a/bundler/helpers/lib/functions.rb +++ b/bundler/helpers/lib/functions.rb @@ -3,7 +3,7 @@ require "functions/lockfile_updater" require "functions/dependency_source" require "functions/version_resolver" -require "functions/parent_dependency_resolver" +require "functions/conflicting_dependency_resolver" module Functions def self.parsed_gemfile(lockfile_name:, gemfile_name:, dir:) @@ -163,15 +163,14 @@ def self.set_bundler_flags_and_credentials(dir:, credentials:, end end - def self.blocking_parent_dependencies(dir:, dependency_name:, target_version:, - lockfile_name:, using_bundler_2:, - credentials:) + def self.conflicting_dependencies(dir:, dependency_name:, target_version:, + lockfile_name:, using_bundler_2:, credentials:) set_bundler_flags_and_credentials(dir: dir, credentials: credentials, using_bundler_2: using_bundler_2) - ParentDependencyResolver.new( + ConflictingDependencyResolver.new( dependency_name: dependency_name, target_version: target_version, lockfile_name: lockfile_name - ).blocking_parent_dependencies + ).conflicting_dependencies end end diff --git a/bundler/helpers/lib/functions/parent_dependency_resolver.rb b/bundler/helpers/lib/functions/conflicting_dependency_resolver.rb similarity index 95% rename from bundler/helpers/lib/functions/parent_dependency_resolver.rb rename to bundler/helpers/lib/functions/conflicting_dependency_resolver.rb index c1978cb597..7085a30947 100644 --- a/bundler/helpers/lib/functions/parent_dependency_resolver.rb +++ b/bundler/helpers/lib/functions/conflicting_dependency_resolver.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Functions - class ParentDependencyResolver + class ConflictingDependencyResolver def initialize(dependency_name:, target_version:, lockfile_name:) @dependency_name = dependency_name @target_version = target_version @@ -14,7 +14,7 @@ def initialize(dependency_name:, target_version:, lockfile_name:) # * name [String] the blocking dependencies name # * version [String] the version of the blocking dependency # * requirement [String] the requirement on the target_dependency - def blocking_parent_dependencies + def conflicting_dependencies Bundler.settings.set_command_option("only_update_to_newer_versions", true) parent_specs.map do |spec| diff --git a/bundler/lib/dependabot/bundler/update_checker.rb b/bundler/lib/dependabot/bundler/update_checker.rb index 47bac396d4..fe6ba35a2d 100644 --- a/bundler/lib/dependabot/bundler/update_checker.rb +++ b/bundler/lib/dependabot/bundler/update_checker.rb @@ -13,7 +13,7 @@ class UpdateChecker < Dependabot::UpdateCheckers::Base require_relative "update_checker/requirements_updater" require_relative "update_checker/version_resolver" require_relative "update_checker/latest_version_finder" - require_relative "update_checker/parent_dependency_resolver" + require_relative "update_checker/conflicting_dependency_resolver" def latest_version return latest_version_for_git_dependency if git_dependency? @@ -108,12 +108,12 @@ def requirements_update_strategy dependency.version.nil? ? :bump_versions_if_necessary : :bump_versions end - def blocking_parent_dependencies - ParentDependencyResolver.new( + def conflicting_dependencies + ConflictingDependencyResolver.new( dependency_files: dependency_files, repo_contents_path: repo_contents_path, credentials: credentials - ).blocking_parent_dependencies( + ).conflicting_dependencies( dependency: dependency, target_version: lowest_security_fix_version ) diff --git a/bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb b/bundler/lib/dependabot/bundler/update_checker/conflicting_dependency_resolver.rb similarity index 91% rename from bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb rename to bundler/lib/dependabot/bundler/update_checker/conflicting_dependency_resolver.rb index 8bcb0fc270..9b23b894e2 100644 --- a/bundler/lib/dependabot/bundler/update_checker/parent_dependency_resolver.rb +++ b/bundler/lib/dependabot/bundler/update_checker/conflicting_dependency_resolver.rb @@ -7,7 +7,7 @@ module Dependabot module Bundler class UpdateChecker < UpdateCheckers::Base - class ParentDependencyResolver + class ConflictingDependencyResolver require_relative "shared_bundler_helpers" include SharedBundlerHelpers @@ -26,11 +26,11 @@ def initialize(dependency_files:, repo_contents_path:, credentials:) # * name [String] the blocking dependencies name # * version [String] the version of the blocking dependency # * requirement [String] the requirement on the target_dependency - def blocking_parent_dependencies(dependency:, target_version:) + 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, - function: "blocking_parent_dependencies", + function: "conflicting_dependencies", args: { dir: tmp_dir, dependency_name: dependency.name, diff --git a/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb b/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb index feade3e776..a5e4b99bdf 100644 --- a/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb +++ b/bundler/spec/dependabot/bundler/update_checker/parent_dependency_resolver_spec.rb @@ -4,9 +4,11 @@ require "shared_contexts" require "dependabot/dependency" require "dependabot/dependency_file" -require "dependabot/bundler/update_checker/parent_dependency_resolver" +require "dependabot/bundler/update_checker/conflicting_dependency_resolver" -RSpec.describe Dependabot::Bundler::UpdateChecker::ParentDependencyResolver do +RSpec.describe( + Dependabot::Bundler::UpdateChecker::ConflictingDependencyResolver +) do include_context "stub rubygems compact index" let(:resolver) do @@ -46,16 +48,16 @@ fixture("ruby", "lockfiles", "subdep_blocked_by_subdep.lock") end - describe "#blocking_parent_dependencies" do - subject(:blocking_parent_dependencies) do - resolver.blocking_parent_dependencies( + describe "#conflicting_dependencies" do + subject(:conflicting_dependencies) do + resolver.conflicting_dependencies( dependency: dependency, target_version: target_version ) end it "returns the right array of blocking dependencies" do - expect(blocking_parent_dependencies).to match_array( + expect(conflicting_dependencies).to match_array( [ { "name" => "dummy-pkg-b", @@ -69,7 +71,7 @@ context "with no blocking dependencies" do let(:target_version) { "1.5.0" } it "returns an empty array" do - expect(blocking_parent_dependencies).to match_array([]) + expect(conflicting_dependencies).to match_array([]) end end @@ -83,7 +85,7 @@ end it "returns all of the blocking dependencies" do - expect(blocking_parent_dependencies).to match_array( + expect(conflicting_dependencies).to match_array( [ { "name" => "actionpack", diff --git a/bundler/spec/dependabot/bundler/update_checker_spec.rb b/bundler/spec/dependabot/bundler/update_checker_spec.rb index 2c3430d600..0d829a0805 100644 --- a/bundler/spec/dependabot/bundler/update_checker_spec.rb +++ b/bundler/spec/dependabot/bundler/update_checker_spec.rb @@ -593,11 +593,11 @@ end end - describe "#blocking_parent_dependencies" do + describe "#conflicting_dependencies" do include_context "stub rubygems compact index" include_context "stub rubygems versions api" - subject { checker.blocking_parent_dependencies } + subject { checker.conflicting_dependencies } let(:gemfile_fixture_name) { "subdep_blocked_by_subdep" } let(:lockfile_fixture_name) { "subdep_blocked_by_subdep.lock" } diff --git a/bundler/spec/native_helpers/functions/parent_dependency_resolver_spec.rb b/bundler/spec/native_helpers/functions/conflicting_dependency_resolver_spec.rb similarity index 69% rename from bundler/spec/native_helpers/functions/parent_dependency_resolver_spec.rb rename to bundler/spec/native_helpers/functions/conflicting_dependency_resolver_spec.rb index 0c049a3b4f..09320b0ae2 100644 --- a/bundler/spec/native_helpers/functions/parent_dependency_resolver_spec.rb +++ b/bundler/spec/native_helpers/functions/conflicting_dependency_resolver_spec.rb @@ -3,10 +3,10 @@ require "native_spec_helper" require "shared_contexts" -RSpec.describe Functions::ParentDependencyResolver do +RSpec.describe Functions::ConflictingDependencyResolver do include_context "in a temporary bundler directory" - let(:parent_dependency_resolver) do + let(:conflicting_dependency_resolver) do described_class.new( dependency_name: dependency_name, target_version: target_version, @@ -20,13 +20,13 @@ let(:gemfile_fixture_name) { "blocked_by_subdep" } let(:lockfile_fixture_name) { "blocked_by_subdep.lock" } - describe "#blocking_parent_dependencies" do - subject(:blocking_parent_dependencies) do - in_tmp_folder { parent_dependency_resolver.blocking_parent_dependencies } + describe "#conflicting_dependencies" do + subject(:conflicting_dependencies) do + in_tmp_folder { conflicting_dependency_resolver.conflicting_dependencies } end it "returns a list of dependencies that block the update" do - expect(blocking_parent_dependencies).to eq( + expect(conflicting_dependencies).to eq( [ { name: "dummy-pkg-b", version: "1.0.0", requirement: "< 2.0.0" } ] @@ -37,7 +37,7 @@ let(:target_version) { "1.0.0" } it "returns an empty list" do - expect(blocking_parent_dependencies).to eq([]) + expect(conflicting_dependencies).to eq([]) end end end diff --git a/common/lib/dependabot/update_checkers/base.rb b/common/lib/dependabot/update_checkers/base.rb index 1491decc3c..a0d03e449f 100644 --- a/common/lib/dependabot/update_checkers/base.rb +++ b/common/lib/dependabot/update_checkers/base.rb @@ -99,7 +99,7 @@ def latest_resolvable_version_with_no_unlock # name [String] the blocking dependencies name # version [String] the version of the blocking dependency # requirement [String] the requirement on the target_dependency - def blocking_parent_dependencies + def conflicting_dependencies [] # return an empty array for ecosystems that don't support this yet end From f8a4f1fd0a4c9b6b5a13de84b2a44eb3b3a5e9f7 Mon Sep 17 00:00:00 2001 From: jurre Date: Wed, 4 Nov 2020 15:55:15 +0100 Subject: [PATCH 12/13] Improve conflicting dependencies code in dry-run script --- bin/dry-run.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/bin/dry-run.rb b/bin/dry-run.rb index e31b2fb18e..78efd310a0 100755 --- a/bin/dry-run.rb +++ b/bin/dry-run.rb @@ -489,14 +489,16 @@ def security_fix?(dependency) end puts " => latest allowed version is #{latest_allowed_version || dep.version}" - parents = checker.conflicting_dependencies - if parents.any? + conflicting_dependencies = checker.conflicting_dependencies + if conflicting_dependencies.any? puts " => The update is not possible because of the following conflicting "\ "dependencies:" - end - parents.each do |parent| - puts " #{parent['name']} (#{parent['version']}) which requires:" - puts " #{dep.name} #{parent['requirement']}" + + conflicting_dependencies.each do |conflicting_dep| + puts " #{conflicting_dep['name']} (#{conflicting_dep['version']}) "\ + "which requires:" + puts " #{dep.name} #{conflicting_dep['requirement']}" + end end if checker.up_to_date? From 2e50b1dfee7777b7b56097289dae2d4a31112177 Mon Sep 17 00:00:00 2001 From: jurre Date: Wed, 4 Nov 2020 17:15:13 +0100 Subject: [PATCH 13/13] Fix string keys in ConflictingDependencyResolver spec --- .../functions/conflicting_dependency_resolver_spec.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bundler/spec/native_helpers/functions/conflicting_dependency_resolver_spec.rb b/bundler/spec/native_helpers/functions/conflicting_dependency_resolver_spec.rb index 09320b0ae2..dbb3d944ea 100644 --- a/bundler/spec/native_helpers/functions/conflicting_dependency_resolver_spec.rb +++ b/bundler/spec/native_helpers/functions/conflicting_dependency_resolver_spec.rb @@ -27,9 +27,11 @@ it "returns a list of dependencies that block the update" do expect(conflicting_dependencies).to eq( - [ - { name: "dummy-pkg-b", version: "1.0.0", requirement: "< 2.0.0" } - ] + [{ + "name" => "dummy-pkg-b", + "version" => "1.0.0", + "requirement" => "< 2.0.0" + }] ) end