From c517e9eb64b8a1d29c0337790e387bbeb255d00d Mon Sep 17 00:00:00 2001
From: Maxwell Weru <mburumaxwell@gmail.com>
Date: Wed, 24 Jul 2024 11:06:45 +0300
Subject: [PATCH] Sync files for updater version 0.266.0
Follow up to https://github.com/tinglesoftware/dependabot-azure-devops/pull/1235
---
updater/lib/dependabot/dependency_change.rb | 23 +++--
updater/lib/dependabot/dependency_snapshot.rb | 2 +-
updater/lib/dependabot/environment.rb | 63 +++++++++---
.../lib/dependabot/file_fetcher_command.rb | 15 +--
.../updater/dependency_group_change_batch.rb | 51 ++--------
.../spec/dependabot/dependency_change_spec.rb | 71 ++++++++-----
.../dependency_group_change_batch_spec.rb | 99 -------------------
7 files changed, 116 insertions(+), 208 deletions(-)
diff --git a/updater/lib/dependabot/dependency_change.rb b/updater/lib/dependabot/dependency_change.rb
index e821b1db..161cbb50 100644
--- a/updater/lib/dependabot/dependency_change.rb
+++ b/updater/lib/dependabot/dependency_change.rb
@@ -165,24 +165,32 @@ def matches_existing_pr?
if grouped_update?
# We only want PRs for the same group that have the same versions
job.existing_group_pull_requests.any? do |pr|
+ directories_in_use = pr["dependencies"].all? { |dep| dep["directory"] }
+
pr["dependency-group-name"] == dependency_group&.name &&
- Set.new(pr["dependencies"]) == updated_dependencies_set
+ Set.new(pr["dependencies"]) == updated_dependencies_set(should_consider_directory: directories_in_use)
end
else
- job.existing_pull_requests.any? { |pr| Set.new(pr) == updated_dependencies_set }
+ job.existing_pull_requests.any? do |pr|
+ directories_in_use = pr.all? { |dep| dep["directory"] }
+
+ Set.new(pr) == updated_dependencies_set(should_consider_directory: directories_in_use)
+ end
end
end
private
- sig { returns(T::Set[T::Hash[String, T.any(String, T::Boolean)]]) }
- def updated_dependencies_set
+ # Older PRs will not have a directory key, in that case do not consider directory in the comparison. This will
+ # allow rebases to continue working for those, but for multi-directory configs we do compare with the directory.
+ sig { params(should_consider_directory: T::Boolean).returns(T::Set[T::Hash[String, T.any(String, T::Boolean)]]) }
+ def updated_dependencies_set(should_consider_directory:)
Set.new(
updated_dependencies.map do |dep|
{
"dependency-name" => dep.name,
"dependency-version" => dep.version,
- "directory" => should_consider_directory? ? dep.directory : nil,
+ "directory" => should_consider_directory ? dep.directory : nil,
"dependency-removed" => dep.removed? ? true : nil
}.compact
end
@@ -202,10 +210,5 @@ def directory
T.must(updated_dependency_files.first).directory
end
-
- sig { returns(T::Boolean) }
- def should_consider_directory?
- grouped_update? && Dependabot::Experiments.enabled?("dependency_has_directory")
- end
end
end
diff --git a/updater/lib/dependabot/dependency_snapshot.rb b/updater/lib/dependabot/dependency_snapshot.rb
index ebd950ed..d347a2c2 100644
--- a/updater/lib/dependabot/dependency_snapshot.rb
+++ b/updater/lib/dependabot/dependency_snapshot.rb
@@ -28,7 +28,7 @@ def self.create_from_job_definition(job:, job_definition:)
file
end
- if Dependabot::Experiments.enabled?(:globs) && job.source.directories
+ if job.source.directories
# The job.source.directory may contain globs, so we use the directories from the fetched files
job.source.directories = decoded_dependency_files.flat_map(&:directory).uniq
end
diff --git a/updater/lib/dependabot/environment.rb b/updater/lib/dependabot/environment.rb
index 7069d877..230423dd 100644
--- a/updater/lib/dependabot/environment.rb
+++ b/updater/lib/dependabot/environment.rb
@@ -1,64 +1,97 @@
-# typed: true
+# typed: strict
# frozen_string_literal: true
+require "sorbet-runtime"
+
module Dependabot
module Environment
+ extend T::Sig
+ extend T::Generic
+
+ sig { returns(String) }
def self.job_id
- @job_id ||= environment_variable("DEPENDABOT_JOB_ID")
+ @job_id ||= T.let(environment_variable("DEPENDABOT_JOB_ID"), T.nilable(String))
end
+ sig { returns(String) }
def self.job_token
- @job_token ||= environment_variable("DEPENDABOT_JOB_TOKEN")
+ @job_token ||= T.let(environment_variable("DEPENDABOT_JOB_TOKEN"), T.nilable(String))
end
+ sig { returns(T::Boolean) }
def self.debug_enabled?
- @debug_enabled ||= job_debug_enabled? || environment_debug_enabled?
+ @debug_enabled ||= T.let(job_debug_enabled? || environment_debug_enabled?, T.nilable(T::Boolean))
end
+ sig { returns(Symbol) }
def self.log_level
debug_enabled? ? :debug : :info
end
+ sig { returns(String) }
def self.api_url
- @api_url ||= environment_variable("DEPENDABOT_API_URL", "http://localhost:3001")
+ @api_url ||= T.let(environment_variable("DEPENDABOT_API_URL", "http://localhost:3001"), T.nilable(String))
end
+ sig { returns(String) }
def self.job_path
- @job_path ||= environment_variable("DEPENDABOT_JOB_PATH")
+ @job_path ||= T.let(environment_variable("DEPENDABOT_JOB_PATH"), T.nilable(String))
end
+ sig { returns(String) }
def self.output_path
- @output_path ||= environment_variable("DEPENDABOT_OUTPUT_PATH")
+ @output_path ||= T.let(environment_variable("DEPENDABOT_OUTPUT_PATH"), T.nilable(String))
end
+ sig { returns(T.nilable(String)) }
def self.repo_contents_path
- @repo_contents_path ||= environment_variable("DEPENDABOT_REPO_CONTENTS_PATH", nil)
+ @repo_contents_path ||= T.let(environment_variable("DEPENDABOT_REPO_CONTENTS_PATH", nil), T.nilable(String))
end
+ sig { returns(T::Boolean) }
def self.github_actions?
- @github_actions ||= environment_variable("GITHUB_ACTIONS", false)
+ b = T.cast(environment_variable("GITHUB_ACTIONS", false), T::Boolean)
+ @github_actions ||= T.let(b, T.nilable(T::Boolean))
end
+ sig { returns(T::Boolean) }
def self.deterministic_updates?
- @deterministic_updates ||= environment_variable("UPDATER_DETERMINISTIC", false)
+ b = T.cast(environment_variable("UPDATER_DETERMINISTIC", false), T::Boolean)
+ @deterministic_updates ||= T.let(b, T.nilable(T::Boolean))
end
+ sig { returns(T::Hash[String, T.untyped]) }
def self.job_definition
- @job_definition ||= JSON.parse(File.read(job_path))
+ @job_definition ||= T.let(JSON.parse(File.read(job_path)), T.nilable(T::Hash[String, T.untyped]))
end
+ sig do
+ type_parameters(:T)
+ .params(variable_name: String, default: T.any(Symbol, T.type_parameter(:T)))
+ .returns(T.any(String, T.type_parameter(:T)))
+ end
private_class_method def self.environment_variable(variable_name, default = :_undefined)
- return ENV.fetch(variable_name, default) unless default == :_undefined
-
- ENV.fetch(variable_name) do
- raise ArgumentError, "Missing environment variable #{variable_name}"
+ case default
+ when :_undefined
+ ENV.fetch(variable_name) do
+ raise ArgumentError, "Missing environment variable #{variable_name}"
+ end
+ else
+ val = ENV.fetch(variable_name, T.cast(default, T.type_parameter(:T)))
+ case val
+ when String
+ val = T.must(val.casecmp("true")).zero? if [true, false].include? default
+ end
+ T.cast(val, T.type_parameter(:T))
end
end
+ sig { returns(T::Boolean) }
private_class_method def self.job_debug_enabled?
!!job_definition.dig("job", "debug")
end
+ sig { returns(T::Boolean) }
private_class_method def self.environment_debug_enabled?
!!environment_variable("DEPENDABOT_DEBUG", false)
end
diff --git a/updater/lib/dependabot/file_fetcher_command.rb b/updater/lib/dependabot/file_fetcher_command.rb
index 21bc2d3f..e18898ea 100644
--- a/updater/lib/dependabot/file_fetcher_command.rb
+++ b/updater/lib/dependabot/file_fetcher_command.rb
@@ -100,19 +100,8 @@ def file_fetcher_for_directory(directory)
end
def dependency_files_for_multi_directories
- if Dependabot::Experiments.enabled?(:globs)
- return @dependency_files_for_multi_directories ||= dependency_files_for_globs
- end
-
- @dependency_files_for_multi_directories ||= job.source.directories.flat_map do |dir|
- ff = with_retries { file_fetcher_for_directory(dir) }
- files = ff.files
- post_ecosystem_versions(ff) if should_record_ecosystem_versions?
- files
- end
- end
+ return @dependency_files_for_multi_directories if defined?(@dependency_files_for_multi_directories)
- def dependency_files_for_globs
has_glob = T.let(false, T::Boolean)
directories = Dir.chdir(job.repo_contents_path) do
job.source.directories.map do |dir|
@@ -124,7 +113,7 @@ def dependency_files_for_globs
end.flatten
end.uniq
- directories.flat_map do |dir|
+ @dependency_files_for_multi_directories = directories.flat_map do |dir|
ff = with_retries { file_fetcher_for_directory(dir) }
begin
diff --git a/updater/lib/dependabot/updater/dependency_group_change_batch.rb b/updater/lib/dependabot/updater/dependency_group_change_batch.rb
index bbbc248a..96f542b7 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, updated_dependencies: [], changed: false, changes: 0 }
+ hsh[file.path] = { file: file, changed: false, changes: 0 }
end
@vendored_dependency_batch = {}
@@ -45,18 +45,8 @@ def updated_dependency_files
end
def merge(dependency_change)
- # FIXME: we shouldn't have to rely on this but because CreateGroupUpdatePullRequest explicitly checks
- # the DependencyChange.updated_dependencies, we need to add the updated dependencies to the global list
merge_dependency_changes(dependency_change.updated_dependencies)
-
- if Dependabot::Experiments.enabled?(:dependency_has_directory)
- merge_file_and_dependency_changes(
- dependency_change.updated_dependencies,
- dependency_change.updated_dependency_files
- )
- else
- merge_file_changes(dependency_change.updated_dependency_files)
- end
+ merge_file_changes(dependency_change.updated_dependency_files)
Dependabot.logger.debug("Dependencies updated:")
debug_updated_dependencies
@@ -72,7 +62,9 @@ def add_updated_dependency(dependency)
private
- # We should retain a list of all dependencies that we change.
+ # We should retain a list of all dependencies that we change, in future we may need to account for the folder
+ # in which these changes are made to permit-cross folder updates of the same dependency.
+ #
# This list may contain duplicates if we make iterative updates to a Dependency within a single group, but
# rather than re-write the Dependency objects to account for the changes from the lowest previous version
# to the final version, we should defer it to the Dependabot::PullRequestCreator::MessageBuilder as a
@@ -100,38 +92,7 @@ def merge_file_to_batch(file, batch)
0
end
- batch[file.path] = {
- file: file,
- updated_dependencies: batch.dig(file.path, :updated_dependencies) || [],
- changed: true,
- changes: change_count + 1
- }
- end
-
- 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_and_dependency_changes_to_batch(updated_file, @vendored_dependency_batch, updated_dependencies)
- else
- merge_file_and_dependency_changes_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
-
- previous_updated_dependencies = batch.dig(file.path, :updated_dependencies) || []
- updated_dependencies_list = previous_updated_dependencies.concat(updated_dependencies)
-
- batch[file.path] =
- { file: file, updated_dependencies: updated_dependencies_list, changed: true, changes: change_count + 1 }
+ batch[file.path] = { file: file, changed: true, changes: change_count + 1 }
end
def debug_updated_dependencies
diff --git a/updater/spec/dependabot/dependency_change_spec.rb b/updater/spec/dependabot/dependency_change_spec.rb
index b467876a..61d6fe0e 100644
--- a/updater/spec/dependabot/dependency_change_spec.rb
+++ b/updater/spec/dependabot/dependency_change_spec.rb
@@ -33,7 +33,8 @@
],
previous_requirements: [
{ file: "Gemfile", requirement: "~> 1.7.0", groups: [], source: nil }
- ]
+ ],
+ directory: "/"
)
]
end
@@ -269,14 +270,6 @@
end
describe "#matches_existing_pr?" do
- before do
- Dependabot::Experiments.register("dependency_has_directory", true)
- end
-
- after do
- Dependabot::Experiments.reset!
- end
-
context "when no existing pull requests are found" do
let(:job) do
instance_double(Dependabot::Job,
@@ -306,7 +299,8 @@
[
updated_dependencies.map do |dep|
{ "dependency-name" => dep.name,
- "dependency-version" => dep.version }
+ "dependency-version" => dep.version,
+ "directory" => dep.directory }
end
]
end
@@ -321,6 +315,21 @@
it "returns true" do
expect(dependency_change.matches_existing_pr?).to be true
end
+
+ context "when there's no directory in an existing PR that otherwise matches" do
+ let(:existing_pull_requests) do
+ [
+ updated_dependencies.map do |dep|
+ { "dependency-name" => dep.name,
+ "dependency-version" => dep.version }
+ end
+ ]
+ end
+
+ it "returns true" do
+ expect(dependency_change.matches_existing_pr?).to be true
+ end
+ end
end
context "when updating a grouped pull request with the same dependencies" do
@@ -332,13 +341,11 @@
let(:existing_group_pull_requests) do
[
{ "dependency-group-name" => "foo",
- "dependencies" => [
- updated_dependencies.map do |dep|
- { "dependency-name" => dep.name.to_s,
- "dependency-version" => dep.version.to_s,
- "directory" => dep.directory.to_s }
- end
- ] }
+ "dependencies" => updated_dependencies.map do |dep|
+ { "dependency-name" => dep.name.to_s,
+ "dependency-version" => dep.version.to_s,
+ "directory" => dep.directory.to_s }
+ end }
]
end
@@ -352,7 +359,23 @@
end
it "returns true" do
- expect(dependency_change.matches_existing_pr?).to be false
+ expect(dependency_change.matches_existing_pr?).to be true
+ end
+
+ context "when there's no directory in a PR that otherwise matches" do
+ let(:existing_group_pull_requests) do
+ [
+ { "dependency-group-name" => "foo",
+ "dependencies" => updated_dependencies.map do |dep|
+ { "dependency-name" => dep.name.to_s,
+ "dependency-version" => dep.version.to_s }
+ end }
+ ]
+ end
+
+ it "returns true" do
+ expect(dependency_change.matches_existing_pr?).to be true
+ end
end
end
@@ -365,13 +388,11 @@
let(:existing_group_pull_requests) do
[
{ "dependency-group-name" => "foo",
- "dependencies" => [
- updated_dependencies.map do |dep|
- { "dependency-name" => dep.name.to_s,
- "dependency-version" => dep.version.to_s,
- "directory" => "/foo" }
- end
- ] }
+ "dependencies" => updated_dependencies.map do |dep|
+ { "dependency-name" => dep.name.to_s,
+ "dependency-version" => dep.version.to_s,
+ "directory" => "/foo" }
+ end }
]
end
diff --git a/updater/spec/dependabot/updater/dependency_group_change_batch_spec.rb b/updater/spec/dependabot/updater/dependency_group_change_batch_spec.rb
index 44e8dc6c..08a2ab82 100644
--- a/updater/spec/dependabot/updater/dependency_group_change_batch_spec.rb
+++ b/updater/spec/dependabot/updater/dependency_group_change_batch_spec.rb
@@ -54,104 +54,5 @@
.current_dependency_files(job).map(&:name)).to eq(%w(Gemfile Gemfile.lock))
end
end
-
- context "when the :dependency_has_directory ff is enabled" do
- before do
- Dependabot::Experiments.register(:dependency_has_directory, true)
- end
-
- after do
- Dependabot::Experiments.reset!
- end
-
- let(:dependency_change) do
- Dependabot::DependencyChange.new(
- job: job,
- updated_dependency_files: updated_dependency_files,
- updated_dependencies: updated_dependencies,
- dependency_group: group
- )
- end
-
- let(:updated_dependency_files) do
- [
- Dependabot::DependencyFile.new(
- name: "Gemfile", content: "mock-updated-gemfile", directory: "/"
- ),
- Dependabot::DependencyFile.new(
- name: "Gemfile.lock", content: "mock-updated-gemfile-lock", directory: "/hello/.."
- ),
- Dependabot::DependencyFile.new(
- name: "Gemfile", content: "mock-updated-package-json", directory: "/elsewhere"
- ),
- Dependabot::DependencyFile.new(
- name: "Gemfile", content: "mock-package-json", directory: "unknown"
- ),
- Dependabot::DependencyFile.new(
- name: "Gemfile", content: "mock-package-json", directory: "../../oob"
- )
- ]
- end
-
- let(:updated_dependencies) do
- [
- Dependabot::Dependency.new(
- name: "test-dependency-1",
- package_manager: "bundler",
- version: "1.1.0",
- requirements: [
- {
- file: "Gemfile",
- requirement: "~> 1.1.0",
- groups: ["test"],
- source: nil
- }
- ],
- directory: "/"
- ),
- Dependabot::Dependency.new(
- name: "test-dependency-2",
- package_manager: "bundler",
- version: "1.1.0",
- requirements: [
- {
- file: "Gemfile",
- requirement: "~> 1.1.0",
- groups: ["test"],
- source: nil
- }
- ],
- directory: "/hello/.."
- )
- ]
- end
-
- let(:group) do
- Dependabot::DependencyGroup.new(
- name: "test",
- rules: { patterns: ["test-*"] }
- )
- end
-
- it "still tracks the updated_dependencies in the global list" do
- batch = described_class.new(initial_dependency_files: files)
- batch.merge(dependency_change)
-
- expect(batch.updated_dependencies).to eq(updated_dependencies)
- end
-
- it "tracks the updated dependencies in the appropriate updated_dependency_files" do
- batch = described_class.new(initial_dependency_files: files)
- batch.merge(dependency_change)
-
- # FIXME: All of the files in the dependency_file_batch will contain a copy of the updated dependencies.
- # Eventually this should be narrowed down so that only the files that were modified
- # by the newly merged dependency_change have their updated_dependencies updated.
- dependency_file_batch = batch.instance_variable_get(:@dependency_file_batch)
- expect(dependency_file_batch["/Gemfile"][:updated_dependencies]).to eq(updated_dependencies)
- expect(dependency_file_batch["/Gemfile.lock"][:updated_dependencies]).to eq(updated_dependencies)
- expect(dependency_file_batch["/unknown/Gemfile"][:updated_dependencies]).to eq(updated_dependencies)
- end
- end
end
end