From 4e096e8eca4a81711f8afe32e47073fb54c306a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net>
Date: Tue, 18 Oct 2022 22:52:45 +0200
Subject: [PATCH 1/3] Refactor overly complicated update checker logic
---
python/lib/dependabot/python/update_checker.rb | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/python/lib/dependabot/python/update_checker.rb b/python/lib/dependabot/python/update_checker.rb
index 55a0747ba05..2bd80d504f7 100644
--- a/python/lib/dependabot/python/update_checker.rb
+++ b/python/lib/dependabot/python/update_checker.rb
@@ -205,13 +205,11 @@ def current_requirement_string
reqs = dependency.requirements
return if reqs.none?
- requirement =
- case resolver_type
- when :pipenv then reqs.find { |r| r[:file] == "Pipfile" }
- when :poetry then reqs.find { |r| r[:file] == "pyproject.toml" }
- when :pip_compile then reqs.find { |r| r[:file].end_with?(".in") }
- when :requirements then reqs.find { |r| r[:file].end_with?(".txt") }
- end
+ requirement = reqs.find do |r|
+ file = r[:file]
+
+ file == "Pipfile" || file == "pyproject.toml" || file.end_with?(".in") || file.end_with?(".txt")
+ end
requirement&.fetch(:requirement)
end
From ebfc87990bc29a0d87d5ccee76f4f41ec84e6853 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net>
Date: Tue, 18 Oct 2022 22:48:00 +0200
Subject: [PATCH 2/3] Extract some helpers for readability and later reuse
---
.../lib/dependabot/python/update_checker.rb | 35 ++++++++++++++-----
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/python/lib/dependabot/python/update_checker.rb b/python/lib/dependabot/python/update_checker.rb
index 2bd80d504f7..06b23a2eb90 100644
--- a/python/lib/dependabot/python/update_checker.rb
+++ b/python/lib/dependabot/python/update_checker.rb
@@ -89,7 +89,7 @@ def lowest_resolvable_security_fix_version
def updated_requirements
RequirementsUpdater.new(
- requirements: dependency.requirements,
+ requirements: requirements,
latest_resolvable_version: preferred_resolvable_version&.to_s,
update_strategy: requirements_update_strategy,
has_lockfile: !(pipfile_lock || poetry_lock || pyproject_lock).nil?
@@ -133,8 +133,7 @@ def fetch_lowest_resolvable_security_fix_version
end
def resolver_type
- reqs = dependency.requirements
- req_files = reqs.map { |r| r.fetch(:file) }
+ reqs = requirements
# If there are no requirements then this is a sub-dependency. It
# must come from one of Pipenv, Poetry or pip-tools, and can't come
@@ -143,9 +142,9 @@ def resolver_type
# Otherwise, this is a top-level dependency, and we can figure out
# which resolver to use based on the filename of its requirements
- return :pipenv if req_files.any?("Pipfile")
- return :poetry if req_files.any?("pyproject.toml")
- return :pip_compile if req_files.any? { |f| f.end_with?(".in") }
+ return :pipenv if updating_pipfile?
+ return :poetry if updating_pyproject?
+ return :pip_compile if updating_in_file?
if dependency.version && !exact_requirement?(reqs)
subdependency_resolver
@@ -202,7 +201,7 @@ def resolver_args
end
def current_requirement_string
- reqs = dependency.requirements
+ reqs = requirements
return if reqs.none?
requirement = reqs.find do |r|
@@ -234,7 +233,7 @@ def updated_version_req_lower_bound
return ">= #{dependency.version}" if dependency.version
version_for_requirement =
- dependency.requirements.filter_map { |r| r[:requirement] }.
+ requirements.filter_map { |r| r[:requirement] }.
reject { |req_string| req_string.start_with?("<") }.
select { |req_string| req_string.match?(VERSION_REGEX) }.
map { |req_string| req_string.match(VERSION_REGEX) }.
@@ -281,6 +280,26 @@ def poetry_library?
false
end
+ def updating_pipfile?
+ requirement_files.any?("Pipfile")
+ end
+
+ def updating_pyproject?
+ requirement_files.any?("pyproject.toml")
+ end
+
+ def updating_in_file?
+ requirement_files.any? { |f| f.end_with?(".in") }
+ end
+
+ def requirement_files
+ requirements.map { |r| r.fetch(:file) }
+ end
+
+ def requirements
+ dependency.requirements
+ end
+
def normalised_name(name)
NameNormaliser.normalise(name)
end
From 7d91b710a4cb928f784cee066783eeb7467a19ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@riseup.net>
Date: Thu, 20 Oct 2022 02:15:31 +0200
Subject: [PATCH 3/3] Fix crash when updating libraries with multiple manifests
If a library has both a pyproject.toml file and a standard
requirements.txt file, we'd end up using the `:widen` strategy for the
dependencies in the `requirements.txt` file and eventually crashing with
an error like the following:
```
/home/dependabot/dependabot-core/common/lib/dependabot/update_checkers/base.rb:266:in `block in preferred_version_resolvable_with_unlock?': undefined method `[]' for nil:NilClass (NoMethodError)
updated_requirements.none? { |r| r[:requirement] == :unfixable }
^^^^^^^^^^^^^^
from /home/dependabot/dependabot-core/common/lib/dependabot/update_checkers/base.rb:266:in `none?'
from /home/dependabot/dependabot-core/common/lib/dependabot/update_checkers/base.rb:266:in `preferred_version_resolvable_with_unlock?'
from /home/dependabot/dependabot-core/common/lib/dependabot/update_checkers/base.rb:249:in `numeric_version_can_update?'
from /home/dependabot/dependabot-core/common/lib/dependabot/update_checkers/base.rb:199:in `version_can_update?'
from /home/dependabot/dependabot-core/common/lib/dependabot/update_checkers/base.rb:44:in `can_update?'
from bin/dry-run.rb:709:in `block in <main>'
from bin/dry-run.rb:661:in `each'
from bin/dry-run.rb:661:in `<main>'
```
I think the crash happens because the requirements.txt file updater does
not supoort the `:widen` strategy. So my fix is to fallback to
`increase` in this case, since requirements.txt files usually include
pinned dependencies so widening probably doesn't make much sense there.
---
.../lib/dependabot/python/update_checker.rb | 2 +-
.../dependabot/python/update_checker_spec.rb | 78 +++++++++++--------
2 files changed, 47 insertions(+), 33 deletions(-)
diff --git a/python/lib/dependabot/python/update_checker.rb b/python/lib/dependabot/python/update_checker.rb
index 06b23a2eb90..91628470684 100644
--- a/python/lib/dependabot/python/update_checker.rb
+++ b/python/lib/dependabot/python/update_checker.rb
@@ -259,7 +259,7 @@ def latest_version_finder
end
def poetry_library?
- return false unless pyproject
+ return false unless updating_pyproject?
# Hit PyPi and check whether there are details for a library with a
# matching name and description
diff --git a/python/spec/dependabot/python/update_checker_spec.rb b/python/spec/dependabot/python/update_checker_spec.rb
index 447a44a1f8e..50590b1c039 100644
--- a/python/spec/dependabot/python/update_checker_spec.rb
+++ b/python/spec/dependabot/python/update_checker_spec.rb
@@ -57,7 +57,7 @@
)
end
let(:requirements_fixture_name) { "version_specified.txt" }
- let(:dependency) do
+ let(:requirements_dependency) do
Dependabot::Dependency.new(
name: dependency_name,
version: dependency_version,
@@ -76,6 +76,8 @@
}]
end
+ let(:dependency) { requirements_dependency }
+
describe "#can_update?" do
subject { checker.can_update?(requirements_to_unlock: :own) }
@@ -507,44 +509,56 @@
let(:dependency_files) { [requirements_file, pyproject] }
let(:pyproject_fixture_name) { "caret_version.toml" }
- let(:dependency) do
- Dependabot::Dependency.new(
- name: "requests",
- version: "1.2.3",
- requirements: [{
- file: "pyproject.toml",
- requirement: "^1.0.0",
- groups: [],
- source: nil
- }],
- package_manager: "pip"
- )
- end
+ context "and updating a dependency inside" do
+ let(:dependency) do
+ Dependabot::Dependency.new(
+ name: "requests",
+ version: "1.2.3",
+ requirements: [{
+ file: "pyproject.toml",
+ requirement: "^1.0.0",
+ groups: [],
+ source: nil
+ }],
+ package_manager: "pip"
+ )
+ end
- let(:pypi_url) { "https://pypi.org/simple/requests/" }
- let(:pypi_response) do
- fixture("pypi", "pypi_simple_response_requests.html")
- end
+ let(:pypi_url) { "https://pypi.org/simple/requests/" }
+ let(:pypi_response) do
+ fixture("pypi", "pypi_simple_response_requests.html")
+ end
- context "for a library" do
- before do
- stub_request(:get, "https://pypi.org/pypi/pendulum/json/").
- to_return(
- status: 200,
- body: fixture("pypi", "pypi_response_pendulum.json")
- )
+ context "for a library" do
+ before do
+ stub_request(:get, "https://pypi.org/pypi/pendulum/json/").
+ to_return(
+ status: 200,
+ body: fixture("pypi", "pypi_response_pendulum.json")
+ )
+ end
+
+ its([:requirement]) { is_expected.to eq(">=1,<3") }
end
- its([:requirement]) { is_expected.to eq(">=1,<3") }
- end
+ context "for a non-library" do
+ before do
+ stub_request(:get, "https://pypi.org/pypi/pendulum/json/").
+ to_return(status: 404)
+ end
- context "for a non-library" do
- before do
- stub_request(:get, "https://pypi.org/pypi/pendulum/json/").
- to_return(status: 404)
+ its([:requirement]) { is_expected.to eq("^2.19.1") }
end
+ end
+
+ context "and updating a dependency in an additional requirements file" do
+ let(:dependency_files) { super().append(requirements_file) }
- its([:requirement]) { is_expected.to eq("^2.19.1") }
+ let(:dependency) { requirements_dependency }
+
+ it "does not get affected by whether it's a library or not and updates using the :increase strategy" do
+ expect(subject[:requirement]).to eq("==2.6.0")
+ end
end
end