Skip to content

pylock: add filename property to PackageSdist and PackageWheel, more validation #1095

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.

Sign up for GitHub

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

Merged
brettcannon merged 3 commits into from
Mar 6, 2026
+236 −2
Merged

pylock: add filename property to PackageSdist and PackageWheel, more validation #1095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
27cbf4a
pylock: add filename property and more validation
sbidoul Feb 21, 2026
0537ad5
Merge branch 'main' into pylock-validate-wheel
brettcannon Feb 24, 2026
e23ae76
Merge branch 'main' into pylock-validate-wheel
brettcannon Mar 6, 2026
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
56 changes: 55 additions & 1 deletion src/packaging/pylock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,16 @@
Protocol,
TypeVar,
)
from urllib.parse import urlparse

from .markers import Marker
from .specifiers import SpecifierSet
from .utils import NormalizedName, is_normalized_name
from .utils import (
NormalizedName,
is_normalized_name,
parse_sdist_filename,
parse_wheel_filename,
)
from .version import Version

if TYPE_CHECKING: # pragma: no cover
Expand Down Expand Up @@ -227,6 +233,26 @@ def _validate_path_url(path: str | None, url: str | None) -> None:
raise PylockValidationError("path or url must be provided")


def _path_name(path: str | None) -> str | None:
if not path:
return None
# If the path is relative it MAY use POSIX-style path separators explicitly
# for portability
if "/" in path:
return path.rsplit("/", 1)[-1]
elif "\\" in path:
return path.rsplit("\\", 1)[-1]
else:
return path
Copy link
Member Author

@sbidoul sbidoul Feb 21, 2026

Choose a reason for hiding this comment

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

Here this is an heuristic to determine the name from the path, as the spec does not enforce path separators.



def _url_name(url: str | None) -> str | None:
if not url:
return None
url_path = urlparse(url).path
return url_path.rsplit("/", 1)[-1]


def _validate_hashes(hashes: Mapping[str, Any]) -> Mapping[str, Any]:
if not hashes:
raise PylockValidationError("At least one hash must be provided")
Expand Down Expand Up @@ -421,8 +447,22 @@ def _from_dict(cls, d: Mapping[str, Any]) -> Self:
hashes=_get_required_as(d, Mapping, _validate_hashes, "hashes"), # type: ignore[type-abstract]
)
_validate_path_url(package_sdist.path, package_sdist.url)
try:
parse_sdist_filename(package_sdist.filename)
except Exception as e:
raise PylockValidationError(
f"Invalid sdist filename {package_sdist.filename!r}"
) from e
return package_sdist

@property
def filename(self) -> str:
"""Get the filename of the sdist."""
filename = self.name or _url_name(self.url) or _path_name(self.path)
Copy link
Member Author

@sbidoul sbidoul Feb 21, 2026

Choose a reason for hiding this comment

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

The spec allows the 3 values to be set and does not explicitly say they must be consistent. Here I chose to not check for consistency and use this priority order to select the name.

Copy link
Member

@brettcannon brettcannon Feb 23, 2026

Choose a reason for hiding this comment

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

I would argue that name takes precedence like you wrote it.

Copy link

@jsirois jsirois Mar 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

That said, shouldn't URL have least precedence since the URL of a package may have a path that has 0 to do with the name? See pypa/packaging.python.org#1863 for discussion of this.

Copy link
Member Author

@sbidoul sbidoul Mar 11, 2026

Choose a reason for hiding this comment

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

@jsirois the current idea (see the discourse thread that Brett opened) is that name is authoritative if set and if not set, path and url must have the same basename (validation check implemented in #1117).

Copy link
Member

@pfmoore pfmoore Mar 12, 2026

Choose a reason for hiding this comment

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

For sdists and wheels, the spec says (item 6, last 2 bullet points) that path must be preferred over url. There's no rule given for other package types - it seems reasonable to me to apply the same rule, but someone could argue that you don't have to in that case.

So if this is just for sdist and wheel types, then IMO the spec is explicit - name first, then path, then url. Choose the first one that exists.

Copy link
Member Author

@sbidoul sbidoul Mar 12, 2026

Choose a reason for hiding this comment

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

Yeah that's fine with me too, but Brett said (#1095 (comment)) that the installation section was not normative and he would prefer to not guess.

Copy link
Member

@pfmoore pfmoore Mar 12, 2026

Choose a reason for hiding this comment

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

Ah, OK - I missed that comment from Brett.

I think I'd prefer to simply make that aspect (the priority between the fields) of the spec normative rather than try to create new consistency rules.

if not filename:
raise PylockValidationError("Cannot determine sdist filename")
return filename


@dataclass(frozen=True, init=False)
class PackageWheel:
Expand Down Expand Up @@ -462,8 +502,22 @@ def _from_dict(cls, d: Mapping[str, Any]) -> Self:
hashes=_get_required_as(d, Mapping, _validate_hashes, "hashes"), # type: ignore[type-abstract]
)
_validate_path_url(package_wheel.path, package_wheel.url)
try:
parse_wheel_filename(package_wheel.filename)
except Exception as e:
raise PylockValidationError(
f"Invalid wheel filename {package_wheel.filename!r}"
) from e
return package_wheel

@property
def filename(self) -> str:
Copy link
Member Author

@sbidoul sbidoul Feb 21, 2026

Choose a reason for hiding this comment

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

I consider renaming this property to wheel_name. See also #1092 (comment).

Copy link
Member Author

@sbidoul sbidoul Feb 24, 2026

Choose a reason for hiding this comment

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

Or not.

"""Get the filename of the wheel."""
filename = self.name or _url_name(self.url) or _path_name(self.path)
if not filename:
raise PylockValidationError("Cannot determine wheel filename")
return filename


@dataclass(frozen=True, init=False)
class Package:
Expand Down
182 changes: 181 additions & 1 deletion tests/test_pylock.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from packaging.pylock import (
Package,
PackageDirectory,
PackageSdist,
PackageVcs,
PackageWheel,
Pylock,
Expand Down Expand Up @@ -156,7 +157,7 @@ def test_pylock_packages_with_dist_and_archive() -> None:
"hashes": {"sha256": "f" * 40},
},
"sdist": {
"path": "example.tar.gz",
"path": "example-1.0.tar.gz",
"hashes": {"sha256": "f" * 40},
},
}
Expand Down Expand Up @@ -280,6 +281,185 @@ def test_pylock_invalid_vcs() -> None:
assert str(exc_info.value) == "path or url must be provided"


@pytest.mark.parametrize(
("dist", "expected_filename"),
[
# sdists
(
PackageSdist(
name="example-1.0.tar.gz",
hashes={},
),
"example-1.0.tar.gz",
),
(
PackageSdist(
path="./example-1.0.tar.gz",
hashes={},
),
"example-1.0.tar.gz",
),
(
PackageSdist(
path=".\\example-1.0.tar.gz",
hashes={},
),
"example-1.0.tar.gz",
),
(
PackageSdist(
url="https://example.com/example-1.0.tar.gz",
hashes={},
),
"example-1.0.tar.gz",
),
(
# name preferred over path
PackageSdist(
name="example-2.0.tar.gz",
path=".\\example-1.0.tar.gz",
hashes={},
),
"example-2.0.tar.gz",
),
(
# name preferred over url
PackageSdist(
name="example-2.0.tar.gz",
url="https://example.com/example-1.0.tar.gz",
hashes={},
),
"example-2.0.tar.gz",
),
(
# url preferred over path
PackageSdist(
url="https://example.com/example-2.0.tar.gz",
path="./example-1.0.tar.gz",
hashes={},
),
"example-2.0.tar.gz",
),
# wheels
(
PackageWheel(
name="example-1.0-py3-none-any.whl",
hashes={},
),
"example-1.0-py3-none-any.whl",
),
(
PackageWheel(
path="./example-1.0-py3-none-any.whl",
hashes={},
),
"example-1.0-py3-none-any.whl",
),
(
PackageWheel(
path=".\\example-1.0-py3-none-any.whl",
hashes={},
),
"example-1.0-py3-none-any.whl",
),
(
PackageWheel(
url="https://example.com/example-1.0-py3-none-any.whl",
hashes={},
),
"example-1.0-py3-none-any.whl",
),
(
# name preferred over path
PackageWheel(
name="example-2.0-py3-none-any.whl",
path=".\\example-1.0-py3-none-any.whl",
hashes={},
),
"example-2.0-py3-none-any.whl",
),
(
# name preferred over url
PackageWheel(
name="example-2.0-py3-none-any.whl",
url="https://example.com/example-1.0-py3-none-any.whl",
hashes={},
),
"example-2.0-py3-none-any.whl",
),
(
# url preferred over path
PackageWheel(
url="https://example.com/example-2.0-py3-none-any.whl",
path="./example-1.0-py3-none-any.whl",
hashes={},
),
"example-2.0-py3-none-any.whl",
),
],
)
def test_dist_filename(
dist: PackageSdist | PackageWheel, expected_filename: str
) -> None:
assert dist.filename == expected_filename


def test_missing_sdist_filename() -> None:
with pytest.raises(PylockValidationError) as exc_info:
_ = PackageSdist(hashes={}).filename
assert str(exc_info.value) == "Cannot determine sdist filename"


def test_missing_wheel_filename() -> None:
with pytest.raises(PylockValidationError) as exc_info:
_ = PackageWheel(hashes={}).filename
assert str(exc_info.value) == "Cannot determine wheel filename"


def test_pylock_invalid_wheel_filename() -> None:
data = {
"lock-version": "1.0",
"created-by": "pip",
"packages": [
{
"name": "example",
"wheels": [
{
"url": "http://example.com/example-1.0.whl",
"hashes": {"sha256": "f" * 40},
}
],
}
],
}
with pytest.raises(PylockValidationError) as exc_info:
Pylock.from_dict(data)
assert str(exc_info.value) == (
"Invalid wheel filename 'example-1.0.whl' in 'packages[0].wheels[0]'"
)


def test_pylock_invalid_sdist_filename() -> None:
data = {
"lock-version": "1.0",
"created-by": "pip",
"packages": [
{
"name": "example",
"sdist": {
"path": "./example.tar.gz",
"hashes": {"sha256": "f" * 40},
},
},
],
}
with pytest.raises(PylockValidationError) as exc_info:
Pylock.from_dict(data)
assert str(exc_info.value) == (
"Invalid sdist filename 'example.tar.gz' in 'packages[0].sdist'"
)


def test_pylock_invalid_wheel() -> None:
data = {
"lock-version": "1.0",
Expand Down