From f9d55f868cdab70bcd9a3130ae3f60a36ec0faa1 Mon Sep 17 00:00:00 2001 From: Barry Gordon Date: Thu, 22 Jun 2023 21:30:22 +0100 Subject: [PATCH] Changes are not stored by default, it must be explicit --- common/lib/dependabot/workspace.rb | 26 +++++++++++++++ common/lib/dependabot/workspace/base.rb | 7 ++-- common/lib/dependabot/workspace/git.rb | 6 ++-- .../updater/group_update_creation.rb | 32 +++++++++++++++++++ 4 files changed, 64 insertions(+), 7 deletions(-) diff --git a/common/lib/dependabot/workspace.rb b/common/lib/dependabot/workspace.rb index ea505be79f..f7178ea8f3 100644 --- a/common/lib/dependabot/workspace.rb +++ b/common/lib/dependabot/workspace.rb @@ -9,5 +9,31 @@ module Workspace class << self attr_accessor :active_workspace end + + def self.setup(repo_contents_path:, directory:) + Dependabot.logger.debug("Setting up workspace in #{repo_contents_path}") + + @active_workspace = Dependabot::Workspace::Git.new( + repo_contents_path, + Pathname.new(directory || "/").cleanpath + ) + end + + def self.store_change(memo:) + return unless @active_workspace + + Dependabot.logger.debug("Storing change to workspace: #{memo}") + + @active_workspace.store_change(memo) + end + + def self.cleanup! + return unless @active_workspace + + Dependabot.logger.debug("Cleaning up current workspace") + + @active_workspace.reset! + @active_workspace = nil + end end end diff --git a/common/lib/dependabot/workspace/base.rb b/common/lib/dependabot/workspace/base.rb index 95620aba32..984bee13b9 100644 --- a/common/lib/dependabot/workspace/base.rb +++ b/common/lib/dependabot/workspace/base.rb @@ -25,15 +25,16 @@ def failed_change_attempts def change(memo = nil) change_attempt = nil Dir.chdir(path) { yield(path) } - change_attempt = capture_change(memo) rescue StandardError => e change_attempt = capture_failed_change_attempt(memo, e) + clean # clean up any failed changes raise e ensure change_attempts << change_attempt unless change_attempt.nil? - clean end + def store_change(memo = nil); end + def to_patch "" end @@ -42,8 +43,6 @@ def reset!; end protected - def capture_change(memo = nil); end - def capture_failed_change_attempt(memo = nil, error = nil); end end end diff --git a/common/lib/dependabot/workspace/git.rb b/common/lib/dependabot/workspace/git.rb index 0b7222e2c1..8fd342cb9e 100644 --- a/common/lib/dependabot/workspace/git.rb +++ b/common/lib/dependabot/workspace/git.rb @@ -26,9 +26,7 @@ def reset! nil end - protected - - def capture_change(memo = nil) + def store_change(memo = nil) changed_files = run_shell_command("git status --short .").strip return nil if changed_files.empty? @@ -36,6 +34,8 @@ def capture_change(memo = nil) ChangeAttempt.new(self, id: sha, memo: memo, diff: diff) end + protected + def capture_failed_change_attempt(memo = nil, error = nil) changed_files = run_shell_command("git status --untracked-files=all --ignored=matching --short .").strip diff --git a/updater/lib/dependabot/updater/group_update_creation.rb b/updater/lib/dependabot/updater/group_update_creation.rb index 98f6f33dc8..61d737d29c 100644 --- a/updater/lib/dependabot/updater/group_update_creation.rb +++ b/updater/lib/dependabot/updater/group_update_creation.rb @@ -2,6 +2,7 @@ require "dependabot/dependency_change_builder" require "dependabot/updater/dependency_group_change_batch" +require "dependabot/workspace" # This module contains the methods required to build a DependencyChange for # a single DependencyGroup. @@ -18,6 +19,8 @@ module GroupUpdateCreation # outcome of attempting to update every dependency iteratively which # can be used for PR creation. def compile_all_dependency_changes_for(group) + prepare_workspace + group_changes = Dependabot::Updater::DependencyGroupChangeBatch.new( initial_dependency_files: dependency_snapshot.dependency_files ) @@ -50,6 +53,7 @@ def compile_all_dependency_changes_for(group) # Store the updated files for the next loop group_changes.merge(dependency_change) + store_changes(dependency) end # Create a single Dependabot::DependencyChange that aggregates everything we've updated @@ -60,6 +64,8 @@ def compile_all_dependency_changes_for(group) updated_dependency_files: group_changes.updated_dependency_files, dependency_group: group ) + ensure + cleanup_workspace end def dependency_file_parser(dependency_files) @@ -232,6 +238,32 @@ def peer_dependency_should_update_instead?(dependency_name, dependency_files, up can_update?(requirements_to_unlock: :own) end end + + def prepare_workspace + return unless job.clone? && job.repo_contents_path + + # TODO: Remove the directory parameter + # + # We should defer calculation of the `path` to the call site in the shared_helper + # so it is impossible to calculate different values for workspace and non-workspace + # calls to the helper. + Dependabot::Workspace.setup( + repo_contents_path: job.repo_contents_path, + directory: Pathname.new(job.source.directory || "/").cleanpath + ) + end + + def store_changes(dependency) + return unless job.clone? && job.repo_contents_path + + Dependabot::Workspace.store_change(memo: "Updating #{dependency.name}") + end + + def cleanup_workspace + return unless job.clone? && job.repo_contents_path + + Dependabot::Workspace.cleanup! + end end end end