Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when updating Python libraries with multiple manifest types #5932

Merged
merged 3 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 33 additions & 16 deletions python/lib/dependabot/python/update_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -202,16 +201,14 @@ def resolver_args
end

def current_requirement_string
reqs = dependency.requirements
reqs = 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
Expand All @@ -236,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) }.
Expand All @@ -262,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
Expand All @@ -283,6 +280,26 @@ def poetry_library?
false
end

def updating_pipfile?
requirement_files.any?("Pipfile")
end

def updating_pyproject?
pavera marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
78 changes: 46 additions & 32 deletions python/spec/dependabot/python/update_checker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -76,6 +76,8 @@
}]
end

let(:dependency) { requirements_dependency }

describe "#can_update?" do
subject { checker.can_update?(requirements_to_unlock: :own) }

Expand Down Expand Up @@ -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

Expand Down