Tighten and simplify extraction of metadata from wheels #1201
Conversation
dnicolodi
force-pushed
the
wheel-cleanup
branch
9 times, most recently
from
09a1278 to
1f07e33
Compare
|
|
||
| def read(self) -> bytes: | ||
| fqn = os.path.abspath(os.path.normpath(self.filename)) |
There was a problem hiding this comment.
It seems we lose this if we just accept filename as passed into the Wheel object, no?
There was a problem hiding this comment.
This is intended. First, os.path.abspath(path) is equivalent to os.path.normpath(os.getcwd(), path) thus normalizing with normpath() before calling abspath() is redundant. Then, normpath() operates only on the representation of the path and does funny things with symbolic links. Most likely corner cases, but making corner cases more confusing is not making them easier to diagnose. Third, I like the error messages to refer to the path as provided by the user and not to a "normalized" path.
TL;DR: I didn't understand why this was a good idea.
|
|
||
| def read(self) -> bytes: | ||
| fqn = os.path.abspath(os.path.normpath(self.filename)) | ||
| if not os.path.exists(fqn): | ||
| raise exceptions.InvalidDistribution("No such file: %s" % fqn) |
There was a problem hiding this comment.
We're also losing the clarity from this exception if the give file/path does not exist?
There was a problem hiding this comment.
The same check existed in the class constructor and in the read() method. I just removed one copy of it. Technically the check in the constructor checks the path as provided by the user, and here the check is on the normalized path, but if the two disagree something went wrong with the normalization (see argument about symlinks above) and it is a bug and not a feature. The SDist class does the check in the constructor, so I kept the check in the constructor here too.
There was a problem hiding this comment.
Now the Wheel class can be used to parse the wheel file name into components (name, version, pyver, and the others could be added, if needed), and that does not depend on accessing the file, thus it could be argued that keeping the file existence check in the read() method would be better. I don't have a preference.
Drop the .dist-info file extension. Enforce that each component of the filename does not contain a dash: dashes are used as separators. See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#file-name-convention Remove spurious grouping, while at it.
The location of the METADATA member in the wheel archive is well defined. It is not necessary nor desirable look for it elsewhere. See https://packaging.python.org/en/latest/specifications/binary-distribution-format/#file-contents This simplifies the implementation and is more correct. Modernize the code a bit, while at it.
dnicolodi
force-pushed
the
wheel-cleanup
branch
from
1f07e33 to
d729c25
Compare
No description provided.