Skip to content

Commit

Permalink
disregard peer dependencies that are in the same group (dependabot#7561)
Browse files Browse the repository at this point in the history
  • Loading branch information
jakecoffman committed Jul 18, 2023
1 parent 2e22eb3 commit 5d57131
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 38 deletions.
5 changes: 3 additions & 2 deletions common/lib/dependabot/update_checkers/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ class Base
attr_reader :dependency, :dependency_files, :repo_contents_path,
:credentials, :ignored_versions, :raise_on_ignored,
:security_advisories, :requirements_update_strategy,
:options
:dependency_group, :options

def initialize(dependency:, dependency_files:, repo_contents_path: nil,
credentials:, ignored_versions: [],
raise_on_ignored: false, security_advisories: [],
requirements_update_strategy: nil,
requirements_update_strategy: nil, dependency_group: nil,
options: {})
@dependency = dependency
@dependency_files = dependency_files
Expand All @@ -25,6 +25,7 @@ def initialize(dependency:, dependency_files:, repo_contents_path: nil,
@ignored_versions = ignored_versions
@raise_on_ignored = raise_on_ignored
@security_advisories = security_advisories
@dependency_group = dependency_group
@options = options
end

Expand Down
3 changes: 2 additions & 1 deletion npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,8 @@ def version_resolver
dependency_files: dependency_files,
latest_allowable_version: latest_version,
latest_version_finder: latest_version_finder,
repo_contents_path: repo_contents_path
repo_contents_path: repo_contents_path,
dependency_group: dependency_group
)
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,12 @@ class VersionResolver
/x

def initialize(dependency:, credentials:, dependency_files:,
latest_allowable_version:, latest_version_finder:, repo_contents_path:)
latest_allowable_version:, latest_version_finder:, repo_contents_path:, dependency_group: nil)
@dependency = dependency
@credentials = credentials
@dependency_files = dependency_files
@latest_allowable_version = latest_allowable_version
@dependency_group = dependency_group

@latest_version_finder = {}
@latest_version_finder[dependency] = latest_version_finder
Expand Down Expand Up @@ -153,7 +154,7 @@ def dependency_updates_from_full_unlock
private

attr_reader :dependency, :credentials, :dependency_files,
:latest_allowable_version, :repo_contents_path
:latest_allowable_version, :repo_contents_path, :dependency_group

def latest_version_finder(dep)
@latest_version_finder[dep] ||=
Expand Down Expand Up @@ -399,6 +400,17 @@ def relevant_unmet_peer_dependencies
dep[:requiring_dep_name] == dependency.name
end

unless dependency_group.nil?
# Ignore unmet peer dependencies that are in the dependency group because
# the update is also updating those dependencies.
relevant_unmet_peer_dependencies.reject! do |dep|
dependency_group.dependencies.any? do |group_dep|
dep[:requirement_name] == group_dep.name ||
dep[:requiring_dep_name] == group_dep.name
end
end
end

return [] if relevant_unmet_peer_dependencies.empty?

# Prune out any pre-existing warnings
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
credentials: credentials,
latest_allowable_version: latest_allowable_version,
latest_version_finder: latest_version_finder,
repo_contents_path: nil
repo_contents_path: nil,
dependency_group: group
)
end
let(:latest_version_finder) do
Expand Down Expand Up @@ -69,6 +70,8 @@
}]
end

let(:group) { nil }

describe "#latest_resolvable_version" do
subject { resolver.latest_resolvable_version }

Expand Down Expand Up @@ -289,6 +292,19 @@
end

it { is_expected.to eq(Gem::Version.new("0.14.9")) }

context "with a dependency group containing the peer" do
let(:group) do
group = Dependabot::DependencyGroup.new(
name: "group",
rules: []
)
group.dependencies.push(dependency)
group
end

it { is_expected.to eq(Gem::Version.new("16.3.1")) }
end
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,8 @@
dependency_files: dependency_files,
latest_version_finder: described_class::LatestVersionFinder,
latest_allowable_version: updated_version,
repo_contents_path: nil
repo_contents_path: nil,
dependency_group: nil
).and_return(dummy_version_resolver)
expect(dummy_version_resolver).
to receive(:latest_resolvable_previous_version).
Expand Down Expand Up @@ -1259,7 +1260,8 @@
dependency_files: dependency_files,
latest_version_finder: described_class::LatestVersionFinder,
latest_allowable_version: Gem::Version.new("1.7.0"),
repo_contents_path: nil
repo_contents_path: nil,
dependency_group: nil
).and_return(dummy_version_resolver)
expect(dummy_version_resolver).
to receive(:dependency_updates_from_full_unlock).
Expand Down
36 changes: 6 additions & 30 deletions updater/lib/dependabot/updater/group_update_creation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,9 @@ def create_change_for(lead_dependency, updated_dependencies, dependency_files, d
#
# This method **must** must return an Array when it errors
#
def compile_updates_for(dependency, dependency_files, group) # rubocop:disable Metrics/MethodLength
checker = update_checker_for(dependency, dependency_files, raise_on_ignored: raise_on_ignored?(dependency))
def compile_updates_for(dependency, dependency_files, group)
checker = update_checker_for(dependency, dependency_files, group,
raise_on_ignored: raise_on_ignored?(dependency))

log_checking_for_update(dependency)

Expand All @@ -135,18 +136,9 @@ def compile_updates_for(dependency, dependency_files, group) # rubocop:disable M
return []
end

updated_deps = checker.updated_dependencies(
checker.updated_dependencies(
requirements_to_unlock: requirements_to_unlock
)

if peer_dependency_should_update_instead?(checker.dependency.name, dependency_files, updated_deps)
Dependabot.logger.info(
"No update possible for #{dependency.name} #{dependency.version} (peer dependency can be updated)"
)
return []
end

updated_deps
rescue Dependabot::InconsistentRegistryResponse => e
error_handler.log_dependency_error(
dependency: dependency,
Expand All @@ -170,7 +162,7 @@ def raise_on_ignored?(dependency)
job.ignore_conditions_for(dependency).any?
end

def update_checker_for(dependency, dependency_files, raise_on_ignored:)
def update_checker_for(dependency, dependency_files, group, raise_on_ignored:)
Dependabot::UpdateCheckers.for_package_manager(job.package_manager).new(
dependency: dependency,
dependency_files: dependency_files,
Expand All @@ -180,6 +172,7 @@ def update_checker_for(dependency, dependency_files, raise_on_ignored:)
security_advisories: [], # FIXME: Version updates do not use advisory data for now
raise_on_ignored: raise_on_ignored,
requirements_update_strategy: job.requirements_update_strategy,
dependency_group: group,
options: job.experiments
)
end
Expand Down Expand Up @@ -222,23 +215,6 @@ def log_requirements_for_update(requirements_to_unlock, checker)
)
end

# If a version update for a peer dependency is possible we should
# defer to the PR that will be created for it to avoid duplicate PRs.
def peer_dependency_should_update_instead?(dependency_name, dependency_files, updated_deps)
updated_deps.
reject { |dep| dep.name == dependency_name }.
any? do |dep|
original_peer_dep = ::Dependabot::Dependency.new(
name: dep.name,
version: dep.previous_version,
requirements: dep.previous_requirements,
package_manager: dep.package_manager
)
update_checker_for(original_peer_dep, dependency_files, raise_on_ignored: false).
can_update?(requirements_to_unlock: :own)
end
end

def warn_group_is_empty(group)
Dependabot.logger.warn(
"Skipping update group for '#{group.name}' as it does not match any allowed dependencies."
Expand Down

0 comments on commit 5d57131

Please sign in to comment.