-
Notifications
You must be signed in to change notification settings - Fork 0
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
Support PEP 621 dependencies (pyproject.toml) #3
Conversation
metadata_please/pep621.py
Outdated
|
||
buf: list[str] = [] | ||
for dep in doc.get("project", {}).get("dependencies", ()): | ||
buf.append(f"Requires-Dist: {dep}\n") |
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.
Does this need a merge_markers(dep)
?
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.
Nope, in this case dep
already has embedded markers in its string, and we're not adding it in the context of an extra (that's the following couple lines).
metadata_please/tests/pep621.py
Outdated
dependencies = ["x"] | ||
|
||
[project.optional-dependencies] | ||
dev = ["Foo <= 2"] |
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.
How about adding a marker to this?
['Foo <= 2 ; platform_system != "Windows"']
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.
I added a "marker" dep as I was moving this between files, as well as adding a "y" dep with a marker re the comment above.
Currently this handles pep621, poetry, and setuptools in setup.cfg in a best-effort sort of way. Callers of this should check for nonempty deps to know whether it found anything. Because of the fragility with intreehooks this does not even look at the build-backend.
9c43515
to
271a5a6
Compare
|
||
# https://python-poetry.org/docs/dependency-specification/#version-constraints | ||
# 1.2.* type wildcards are supported natively in packaging | ||
if version.startswith("^"): |
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.
I assume poetry allows combining this like x = "^1.2.3,<1.5"
but I don't support that here.
version = _translate_caret(version) | ||
elif version.startswith("~"): | ||
version = _translate_tilde(version) | ||
elif version == "*": |
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.
I don't know if this can be combined with an operator, e.g. >=*
if python: | ||
m = OPERATOR_RE.fullmatch(python) | ||
assert m is not None | ||
# TODO do ^/~ work on python version? |
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.
I assume this goes through the same logic as generic version matching, so I should hoist the ^/~ handling out into something I can call here. Failing tests would be good.
for i in v: | ||
buf.append("Requires-Dist: " + merge_extra_marker(extra_name, i) + "\n") | ||
|
||
return "".join(buf).encode("utf-8") |
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.
It looks like tilde is allowed by standard python packaging. https://packaging.python.org/en/latest/specifications/version-specifiers/#examples
Do we need to convert any tildes in the pep621 into less than/greater than operators?
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.
No, but it probably means the poetry translation could s/~/~=/
if they use the same rules. I intend to revisit how the translation happens next big block of travel time I have.
Fixes #2