diff --git a/silent/tests/testdata/err-glob-not-found.txt b/silent/tests/testdata/err-glob-not-found.txt new file mode 100644 index 00000000000..3df45959567 --- /dev/null +++ b/silent/tests/testdata/err-glob-not-found.txt @@ -0,0 +1,15 @@ +! dependabot update -f input.yml --local . --updater-image ghcr.io/dependabot/dependabot-updater-silent +stderr dependency_file_not_found + +# Testing glob configuration when there are no manifests. + +-- input.yml -- +job: + package-manager: "silent" + source: + directories: + - "**/*" + provider: example + hostname: example.com + api-endpoint: https://example.com/api/v3 + repo: dependabot/smoke-tests diff --git a/silent/tests/testdata/vu-group-multidir-all.txt b/silent/tests/testdata/vu-group-multidir-all.txt index ddb277c011b..3efe2661c99 100644 --- a/silent/tests/testdata/vu-group-multidir-all.txt +++ b/silent/tests/testdata/vu-group-multidir-all.txt @@ -120,5 +120,3 @@ job: - dependency-name: dependency-c dependency-version: 3.2.5 directory: "/bar" - experiments: - dependency_has_directory: true diff --git a/silent/tests/testdata/vu-group-multidir-semver.txt b/silent/tests/testdata/vu-group-multidir-semver.txt index 6d9d1d795f3..49ee1c14d6a 100644 --- a/silent/tests/testdata/vu-group-multidir-semver.txt +++ b/silent/tests/testdata/vu-group-multidir-semver.txt @@ -3,7 +3,7 @@ dependabot update -f input.yml --local . --updater-image ghcr.io/dependabot/dependabot-updater-silent stdout -count=2 create_pull_request pr-created foo/expected-1.json bar/expected-1.json -pr-created foo/expected-2.json +pr-created foo/expected-2.json bar/expected-2.json # When there is the same dependency in both directories, one is a major update the other a patch, # and the user asked for majors in one group and minors in the other, we should create two group PRs @@ -32,6 +32,12 @@ pr-created foo/expected-2.json "dependency-b": { "version": "2.0.1" } } +-- bar/expected-2.json -- +{ + "dependency-a": { "version": "2.0.1" }, + "dependency-b": { "version": "1.0.0" } +} + -- foo/expected-2.json -- { "dependency-a": { "version": "1.0.0" }, @@ -77,5 +83,3 @@ job: update-types: - minor - patch - experiments: - dependency_has_directory: true diff --git a/updater/lib/dependabot/dependency_snapshot.rb b/updater/lib/dependabot/dependency_snapshot.rb index 108a3fd4e88..9fe29693748 100644 --- a/updater/lib/dependabot/dependency_snapshot.rb +++ b/updater/lib/dependabot/dependency_snapshot.rb @@ -55,11 +55,13 @@ def all_dependencies sig { returns(T::Array[Dependabot::DependencyFile]) } def dependency_files + assert_current_directory_set! @dependency_files.select { |f| f.directory == @current_directory } end sig { returns(T::Array[Dependabot::Dependency]) } def dependencies + assert_current_directory_set! T.must(@dependencies[@current_directory]) end @@ -103,10 +105,22 @@ def job_group @dependency_group_engine.find_group(name: T.must(job.dependency_group_to_refresh)) end + sig { params(group: Dependabot::DependencyGroup).void } + def mark_group_handled(group) + directories.each do |directory| + @current_directory = directory + + # add the existing dependencies in the group so individual updates don't try to update them + add_handled_dependencies(dependencies_in_existing_pr_for_group(group).filter_map { |d| d["dependency-name"] }) + # also add dependencies that might be in the group, as a rebase would add them; + # this avoids individual PR creation that immediately is superseded by a group PR supersede + add_handled_dependencies(group.dependencies.map(&:name)) + end + end + sig { params(dependency_names: T.any(String, T::Array[String])).void } def add_handled_dependencies(dependency_names) - raise "Current directory not set" if @current_directory == "" - + assert_current_directory_set! set = @handled_dependencies[@current_directory] || Set.new set += Array(dependency_names) @handled_dependencies[@current_directory] = set @@ -114,18 +128,10 @@ def add_handled_dependencies(dependency_names) sig { returns(T::Set[String]) } def handled_dependencies - raise "Current directory not set" if @current_directory == "" - + assert_current_directory_set! T.must(@handled_dependencies[@current_directory]) end - # rubocop:disable Performance/Sum - sig { returns(T::Set[String]) } - def handled_dependencies_all_directories - T.must(@handled_dependencies.values.reduce(&:+)) - end - # rubocop:enable Performance/Sum - sig { params(dir: String).void } def current_directory=(dir) @current_directory = dir @@ -142,10 +148,6 @@ def ungrouped_dependencies # If no groups are defined, all dependencies are ungrouped by default. return allowed_dependencies unless groups.any? - if Dependabot::Experiments.enabled?(:dependency_has_directory) - return allowed_dependencies.reject { |dep| handled_dependencies_all_directories.include?(dep.name) } - end - # Otherwise return dependencies that haven't been handled during the group update portion. allowed_dependencies.reject { |dep| handled_dependencies.include?(dep.name) } end @@ -184,6 +186,8 @@ def initialize(job:, base_commit_sha:, dependency_files:) # rubocop:disable Metr end job.source.directory = @original_directory + # reset to ensure we don't accidentally use it later without setting it + @current_directory = "" return unless job.source.directory @current_directory = T.must(job.source.directory) @@ -210,6 +214,7 @@ def parse_files! sig { returns(Dependabot::FileParsers::Base) } def dependency_file_parser + assert_current_directory_set! job.source.directory = @current_directory Dependabot::FileParsers.for_package_manager(job.package_manager).new( dependency_files: dependency_files, @@ -220,5 +225,22 @@ def dependency_file_parser options: job.experiments ) end + + sig { params(group: Dependabot::DependencyGroup).returns(T::Array[T::Hash[String, String]]) } + def dependencies_in_existing_pr_for_group(group) + job.existing_group_pull_requests.find do |pr| + pr["dependency-group-name"] == group.name + end&.fetch("dependencies", []) || [] + end + + sig { void } + def assert_current_directory_set! + if @current_directory == "" && directories.count == 1 + @current_directory = T.must(directories.first) + return + end + + raise DependabotError, "Assertion failed: Current directory not set" if @current_directory == "" + end end end diff --git a/updater/lib/dependabot/file_fetcher_command.rb b/updater/lib/dependabot/file_fetcher_command.rb index e18898ea494..927db8f3e55 100644 --- a/updater/lib/dependabot/file_fetcher_command.rb +++ b/updater/lib/dependabot/file_fetcher_command.rb @@ -99,6 +99,7 @@ def file_fetcher_for_directory(directory) @file_fetchers[directory] ||= create_file_fetcher(directory: directory) end + # rubocop:disable Metrics/PerceivedComplexity def dependency_files_for_multi_directories return @dependency_files_for_multi_directories if defined?(@dependency_files_for_multi_directories) @@ -128,7 +129,14 @@ def dependency_files_for_multi_directories post_ecosystem_versions(ff) if should_record_ecosystem_versions? files end.compact + + if @dependency_files_for_multi_directories.empty? + raise Dependabot::DependencyFileNotFound, job.source.directories.join(", ") + end + + @dependency_files_for_multi_directories end + # rubocop:enable Metrics/PerceivedComplexity def dependency_files return @dependency_files if defined?(@dependency_files) diff --git a/updater/lib/dependabot/updater/group_update_creation.rb b/updater/lib/dependabot/updater/group_update_creation.rb index ba975dc5c0d..50d5ddca9d6 100644 --- a/updater/lib/dependabot/updater/group_update_creation.rb +++ b/updater/lib/dependabot/updater/group_update_creation.rb @@ -54,8 +54,7 @@ def compile_all_dependency_changes_for(group) Dependabot.logger.info("Updating the #{job.source.directory} directory.") group.dependencies.each do |dependency| - # We check dependency_snapshot.handled_dependencies instead of handled_dependencies_all_directories here - # because we still want to update a dependency if it's been updated in another manifest files, + # We still want to update a dependency if it's been updated in another manifest files, # but we should skip it if it's been updated in _the same_ manifest file if dependency_snapshot.handled_dependencies.include?(dependency.name) Dependabot.logger.info( @@ -427,13 +426,6 @@ def pr_exists_for_dependency_group?(group) job.existing_group_pull_requests.any? { |pr| pr["dependency-group-name"] == group.name } end - sig { params(group: Dependabot::DependencyGroup).returns(T::Array[T::Hash[String, String]]) } - def dependencies_in_existing_pr_for_group(group) - job.existing_group_pull_requests.find do |pr| - pr["dependency-group-name"] == group.name - end&.fetch("dependencies", []) - end - sig do params( dependency: T.nilable(Dependabot::Dependency), 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 06a365bf78e..afb90751448 100644 --- a/updater/lib/dependabot/updater/operations/group_update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/group_update_all_versions.rb @@ -99,7 +99,6 @@ def perform sig { returns(Dependabot::Updater::ErrorHandler) } attr_reader :error_handler - # rubocop:disable Metrics/AbcSize sig { returns(T::Array[Dependabot::DependencyGroup]) } def run_grouped_dependency_updates Dependabot.logger.info("Starting grouped update job for #{job.source.repo}") @@ -113,14 +112,7 @@ def run_grouped_dependency_updates Dependabot.logger.info( "Deferring creation of a new pull request. The existing pull request will update in a separate job." ) - - # add the dependencies in the group so individual updates don't try to update them - dependency_snapshot.add_handled_dependencies( - dependencies_in_existing_pr_for_group(group).filter_map { |d| d["dependency-name"] } - ) - # also add dependencies that might be in the group, as a rebase would add them; - # this avoids individual PR creation that immediately is superseded by a group PR supersede - dependency_snapshot.add_handled_dependencies(group.dependencies.map(&:name)) + dependency_snapshot.mark_group_handled(group) next end @@ -128,17 +120,11 @@ def run_grouped_dependency_updates end groups_without_pr.each do |group| - 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(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)) - end + dependency_change = run_grouped_update_for(group) + # The update failed, add the suspected dependencies to the handled list so they don't update individually. + dependency_snapshot.mark_group_handled(group) if dependency_change.nil? end end - # rubocop:enable Metrics/AbcSize sig { params(group: Dependabot::DependencyGroup).returns(T.nilable(Dependabot::DependencyChange)) } def run_grouped_update_for(group) diff --git a/updater/lib/dependabot/updater/operations/refresh_group_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_group_update_pull_request.rb index f535f92d934..4f813232748 100644 --- a/updater/lib/dependabot/updater/operations/refresh_group_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_group_update_pull_request.rb @@ -108,9 +108,7 @@ def perform # rubocop:disable Metrics/AbcSize dependency_snapshot.groups.each do |group| next unless group.name != job_group.name && pr_exists_for_dependency_group?(group) - dependency_snapshot.add_handled_dependencies( - dependencies_in_existing_pr_for_group(group).filter_map { |d| d["dependency-name"] } - ) + dependency_snapshot.mark_group_handled(group) end if dependency_change.nil? diff --git a/updater/spec/dependabot/dependency_snapshot_spec.rb b/updater/spec/dependabot/dependency_snapshot_spec.rb index 876337f48e1..2830dfe8fc4 100644 --- a/updater/spec/dependabot/dependency_snapshot_spec.rb +++ b/updater/spec/dependabot/dependency_snapshot_spec.rb @@ -140,59 +140,6 @@ end end - describe "::handled_dependencies_all_directories" do - subject(:create_dependency_snapshot) do - described_class.create_from_job_definition( - job: job, - job_definition: job_definition - ) - end - - let(:job_definition) do - { - "base_commit_sha" => base_commit_sha, - "base64_dependency_files" => encode_dependency_files(dependency_files) - } - end - - it "handles dependencies" do - snapshot = create_dependency_snapshot - snapshot.add_handled_dependencies(%w(a b)) - expect(snapshot.handled_dependencies_all_directories).to eq(Set.new(%w(a b))) - end - - context "when there are multiple directories" do - let(:directory) { nil } - let(:directories) { %w(/foo /bar) } - let(:dependency_files) do - [ - Dependabot::DependencyFile.new( - name: "Gemfile", - content: fixture("bundler/original/Gemfile"), - directory: "/foo" - ), - Dependabot::DependencyFile.new( - name: "Gemfile", - content: fixture("bundler/original/Gemfile"), - directory: "/bar" - ) - ] - end - - it "is agnostic of the current directory" do - snapshot = create_dependency_snapshot - snapshot.current_directory = "/foo" - snapshot.add_handled_dependencies("a") - snapshot.current_directory = "/bar" - snapshot.add_handled_dependencies("b") - - expect(snapshot.handled_dependencies_all_directories).to eq(Set.new(%w(a b))) - snapshot.current_directory = "/bar" - expect(snapshot.handled_dependencies_all_directories).to eq(Set.new(%w(a b))) - end - end - end - describe "::create_from_job_definition" do subject(:create_dependency_snapshot) do described_class.create_from_job_definition( @@ -330,4 +277,119 @@ end end end + + describe "::mark_group_handled" do + subject(:create_dependency_snapshot) do + described_class.create_from_job_definition( + job: job, + job_definition: job_definition + ) + end + + let(:job) do + instance_double(Dependabot::Job, + package_manager: "bundler", + security_updates_only?: false, + repo_contents_path: nil, + credentials: [], + reject_external_code?: false, + source: source, + dependency_groups: dependency_groups, + allowed_update?: true, + dependency_group_to_refresh: nil, + dependencies: nil, + experiments: { large_hadron_collider: true }, + existing_group_pull_requests: existing_group_pull_requests) + end + + let(:source) do + Dependabot::Source.new( + provider: "github", + repo: "dependabot-fixtures/dependabot-test-ruby-package", + directories: %w(/foo /bar), + branch: nil, + api_endpoint: "https://api.github.com/", + hostname: "github.com" + ) + end + + let(:dependency_groups) do + [ + { + "name" => "group-a", + "rules" => { + "patterns" => ["dummy-pkg-*"], + "exclude-patterns" => ["dummy-pkg-b"] + } + } + ] + end + + let(:existing_group_pull_requests) do + [ + { + "group" => "group-a", + "dependencies" => %w(dummy-pkg-a) + } + ] + end + + let(:job_definition) do + { + "base_commit_sha" => base_commit_sha, + "base64_dependency_files" => encode_dependency_files(dependency_files) + } + end + + let(:dependency_files) do + [ + Dependabot::DependencyFile.new( + name: "Gemfile", + content: fixture("bundler/original/Gemfile"), + directory: "/foo" + ), + Dependabot::DependencyFile.new( + name: "Gemfile.lock", + content: fixture("bundler/original/Gemfile.lock"), + directory: "/foo" + ), + Dependabot::DependencyFile.new( + name: "Gemfile", + content: fixture("bundler/original/Gemfile"), + directory: "/bar" + ), + Dependabot::DependencyFile.new( + name: "Gemfile.lock", + content: fixture("bundler/original/Gemfile.lock"), + directory: "/bar" + ) + ] + end + + it "marks the dependencies handled for all directories" do + snapshot = create_dependency_snapshot + snapshot.mark_group_handled(snapshot.groups.first) + + snapshot.current_directory = "/foo" + expect(snapshot.handled_dependencies).to eq(Set.new(%w(dummy-pkg-a))) + + snapshot.current_directory = "/bar" + expect(snapshot.handled_dependencies).to eq(Set.new(%w(dummy-pkg-a))) + end + + context "when there are no existing group pull requests" do + let(:existing_group_pull_requests) { [] } + + it "marks the dependencies that would have been covered as handled" do + snapshot = create_dependency_snapshot + snapshot.mark_group_handled(snapshot.groups.first) + + snapshot.current_directory = "/foo" + expect(snapshot.handled_dependencies).to eq(Set.new(%w(dummy-pkg-a))) + + snapshot.current_directory = "/bar" + expect(snapshot.handled_dependencies).to eq(Set.new(%w(dummy-pkg-a))) + end + end + end end diff --git a/updater/spec/dependabot/updater/operations/refresh_group_update_pull_request_spec.rb b/updater/spec/dependabot/updater/operations/refresh_group_update_pull_request_spec.rb index ceb22fe739b..3825770be6d 100644 --- a/updater/spec/dependabot/updater/operations/refresh_group_update_pull_request_spec.rb +++ b/updater/spec/dependabot/updater/operations/refresh_group_update_pull_request_spec.rb @@ -70,11 +70,6 @@ before do stub_rubygems_calls - Dependabot::Experiments.register("dependency_has_directory", true) - end - - after do - Dependabot::Experiments.reset! end it "updates the existing pull request without errors" do