From f82a75219a934c7cac5486bc48b95f1157087302 Mon Sep 17 00:00:00 2001 From: Nish Sinha Date: Tue, 11 Jun 2024 22:28:51 -0400 Subject: [PATCH] spike --- updater/lib/dependabot/dependency_change.rb | 2 + .../updater/dependency_group_change_batch.rb | 40 +++++++++++++++++-- .../create_group_update_pull_request.rb | 6 +++ .../operations/group_update_all_versions.rb | 8 ++-- 4 files changed, 49 insertions(+), 7 deletions(-) diff --git a/updater/lib/dependabot/dependency_change.rb b/updater/lib/dependabot/dependency_change.rb index 1030a8ea2306..2d86490904cb 100644 --- a/updater/lib/dependabot/dependency_change.rb +++ b/updater/lib/dependabot/dependency_change.rb @@ -128,6 +128,7 @@ def should_replace_existing_pr? updated_dependencies.map { |x| x.name.downcase }.uniq.sort != T.must(job.dependencies).map(&:downcase).uniq.sort end + # Now that updated_dependency_files can store the updated_dependencies too, we need to make sure they don't get lost sig { params(dependency_changes: T::Array[DependencyChange]).void } def merge_changes!(dependency_changes) dependency_changes.each do |dependency_change| @@ -173,6 +174,7 @@ def matches_existing_pr? private + # FIXME: this needs to be updated to also consider the directory once it's in teh existing-group-pull-requests repsonse sig { returns(T::Set[T::Hash[String, T.any(String, T::Boolean)]]) } def updated_dependencies_set Set.new( diff --git a/updater/lib/dependabot/updater/dependency_group_change_batch.rb b/updater/lib/dependabot/updater/dependency_group_change_batch.rb index 96f542b70c0b..fe6e60c0bb52 100644 --- a/updater/lib/dependabot/updater/dependency_group_change_batch.rb +++ b/updater/lib/dependabot/updater/dependency_group_change_batch.rb @@ -14,7 +14,7 @@ def initialize(initial_dependency_files:) @updated_dependencies = [] @dependency_file_batch = initial_dependency_files.each_with_object({}) do |file, hsh| - hsh[file.path] = { file: file, changed: false, changes: 0 } + hsh[file.path] = { file: file, updated_dependencies: [], changed: false, changes: 0 } end @vendored_dependency_batch = {} @@ -45,8 +45,15 @@ def updated_dependency_files end def merge(dependency_change) - merge_dependency_changes(dependency_change.updated_dependencies) - merge_file_changes(dependency_change.updated_dependency_files) + if feature_enabled?(:dependency_has_directory) + merge_file_and_dependency_changes_to_batch( + dependency_change.updated_dependencies, + dependency_change.updated_dependency_files + ) + else + merge_dependency_changes(dependency_change.updated_dependencies) + merge_file_changes(dependency_change.updated_dependency_files) + end Dependabot.logger.debug("Dependencies updated:") debug_updated_dependencies @@ -95,6 +102,33 @@ def merge_file_to_batch(file, batch) batch[file.path] = { file: file, changed: true, changes: change_count + 1 } end + # FIXME: This is one of the harder changes to make + # we can either store the updated dpeendencies on the updated_dependnecy_file object + # or we can save the directory on the updated_dependency themselves + # Here I take a hybrid approach and end up saving the file separate from the updated dependencies list that goes on it + def merge_file_and_dependency_changes(updated_dependencies, updated_dependency_files) + updated_dependency_files.each do |updated_file| + if updated_file.vendored_file? + merge_file_to_batch(updated_file, @vendored_dependency_batch, updated_dependencies) + else + merge_file_to_batch(updated_file, @dependency_file_batch, updated_dependencies) + end + end + end + + def merge_file_and_dependency_changes_to_batch(file, batch, updated_dependencies) + change_count = if (existing_file = batch[file.path]) + existing_file.fetch(:change_count, 0) + else + # The file is newly encountered + Dependabot.logger.debug("File #{file.operation}d: '#{file.path}'") + 0 + end + + updated_dependencies_list = merge_dependency_changes(updated_dependencies) + batch[file.path] = { file: file, updated_dependencies: updated_dependencies_list, changed: true, changes: change_count + 1 } + end + def debug_updated_dependencies return unless Dependabot.logger.debug? diff --git a/updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb b/updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb index 91c047ba8578..8bb5e377fec0 100644 --- a/updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/create_group_update_pull_request.rb @@ -89,6 +89,12 @@ def perform sig { returns(Dependabot::DependencyGroup) } attr_reader :group + # FIXME: + # - every time the directory changes we lose track of the updated_dependencies from the previous directory + # for a grouped udpate + # - we want to list the updated dependencies under each updated_dependency_file. We will have to keep the current list + # of updated dependencies for backwards compatibility for now. We lose the list of updated dependencies for each directory + # after the dependency_change.merge_changes! call. sig { returns(T.nilable(Dependabot::DependencyChange)) } def dependency_change return @dependency_change if defined?(@dependency_change) diff --git a/updater/lib/dependabot/updater/operations/group_update_all_versions.rb b/updater/lib/dependabot/updater/operations/group_update_all_versions.rb index 113709eefe92..4a874ed9e0a3 100644 --- a/updater/lib/dependabot/updater/operations/group_update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/group_update_all_versions.rb @@ -126,10 +126,10 @@ def run_grouped_dependency_updates # rubocop:disable Metrics/AbcSize end groups_without_pr.each do |group| - result = run_update_for(group) - if result + grouped_update_result = run_grouped_update_for(group) + if grouped_update_result # Add the actual updated dependencies to the handled list so they don't get updated individually. - dependency_snapshot.add_handled_dependencies(result.updated_dependencies.map(&:name)) + dependency_snapshot.add_handled_dependencies(grouped_update_result.updated_dependencies.map(&:name)) else # The update failed, add the suspected dependencies to the handled list so they don't update individually. dependency_snapshot.add_handled_dependencies(group.dependencies.map(&:name)) @@ -138,7 +138,7 @@ def run_grouped_dependency_updates # rubocop:disable Metrics/AbcSize end sig { params(group: Dependabot::DependencyGroup).returns(T.nilable(Dependabot::DependencyChange)) } - def run_update_for(group) + def run_grouped_update_for(group) Dependabot::Updater::Operations::CreateGroupUpdatePullRequest.new( service: service, job: job,