From 8767db870b4f0c63940e8c92dcf1dd93f351efd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Mon, 17 Oct 2022 14:28:59 +0200 Subject: [PATCH 1/2] Support custom update strategies for cargo Currently only the "lockfile_only" and "auto" strategies are supported, and the lockfile_only one is handled separately. However, once the lockfile_only strategy is handled as just another requirement update strategy, this will need to support custom strategies, since that will be passed around. Fix that for now, also making cargo look like all the other update checkers, that support custom strategies. --- cargo/lib/dependabot/cargo/update_checker.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cargo/lib/dependabot/cargo/update_checker.rb b/cargo/lib/dependabot/cargo/update_checker.rb index 23a3121f904..98cb0ef277f 100644 --- a/cargo/lib/dependabot/cargo/update_checker.rb +++ b/cargo/lib/dependabot/cargo/update_checker.rb @@ -76,6 +76,10 @@ def updated_requirements end def requirements_update_strategy + # If passed in as an option (in the base class) honour that option + return @requirements_update_strategy.to_sym if @requirements_update_strategy + + # Otherwise, widen ranges for libraries and bump versions for apps library? ? :bump_versions_if_necessary : :bump_versions end From c1fa6d81df62c07142b6e7806765d299bb50bebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Mon, 17 Oct 2022 13:10:34 +0200 Subject: [PATCH 2/2] Reapply lockfile-only fix In addition to avoiding updating Requirements in `RequirementUpdater` classes, the lockfile-only strategy needs to also be checked when we decide which requirements to unlock in order to figure out the target version, because otherwise, even if we don't allow the manifest file to change, we'll still may be allowing higher target versions than the manifest allows, causing the lock file to fall out of sync. --- bin/dry-run.rb | 9 ++------ .../lib/dependabot/bundler/update_checker.rb | 10 +++++++-- .../update_checker/requirements_updater.rb | 4 +++- .../requirements_updater_spec.rb | 8 +++++++ .../dependabot/bundler/update_checker_spec.rb | 16 +++++++++++++- cargo/lib/dependabot/cargo/update_checker.rb | 4 ++++ .../update_checker/requirements_updater.rb | 4 +++- .../requirements_updater_spec.rb | 8 +++++++ .../dependabot/cargo/update_checker_spec.rb | 16 +++++++++++++- common/lib/dependabot/dependency.rb | 4 ++++ .../lib/dependabot/composer/update_checker.rb | 4 ++++ .../update_checker/requirements_updater.rb | 3 ++- .../requirements_updater_spec.rb | 8 +++++++ .../composer/update_checker_spec.rb | 16 +++++++++++++- .../dependabot/npm_and_yarn/update_checker.rb | 4 ++++ .../update_checker/requirements_updater.rb | 4 +++- .../requirements_updater_spec.rb | 8 +++++++ .../npm_and_yarn/update_checker_spec.rb | 14 ++++++++++++ .../lib/dependabot/python/update_checker.rb | 4 ++++ .../update_checker/requirements_updater.rb | 2 ++ .../requirements_updater_spec.rb | 8 +++++++ .../dependabot/python/update_checker_spec.rb | 16 +++++++++++++- updater/lib/dependabot/job.rb | 17 +++++++++----- .../updater/group_update_creation.rb | 2 +- .../create_security_update_pull_request.rb | 2 +- .../refresh_security_update_pull_request.rb | 2 +- .../refresh_version_update_pull_request.rb | 2 +- .../updater/operations/update_all_versions.rb | 2 +- updater/spec/dependabot/job_spec.rb | 11 +++++++++- updater/spec/dependabot/updater_spec.rb | 22 +++++++++++++++++-- 30 files changed, 203 insertions(+), 31 deletions(-) diff --git a/bin/dry-run.rb b/bin/dry-run.rb index 6eed8d3f387..7b9266083cc 100755 --- a/bin/dry-run.rb +++ b/bin/dry-run.rb @@ -115,7 +115,6 @@ cache_steps: [], write: false, clone: false, - lockfile_only: false, reject_external_code: false, requirements_update_strategy: nil, commit: nil, @@ -186,15 +185,11 @@ $options[:write] = true end - opts.on("--lockfile-only", "Only update the lockfile") do |_value| - $options[:lockfile_only] = true - end - opts.on("--reject-external-code", "Reject external code") do |_value| $options[:reject_external_code] = true end - opts_req_desc = "Options: auto, widen_ranges, bump_versions or " \ + opts_req_desc = "Options: lockfile_only, auto, widen_ranges, bump_versions or " \ "bump_versions_if_necessary" opts.on("--requirements-update-strategy STRATEGY", opts_req_desc) do |value| value = nil if value == "auto" @@ -695,7 +690,7 @@ def security_fix?(dependency) puts " => latest allowed version is #{latest_allowed_version || dep.version}" requirements_to_unlock = - if $options[:lockfile_only] || !checker.requirements_unlocked_or_can_be? + if !checker.requirements_unlocked_or_can_be? if checker.can_update?(requirements_to_unlock: :none) then :none else :update_not_possible diff --git a/bundler/lib/dependabot/bundler/update_checker.rb b/bundler/lib/dependabot/bundler/update_checker.rb index c98d404c911..06e9ee2d156 100644 --- a/bundler/lib/dependabot/bundler/update_checker.rb +++ b/bundler/lib/dependabot/bundler/update_checker.rb @@ -73,8 +73,10 @@ def updated_requirements end def requirements_unlocked_or_can_be? - dependency.requirements. - select { |r| requirement_class.new(r[:requirement]).specific? }. + return true if requirements_unlocked? + return false if requirements_update_strategy == :lockfile_only + + dependency.specific_requirements. all? do |req| file = dependency_files.find { |f| f.name == req.fetch(:file) } updated = FileUpdater::RequirementReplacer.new( @@ -109,6 +111,10 @@ def conflicting_dependencies private + def requirements_unlocked? + dependency.specific_requirements.none? + end + def latest_version_resolvable_with_full_unlock? return false unless latest_version diff --git a/bundler/lib/dependabot/bundler/update_checker/requirements_updater.rb b/bundler/lib/dependabot/bundler/update_checker/requirements_updater.rb index 9ae4bd1e051..bf5975384b8 100644 --- a/bundler/lib/dependabot/bundler/update_checker/requirements_updater.rb +++ b/bundler/lib/dependabot/bundler/update_checker/requirements_updater.rb @@ -9,7 +9,7 @@ class RequirementsUpdater class UnfixableRequirement < StandardError; end ALLOWED_UPDATE_STRATEGIES = - %i(bump_versions bump_versions_if_necessary).freeze + %i(lockfile_only bump_versions bump_versions_if_necessary).freeze def initialize(requirements:, update_strategy:, updated_source:, latest_version:, latest_resolvable_version:) @@ -27,6 +27,8 @@ def initialize(requirements:, update_strategy:, updated_source:, end def updated_requirements + return requirements if update_strategy == :lockfile_only + requirements.map do |req| if req[:file].include?(".gemspec") update_gemspec_requirement(req) diff --git a/bundler/spec/dependabot/bundler/update_checker/requirements_updater_spec.rb b/bundler/spec/dependabot/bundler/update_checker/requirements_updater_spec.rb index d0ca3660cae..a9c63c9ba94 100644 --- a/bundler/spec/dependabot/bundler/update_checker/requirements_updater_spec.rb +++ b/bundler/spec/dependabot/bundler/update_checker/requirements_updater_spec.rb @@ -505,5 +505,13 @@ end end end + + context "when lockfile_only configured" do + let(:update_strategy) { :lockfile_only } + + it "does not change any requirements" do + expect(updated_requirements).to eq(requirements) + end + end end end diff --git a/bundler/spec/dependabot/bundler/update_checker_spec.rb b/bundler/spec/dependabot/bundler/update_checker_spec.rb index 01043f33d4b..3cf5321e957 100644 --- a/bundler/spec/dependabot/bundler/update_checker_spec.rb +++ b/bundler/spec/dependabot/bundler/update_checker_spec.rb @@ -16,7 +16,8 @@ dependency_files: dependency_files, credentials: credentials, ignored_versions: ignored_versions, - security_advisories: security_advisories + security_advisories: security_advisories, + requirements_update_strategy: requirements_update_strategy ) end let(:credentials) do @@ -33,6 +34,7 @@ let(:directory) { "/" } let(:ignored_versions) { [] } let(:security_advisories) { [] } + let(:requirements_update_strategy) { nil } let(:dependency) do Dependabot::Dependency.new( @@ -1795,6 +1797,12 @@ end it { is_expected.to eq(true) } + + context "and with the lockfile-only requirements update strategy set" do + let(:requirements_update_strategy) { :lockfile_only } + + it { is_expected.to eq(true) } + end end context "with a sub-dependency" do @@ -1821,6 +1829,12 @@ it { is_expected.to eq(true) } end + + context "but with the lockfile-only requirements update strategy set" do + let(:requirements_update_strategy) { :lockfile_only } + + it { is_expected.to eq(false) } + end end # For now we always let git dependencies through diff --git a/cargo/lib/dependabot/cargo/update_checker.rb b/cargo/lib/dependabot/cargo/update_checker.rb index 98cb0ef277f..f215ed0062e 100644 --- a/cargo/lib/dependabot/cargo/update_checker.rb +++ b/cargo/lib/dependabot/cargo/update_checker.rb @@ -75,6 +75,10 @@ def updated_requirements ).updated_requirements end + def requirements_unlocked_or_can_be? + requirements_update_strategy != :lockfile_only + end + def requirements_update_strategy # If passed in as an option (in the base class) honour that option return @requirements_update_strategy.to_sym if @requirements_update_strategy diff --git a/cargo/lib/dependabot/cargo/update_checker/requirements_updater.rb b/cargo/lib/dependabot/cargo/update_checker/requirements_updater.rb index a19b71beca0..81f116f2e5d 100644 --- a/cargo/lib/dependabot/cargo/update_checker/requirements_updater.rb +++ b/cargo/lib/dependabot/cargo/update_checker/requirements_updater.rb @@ -18,7 +18,7 @@ class UnfixableRequirement < StandardError; end VERSION_REGEX = /[0-9]+(?:\.[A-Za-z0-9\-*]+)*/ ALLOWED_UPDATE_STRATEGIES = - %i(bump_versions bump_versions_if_necessary).freeze + %i(lockfile_only bump_versions bump_versions_if_necessary).freeze def initialize(requirements:, updated_source:, update_strategy:, target_version:) @@ -34,6 +34,8 @@ def initialize(requirements:, updated_source:, update_strategy:, end def updated_requirements + return requirements if update_strategy == :lockfile_only + # NOTE: Order is important here. The FileUpdater needs the updated # requirement at index `i` to correspond to the previous requirement # at the same index. diff --git a/cargo/spec/dependabot/cargo/update_checker/requirements_updater_spec.rb b/cargo/spec/dependabot/cargo/update_checker/requirements_updater_spec.rb index 57fc55d99e1..7cdda35cff7 100644 --- a/cargo/spec/dependabot/cargo/update_checker/requirements_updater_spec.rb +++ b/cargo/spec/dependabot/cargo/update_checker/requirements_updater_spec.rb @@ -363,5 +363,13 @@ end end end + + context "for a lockfile_only strategy" do + let(:update_strategy) { :lockfile_only } + + it "does not change any requirements" do + expect(updater.updated_requirements).to eq(requirements) + end + end end end diff --git a/cargo/spec/dependabot/cargo/update_checker_spec.rb b/cargo/spec/dependabot/cargo/update_checker_spec.rb index e10af77dd35..df0a15389a6 100644 --- a/cargo/spec/dependabot/cargo/update_checker_spec.rb +++ b/cargo/spec/dependabot/cargo/update_checker_spec.rb @@ -23,13 +23,15 @@ credentials: credentials, ignored_versions: ignored_versions, raise_on_ignored: raise_on_ignored, - security_advisories: security_advisories + security_advisories: security_advisories, + requirements_update_strategy: requirements_update_strategy ) end let(:ignored_versions) { [] } let(:raise_on_ignored) { false } let(:security_advisories) { [] } + let(:requirements_update_strategy) { nil } let(:credentials) do [{ "type" => "git_source", @@ -483,4 +485,16 @@ end end end + + context "#requirements_unlocked_or_can_be?" do + subject { checker.requirements_unlocked_or_can_be? } + + it { is_expected.to eq(true) } + + context "with the lockfile-only requirements update strategy set" do + let(:requirements_update_strategy) { :lockfile_only } + + it { is_expected.to eq(false) } + end + end end diff --git a/common/lib/dependabot/dependency.rb b/common/lib/dependabot/dependency.rb index e1fa7da1e5a..bc33e56b048 100644 --- a/common/lib/dependabot/dependency.rb +++ b/common/lib/dependabot/dependency.rb @@ -199,6 +199,10 @@ def eql?(other) self == other end + def specific_requirements + requirements.select { |r| requirement_class.new(r[:requirement]).specific? } + end + def requirement_class Utils.requirement_class_for_package_manager(package_manager) end diff --git a/composer/lib/dependabot/composer/update_checker.rb b/composer/lib/dependabot/composer/update_checker.rb index 66820257b92..8a7e7010963 100644 --- a/composer/lib/dependabot/composer/update_checker.rb +++ b/composer/lib/dependabot/composer/update_checker.rb @@ -68,6 +68,10 @@ def updated_requirements ).updated_requirements end + def requirements_unlocked_or_can_be? + requirements_update_strategy != :lockfile_only + end + def requirements_update_strategy # If passed in as an option (in the base class) honour that option return @requirements_update_strategy.to_sym if @requirements_update_strategy diff --git a/composer/lib/dependabot/composer/update_checker/requirements_updater.rb b/composer/lib/dependabot/composer/update_checker/requirements_updater.rb index 99ed5e6c3e3..62318f78b3c 100644 --- a/composer/lib/dependabot/composer/update_checker/requirements_updater.rb +++ b/composer/lib/dependabot/composer/update_checker/requirements_updater.rb @@ -19,7 +19,7 @@ class RequirementsUpdater OR_SEPARATOR = /(?<=[a-zA-Z0-9*])[\s,]*\|\|?\s*/ SEPARATOR = /(?:#{AND_SEPARATOR})|(?:#{OR_SEPARATOR})/ ALLOWED_UPDATE_STRATEGIES = - %i(widen_ranges bump_versions bump_versions_if_necessary).freeze + %i(lockfile_only widen_ranges bump_versions bump_versions_if_necessary).freeze def initialize(requirements:, update_strategy:, latest_resolvable_version:) @@ -35,6 +35,7 @@ def initialize(requirements:, update_strategy:, end def updated_requirements + return requirements if update_strategy == :lockfile_only return requirements unless latest_resolvable_version requirements.map { |req| updated_requirement(req) } diff --git a/composer/spec/dependabot/composer/update_checker/requirements_updater_spec.rb b/composer/spec/dependabot/composer/update_checker/requirements_updater_spec.rb index 395a57f72bc..1c9e8f984c7 100644 --- a/composer/spec/dependabot/composer/update_checker/requirements_updater_spec.rb +++ b/composer/spec/dependabot/composer/update_checker/requirements_updater_spec.rb @@ -705,5 +705,13 @@ end end end + + context "with lockfile_only as the update strategy" do + let(:update_strategy) { :lockfile_only } + + it "does not update any requirements" do + expect(updater.updated_requirements).to eq(requirements) + end + end end end diff --git a/composer/spec/dependabot/composer/update_checker_spec.rb b/composer/spec/dependabot/composer/update_checker_spec.rb index d829354835c..d0a8143033a 100644 --- a/composer/spec/dependabot/composer/update_checker_spec.rb +++ b/composer/spec/dependabot/composer/update_checker_spec.rb @@ -16,7 +16,8 @@ credentials: credentials, ignored_versions: ignored_versions, raise_on_ignored: raise_on_ignored, - security_advisories: security_advisories + security_advisories: security_advisories, + requirements_update_strategy: requirements_update_strategy ) end @@ -31,6 +32,7 @@ let(:ignored_versions) { [] } let(:raise_on_ignored) { false } let(:security_advisories) { [] } + let(:requirements_update_strategy) { nil } let(:dependency_name) { "monolog/monolog" } let(:dependency_version) { "1.0.1" } let(:requirements) do @@ -871,4 +873,16 @@ end end end + + context "#requirements_unlocked_or_can_be?" do + subject { checker.requirements_unlocked_or_can_be? } + + it { is_expected.to eq(true) } + + context "with the lockfile-only requirements update strategy set" do + let(:requirements_update_strategy) { :lockfile_only } + + it { is_expected.to eq(false) } + end + end end diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb index 6cb6eb1f9bc..c2f07105970 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker.rb @@ -102,6 +102,10 @@ def updated_requirements ).updated_requirements end + def requirements_unlocked_or_can_be? + requirements_update_strategy != :lockfile_only + end + def requirements_update_strategy # If passed in as an option (in the base class) honour that option return @requirements_update_strategy.to_sym if @requirements_update_strategy diff --git a/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/requirements_updater.rb b/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/requirements_updater.rb index 2bae3cd2dd3..59fc7a1e521 100644 --- a/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/requirements_updater.rb +++ b/npm_and_yarn/lib/dependabot/npm_and_yarn/update_checker/requirements_updater.rb @@ -15,7 +15,7 @@ class UpdateChecker class RequirementsUpdater VERSION_REGEX = /[0-9]+(?:\.[A-Za-z0-9\-_]+)*/ SEPARATOR = /(?<=[a-zA-Z0-9*])[\s|]+(?![\s|-])/ - ALLOWED_UPDATE_STRATEGIES = %i(widen_ranges bump_versions bump_versions_if_necessary).freeze + ALLOWED_UPDATE_STRATEGIES = %i(lockfile_only widen_ranges bump_versions bump_versions_if_necessary).freeze def initialize(requirements:, updated_source:, update_strategy:, latest_resolvable_version:) @@ -32,6 +32,8 @@ def initialize(requirements:, updated_source:, update_strategy:, end def updated_requirements + return requirements if update_strategy == :lockfile_only + requirements.map do |req| req = req.merge(source: updated_source) next req unless latest_resolvable_version diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/requirements_updater_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/requirements_updater_spec.rb index e41b5e85baa..17ed59075ed 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/requirements_updater_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/requirements_updater_spec.rb @@ -606,5 +606,13 @@ end end end + + context "for a requirement being left alone" do + let(:update_strategy) { :lockfile_only } + + it "does not update any requirements" do + expect(updater.updated_requirements).to eq(requirements) + end + end end end diff --git a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb index 41e70d8ba26..81a4492ea9f 100644 --- a/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb +++ b/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker_spec.rb @@ -30,11 +30,13 @@ credentials: credentials, ignored_versions: ignored_versions, security_advisories: security_advisories, + requirements_update_strategy: requirements_update_strategy, options: options ) end let(:ignored_versions) { [] } let(:security_advisories) { [] } + let(:requirements_update_strategy) { nil } let(:dependency_files) { project_dependency_files("npm6/no_lockfile") } let(:options) { {} } @@ -1214,6 +1216,18 @@ end end + context "#requirements_unlocked_or_can_be?" do + subject { checker.requirements_unlocked_or_can_be? } + + it { is_expected.to eq(true) } + + context "with the lockfile-only requirements update strategy set" do + let(:requirements_update_strategy) { :lockfile_only } + + it { is_expected.to eq(false) } + end + end + context "#updated_dependencies_after_full_unlock" do let(:dependency_files) { project_dependency_files("npm6/no_lockfile") } let(:dependency) do diff --git a/python/lib/dependabot/python/update_checker.rb b/python/lib/dependabot/python/update_checker.rb index f2c9859e118..d377d40c6a2 100644 --- a/python/lib/dependabot/python/update_checker.rb +++ b/python/lib/dependabot/python/update_checker.rb @@ -78,6 +78,10 @@ def updated_requirements ).updated_requirements end + def requirements_unlocked_or_can_be? + requirements_update_strategy != :lockfile_only + end + def requirements_update_strategy # If passed in as an option (in the base class) honour that option return @requirements_update_strategy.to_sym if @requirements_update_strategy diff --git a/python/lib/dependabot/python/update_checker/requirements_updater.rb b/python/lib/dependabot/python/update_checker/requirements_updater.rb index 28289186121..d8f4f37487e 100644 --- a/python/lib/dependabot/python/update_checker/requirements_updater.rb +++ b/python/lib/dependabot/python/update_checker/requirements_updater.rb @@ -30,6 +30,8 @@ def initialize(requirements:, update_strategy:, has_lockfile:, end def updated_requirements + return requirements if update_strategy == :lockfile_only + requirements.map do |req| case req[:file] when /setup\.(?:py|cfg)$/ then updated_setup_requirement(req) diff --git a/python/spec/dependabot/python/update_checker/requirements_updater_spec.rb b/python/spec/dependabot/python/update_checker/requirements_updater_spec.rb index 34add947fec..68351cdfbb5 100644 --- a/python/spec/dependabot/python/update_checker/requirements_updater_spec.rb +++ b/python/spec/dependabot/python/update_checker/requirements_updater_spec.rb @@ -649,6 +649,14 @@ end end end + + context "when asked to not change requirements" do + let(:update_strategy) { :lockfile_only } + + it "does not update any requirements" do + expect(updated_requirements).to eq(requirements) + end + end end end end diff --git a/python/spec/dependabot/python/update_checker_spec.rb b/python/spec/dependabot/python/update_checker_spec.rb index 2bfa1487730..7ccd79947ae 100644 --- a/python/spec/dependabot/python/update_checker_spec.rb +++ b/python/spec/dependabot/python/update_checker_spec.rb @@ -21,7 +21,8 @@ credentials: credentials, ignored_versions: ignored_versions, raise_on_ignored: raise_on_ignored, - security_advisories: security_advisories + security_advisories: security_advisories, + requirements_update_strategy: requirements_update_strategy ) end let(:credentials) do @@ -35,6 +36,7 @@ let(:ignored_versions) { [] } let(:raise_on_ignored) { false } let(:security_advisories) { [] } + let(:requirements_update_strategy) { nil } let(:dependency_files) { [requirements_file] } let(:pipfile) do Dependabot::DependencyFile.new( @@ -751,4 +753,16 @@ end end end + + context "#requirements_unlocked_or_can_be?" do + subject { checker.requirements_unlocked_or_can_be? } + + it { is_expected.to eq(true) } + + context "with the lockfile-only requirements update strategy set" do + let(:requirements_update_strategy) { :lockfile_only } + + it { is_expected.to eq(false) } + end + end end diff --git a/updater/lib/dependabot/job.rb b/updater/lib/dependabot/job.rb index 5ccf203e194..dae37be71ce 100644 --- a/updater/lib/dependabot/job.rb +++ b/updater/lib/dependabot/job.rb @@ -94,11 +94,14 @@ def initialize(attributes) @existing_group_pull_requests = attributes.fetch(:existing_group_pull_requests, []) @experiments = attributes.fetch(:experiments, {}) @ignore_conditions = attributes.fetch(:ignore_conditions) - @lockfile_only = attributes.fetch(:lockfile_only) @package_manager = attributes.fetch(:package_manager) @reject_external_code = attributes.fetch(:reject_external_code, false) @repo_contents_path = attributes.fetch(:repo_contents_path, nil) - @requirements_update_strategy = attributes.fetch(:requirements_update_strategy) + + @requirements_update_strategy = build_update_strategy( + **attributes.slice(:requirements_update_strategy, :lockfile_only) + ) + @security_advisories = attributes.fetch(:security_advisories) @security_updates_only = attributes.fetch(:security_updates_only) @source = build_source(attributes.fetch(:source)) @@ -127,10 +130,6 @@ def repo_contents_path @repo_contents_path end - def lockfile_only? - @lockfile_only - end - def updating_a_pull_request? @updating_a_pull_request end @@ -306,6 +305,12 @@ def name_match?(name1, name2) ) end + def build_update_strategy(requirements_update_strategy:, lockfile_only:) + return requirements_update_strategy unless requirements_update_strategy.nil? + + lockfile_only ? "lockfile_only" : nil + end + def build_source(source_details) Dependabot::Source.new( **source_details.transform_keys { |k| k.tr("-", "_").to_sym } diff --git a/updater/lib/dependabot/updater/group_update_creation.rb b/updater/lib/dependabot/updater/group_update_creation.rb index 713dc7b054d..70fa86686a1 100644 --- a/updater/lib/dependabot/updater/group_update_creation.rb +++ b/updater/lib/dependabot/updater/group_update_creation.rb @@ -196,7 +196,7 @@ def all_versions_ignored?(dependency, checker) end def requirements_to_unlock(checker) - if job.lockfile_only? || !checker.requirements_unlocked_or_can_be? + if !checker.requirements_unlocked_or_can_be? if checker.can_update?(requirements_to_unlock: :none) then :none else :update_not_possible diff --git a/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb index b70b4876ffd..afe62a948e8 100644 --- a/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/create_security_update_pull_request.rb @@ -217,7 +217,7 @@ def existing_pull_request(updated_dependencies) end def requirements_to_unlock(checker) - if job.lockfile_only? || !checker.requirements_unlocked_or_can_be? + if !checker.requirements_unlocked_or_can_be? if checker.can_update?(requirements_to_unlock: :none) then :none else :update_not_possible diff --git a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb index b0d1414ff4d..3772828fb19 100644 --- a/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_security_update_pull_request.rb @@ -144,7 +144,7 @@ def check_and_update_pull_request(dependencies) # rubocop:enable Metrics/MethodLength def requirements_to_unlock(checker) - if job.lockfile_only? || !checker.requirements_unlocked_or_can_be? + if !checker.requirements_unlocked_or_can_be? if checker.can_update?(requirements_to_unlock: :none) then :none else :update_not_possible diff --git a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb index 72ccda674e5..1690ccecc7b 100644 --- a/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb +++ b/updater/lib/dependabot/updater/operations/refresh_version_update_pull_request.rb @@ -172,7 +172,7 @@ def all_versions_ignored?(dependency, checker) end def requirements_to_unlock(checker) - if job.lockfile_only? || !checker.requirements_unlocked_or_can_be? + if !checker.requirements_unlocked_or_can_be? if checker.can_update?(requirements_to_unlock: :none) then :none else :update_not_possible diff --git a/updater/lib/dependabot/updater/operations/update_all_versions.rb b/updater/lib/dependabot/updater/operations/update_all_versions.rb index e41f66f22f3..d13cb0f27d9 100644 --- a/updater/lib/dependabot/updater/operations/update_all_versions.rb +++ b/updater/lib/dependabot/updater/operations/update_all_versions.rb @@ -196,7 +196,7 @@ def existing_pull_request(updated_dependencies) end def requirements_to_unlock(checker) - if job.lockfile_only? || !checker.requirements_unlocked_or_can_be? + if !checker.requirements_unlocked_or_can_be? if checker.can_update?(requirements_to_unlock: :none) then :none else :update_not_possible diff --git a/updater/spec/dependabot/job_spec.rb b/updater/spec/dependabot/job_spec.rb index 92118cdfb4e..25744ee6350 100644 --- a/updater/spec/dependabot/job_spec.rb +++ b/updater/spec/dependabot/job_spec.rb @@ -32,7 +32,7 @@ "username" => "x-access-token", "password" => "github-token" }], - lockfile_only: false, + lockfile_only: lockfile_only, requirements_update_strategy: nil, update_subdependencies: false, updating_a_pull_request: false, @@ -47,6 +47,7 @@ let(:dependencies) { nil } let(:security_advisories) { [] } let(:package_manager) { "bundler" } + let(:lockfile_only) { false } let(:security_updates_only) { false } let(:allowed_updates) do [ @@ -94,6 +95,14 @@ end end + context "when lockfile_only is passed as true" do + let(:lockfile_only) { true } + + it "infers a lockfile_only requirements_update_strategy" do + expect(subject.requirements_update_strategy).to eq("lockfile_only") + end + end + describe "#allowed_update?" do subject { job.allowed_update?(dependency) } let(:dependency) do diff --git a/updater/spec/dependabot/updater_spec.rb b/updater/spec/dependabot/updater_spec.rb index b0bd48bb0c8..b7d86a5c7ef 100644 --- a/updater/spec/dependabot/updater_spec.rb +++ b/updater/spec/dependabot/updater_spec.rb @@ -206,6 +206,23 @@ end end + context "when lockfile_only is set in the job" do + it "still tries to unlock requirements of dependencies" do + checker = stub_update_checker + allow(checker).to receive(:requirements_unlocked_or_can_be?).and_return(true) + + job = build_job(lockfile_only: true) + service = build_service + updater = build_updater(service: service, job: job) + + expect(Dependabot.logger). + to receive(:info). + with("Requirements to unlock own") + + updater.run + end + end + context "when no dependencies are allowed" do it "logs the current and latest versions" do job = build_job( @@ -2342,7 +2359,8 @@ def build_service def build_job(requested_dependencies: nil, allowed_updates: default_allowed_updates, # rubocop:disable Metrics/MethodLength existing_pull_requests: [], ignore_conditions: [], security_advisories: [], - experiments: {}, updating_a_pull_request: false, security_updates_only: false, dependency_groups: []) + experiments: {}, updating_a_pull_request: false, security_updates_only: false, dependency_groups: [], + lockfile_only: false) Dependabot::Job.new( id: 1, token: "token", @@ -2372,7 +2390,7 @@ def build_job(requested_dependencies: nil, allowed_updates: default_allowed_upda "secret" => "codes" } ], - lockfile_only: false, + lockfile_only: lockfile_only, requirements_update_strategy: nil, update_subdependencies: false, updating_a_pull_request: updating_a_pull_request,