-
Notifications
You must be signed in to change notification settings - Fork 124
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
Add more helpful error message when metadata is malformed #695
base: main
Are you sure you want to change the base?
Conversation
0fd1d70
to
c7d4233
Compare
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.
Please make the CI pass 😊
74c60a4
to
5a388a7
Compare
5a388a7
to
09f97cf
Compare
@@ -21,7 +21,7 @@ | |||
|
|||
DEFAULT_BACKEND = { | |||
'build-backend': 'setuptools.build_meta:__legacy__', | |||
'requires': ['setuptools >= 40.8.0'], | |||
'requires': ['setuptools >= 40.8.0', 'wheel'], |
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.
This seems like a regression. Why is it needed? wheel
was required on older setuptools
versions, but shouldn't be anymore. If our minimum setuptools
version does not yet include the fix to return wheel
in get_requires_for_build_wheel
, we should update it.
# Malformed dist-info or some other form of metadata corruption | ||
# req.specifier.contains will raise TypeError, so let's do the same | ||
# with a more helpful error message | ||
msg = f'Package {req.name} has malformed metadata and no version information could be found' |
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.
nitpick: so that the name is quoted:
msg = f'Package {req.name} has malformed metadata and no version information could be found' | |
msg = f'Package {req.name!r} has malformed metadata and no version information could be found' |
# req.specifier.contains will raise TypeError, so let's do the same | ||
# with a more helpful error message | ||
msg = f'Package {req.name} has malformed metadata and no version information could be found' | ||
raise TypeError(msg) |
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 wonder if we shouldn't make this a warning instead and return this package as an unmet dependency. That should give more information to the user. As it stands, they would only see an error message about a specific bad dependency, instead of getting a list of all bad ones with the extra context of why this one was considered bad.
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.
Seems fine to me, sounds reasonable to want all issues at the same time instead of potentially having to address one-by-one
xref #558 #626