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

pyproject.toml: support PEP621 projects too #568

Merged
merged 7 commits into from
Feb 6, 2023

Conversation

tiegz
Copy link
Contributor

@tiegz tiegz commented Feb 3, 2023

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.

spec/parsers/pypi_spec.rb Outdated Show resolved Hide resolved
@@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lib/bibliothecary/parsers/pypi.rb Outdated Show resolved Hide resolved
Comment on lines +101 to +109
# 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')
Copy link
Contributor

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?

Copy link
Contributor Author

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

Copy link
Contributor Author

@tiegz tiegz Feb 6, 2023

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

Copy link
Contributor

@mpace965 mpace965 Feb 6, 2023

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

Copy link
Contributor Author

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.

@tiegz tiegz merged commit 8dc44e5 into main Feb 6, 2023
@tiegz tiegz deleted the tz/support-pep621-in-pyproject-toml branch February 6, 2023 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants