-
Notifications
You must be signed in to change notification settings - Fork 36
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
pyproject.toml: support PEP621 projects too #568
Conversation
@@ -14,6 +14,9 @@ class Pypi | |||
MANIFEST_REGEXP = /.*require[^\/]*(\/)?[^\/]*\.(txt|pip|in)$/ | |||
PIP_COMPILE_REGEXP = /.*require.*$/ | |||
|
|||
# Adapted from https://peps.python.org/pep-0508/#names | |||
PEP_508_NAME_REGEX = /^([A-Z0-9][A-Z0-9._-]*[A-Z0-9]|[A-Z0-9])/i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Parse poetry [tool.poetry] deps | ||
poetry_manifest = file_contents.fetch('tool', {}).fetch('poetry', {}) | ||
deps += map_dependencies(poetry_manifest['dependencies'], 'runtime') | ||
deps += map_dependencies(poetry_manifest['dev-dependencies'], 'develop') | ||
|
||
# Parse PEP621 [project] deps | ||
pep621_manifest = file_contents.fetch('project', {}) | ||
pep621_deps = pep621_manifest.fetch('dependencies', []).map { |d| parse_pep_508_dep_spec(d) } | ||
deps += map_dependencies(pep621_deps, 'runtime') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should these be mutually excluded or is it possible for a single manifest to specify both of these dependencies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PEP621 is the "tool-agnostic" way to declare build system requirements in pyproject.toml, and from what I understand Poetry is the only build system that doesn't support it yet. They're discussing it tho, and it looks like one of the examples includes both the "tool.poetry" And "project" namespaces for declaring deps: python-poetry/roadmap#3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yeah, just found an example here. The deps are the same in both sections, but still seems safer to combine them both until we have a reason not to: https://sourcegraph.com/github.com/IceTiki/ruoli-sign-optimization/-/blob/pyproject.toml?subtree=true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, does deps
need to be deduped? I'm not sure what clients of this method are expecting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yes! Good call, I think we do that higher up in Bibliothecary.analyse_file
but it does seem like some of the platform-specific parsers will uniq too.
pyproject.toml
can include poetry metadata, but it can also include project metadata according to PEP 621, under the[project]
field. This adds support for those[project.dependencies]
deps.