Update Pypi.dependencies after update to package manager api #3040
Conversation
| @@ -123,14 +123,14 @@ def self.known_versions(name) | |||
| &.index_by { |v| v[:number] } || {} | |||
| end | |||
|
|
|||
| def self.dependencies(name, version, _mapped_project) | |||
| def self.dependencies(name, version, _mapped_project = nil) | |||
There was a problem hiding this comment.
Not sure we need this change. Is something calling this method without passing in the third argument?
app/models/package_manager/pypi.rb
Outdated
| Rails.logger.warn("Pypi sdist (no deps): #{name}") unless source_info.any? { |rel| rel["packagetype"] == "bdist_wheel" } | ||
|
|
||
| deps.map do |dep| | ||
| name, version = dep.split | ||
| name, version = dep.split(/ /, 2) |
There was a problem hiding this comment.
What was the reason for this change?
There was a problem hiding this comment.
TLDR: before we were saying definitively that a package had a certain number of dependencies, even if some of those dependencies only came with variants of the package. Now we include the information specifying which ones are only included with a variant.
There was a problem hiding this comment.
I wonder if it's better to split on the semi-colon? Feeling slightly nervous about splitting on a whitespace (unless pypi is really strict about whitespaces in ranges) nevermind, I misread the tests at first!
There was a problem hiding this comment.
Allegedly (according to this StackOverflow post), these dependencies are in the PEP 508 format. I can't find an official PyPI source stating this though.
Looking at the tests in the grammar section, there are a couple of instances of valid PEP 508 dependencies where the part after a space would not specify a version, e.g.
name[quux, strange];python_version<'2.7' and platform_version=='2'
=> ["name[quux,", "strange];python_version<'2.7'"and platform_version=='2'"]
That's not the point of this change, but it's possible that the current (and this) implementation would create bad data
There was a problem hiding this comment.
That being said we should probably file something else to address it
There was a problem hiding this comment.
oof...
According to this, we should be able to use this regex to parse the name: ^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9]). I suppose the rest of it we could provide in requirements?
Edit: Please make sure I'm understanding that right. I think when they say "names" they're talking about package names... I'm not 100% sure though.
I noticed it would mess with dependency specifications like this: |
papiro
deleted the
pp/sc-29698/dependencies-not-showing-up-in-release-api
branch
This is fixing Pypi dependencies after this update: pypi/warehouse#11775 which broke the existing implementation.