Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
65 changes: 49 additions & 16 deletions metadata_please/source_checkout.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from configparser import NoOptionError, NoSectionError, RawConfigParser

from packaging.utils import canonicalize_name
from packaging.version import Version

from .source_checkout_ast import SetupFindingVisitor, UNKNOWN

Expand Down Expand Up @@ -139,22 +140,35 @@ def _translate_caret(specifier: str) -> str:
Given a string like "^0.2.3" returns ">=0.2.3,<0.3.0".
"""
assert "," not in specifier
parts = specifier[1:].split(".")
while len(parts) < 3:
parts.append("0")

for i in range(len(parts)):
if parts[i] != "0":
# The docs are not super clear about how this behaves, but let's
# assume integer-valued parts and just let the exception raise
# otherwise.
incremented = parts[:]
incremented[i] = str(int(parts[i]) + 1)
del incremented[i + 1 :]
incremented_version = ".".join(incremented)
break

version = Version(specifier[1:])
version_parts = list(version.release)
if version.is_prerelease or version.is_postrelease:
# Return next version, incrementing the least significant number
version_parts[-1] += 1

else:
raise ValueError("All components were zero?")
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a test with ^0 and make sure it does something predictable? I think it's invalid, and I don't know if it will raise (which is fine) or do something less useful like >=0,<1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That case would return >=0,<1. Similarly, ^0.0 returns >=0.0,<1.0 and ^0.0.0 returns >=0.0.0,<1.0.0. Is it better to raise?

Copy link
Member

Choose a reason for hiding this comment

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

There are two examples given at the bottom of https://python-poetry.org/docs/dependency-specification/#caret-requirements that are the best spec I can find. My personal read of

An update is allowed if the new version number does not modify the left-most non-zero digit in the major, minor, patch grouping.

is that this isn't a case they've really designed for. I'd think ^0.0.x is compatible with 0.0.xpost1 for example, but the prose implies that isn't the case.

I'm just saying, please add tests for the zero case if it returns now instead of raising.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a check for this case that raises a ValueError!

if version.major > 0 or version.minor == 0:
# Next major version
version_parts[0] += 1

# Set the rest of the values to 0
for i in range(1, len(version_parts)):
version_parts[i] = 0

elif version.minor > 0 or version.micro == 0:
# Next minor version.
version_parts[1] += 1

# Set the rest of the values to 0
for i in range(2, len(version_parts)):
version_parts[i] = 0

else:
# Next patch version
version_parts[2] += 1

incremented_version = ".".join([str(i) for i in version_parts])
return f">={specifier[1:]},<{incremented_version}"


Expand Down Expand Up @@ -213,7 +227,26 @@ def from_poetry_checkout(path: Path) -> bytes:
optional = False

if not version:
# e.g. git, path or url dependencies, skip for now
# e.g. git, path or url dependencies
if "path" in v:
Copy link
Member

Choose a reason for hiding this comment

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

Can you split this change out to a separate PR? This needs more thought and probably some sort of X- prefixed name, as I'm pretty sure they are not valid Requires-Dist.

buf.append(f"Requires-Dist: {v['path']}\n")

elif "url" in v:
buf.append(f"Requires-Dist: {v['url']}\n")

elif "git" in v:
git_link = f"git+{v['git']}"

# from both poetry and pypa docs, seems like only one of the following should be specified
revision = v.get("rev") or v.get("tag") or v.get("branch")
if revision:
git_link += f"@{revision}"

if "subdirectory" in v:
git_link += f"#subdirectory={v['subdirectory']}"

buf.append(f"Requires-Dist: {k} @ {git_link}\n")

continue

# https://python-poetry.org/docs/dependency-specification/#version-constraints
Expand Down
15 changes: 11 additions & 4 deletions metadata_please/tests/source_checkout.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,15 @@ def test_poetry_full(self) -> None:
a2 = "*"
b = "^1.2.3"
b2 = "^0.2.3"
b3 = "^0.44b0"
c = "~1.2.3"
c2 = "~1.2"
c3 = "~1"
d = {version="2", python="<3.11"}
e = {version="2", markers="sys_platform == 'darwin'"}
skipped = {git = "..."}
skipped = {git = "...", tag = "12345"}
my-package = { url = "https://example.com/my-package-0.1.0.tar.gz" }
my-other-package = { path = "../my-package/dist/my-other-package-0.1.0.tar.gz" }
complex = {extras=["bar", "baz"], version="2"}
opt = { version = "^2.9", optional = true}
unused-extra = { version = "2", optional = true }
Expand All @@ -124,15 +127,19 @@ def test_poetry_full(self) -> None:
[
"a==1.0",
"a2",
"b>=1.2.3,<2",
"b2>=0.2.3,<0.3",
"b>=1.2.3,<2.0.0",
"b2>=0.2.3,<0.3.0",
"b3>=0.44b0,<0.45",
"c>=1.2.3,<1.3",
"c2>=1.2,<1.3",
"c3>=1,<2",
"d==2 ; python_version < '3.11'",
"e==2 ; sys_platform == 'darwin'",
"skipped @ git+...@12345",
"https://example.com/my-package-0.1.0.tar.gz",
"../my-package/dist/my-other-package-0.1.0.tar.gz",
"complex[bar,baz]==2",
'opt>=2.9,<3 ; extra == "foo"',
'opt>=2.9,<3.0 ; extra == "foo"',
],
rv.reqs,
)
Expand Down
Loading