pylock: add filename property to PackageSdist and PackageWheel, more validation#1095
pylock: add filename property to PackageSdist and PackageWheel, more validation#1095brettcannon merged 3 commits intopypa:mainfrom
Conversation
| @property | ||
| def filename(self) -> str: | ||
| """Get the filename of the sdist.""" | ||
| filename = self.name or _url_name(self.url) or _path_name(self.path) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I would argue that name takes precedence like you wrote it.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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).
| elif "\\" in path: | ||
| return path.rsplit("\\", 1)[-1] | ||
| else: | ||
| return path |
There was a problem hiding this comment.
Here this is an heuristic to determine the name from the path, as the spec does not enforce path separators.
|
@brettcannon a few questions for you on the interpretation of PEP 751. |
| return package_wheel | ||
|
|
||
| @property | ||
| def filename(self) -> str: |
There was a problem hiding this comment.
I consider renaming this property to wheel_name. See also #1092 (comment).
- add filename property to PackageSdist and PackageWheel - validate sdist and wheel filenames
sbidoul
force-pushed
the
pylock-validate-wheel
branch
from
58728a0 to
27cbf4a
Compare
I think I answered everything, but in case I didn't just let me know! |
|
Thanks Brett. So this is ready for review. |
|
FYI I haven't forgotten about this PR. |
|
@sbidoul I know I already merged this, but I just realized around the ambiguity case when a name isn't explicitly provided but both a path and URL, "In the face of ambiguity, refuse the temptation to guess" and raise an ambiguity exception. |
|
@brettcannon that is more or less what I wanted to get at in #1095 (comment) I did not find how to handle such a case in the spec. My preference would be to handle that at validation time and reject lock files where |
Yeah, I forgot to cover that case. 😅 But the installation outline isn't a spec itself and since the overall spec doesn't address it, I would say it's undefined behaviour (but I'll open a discussion about it).
I think that's reasonable.
I would say if |
3 participants
To implement
Pylock.selectwe need a way to determine the filename of aPackageWheelfrom itsnameorurlorpath.So here I add a
filenameproperty toPackageWheelandPackageSdist.I also add validation of these names in
from_dict.