-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Provide a better error when setup.py is missing #1710
Comments
I think this is probably already fixed in master. PR #1675 has added support for |
This issue is to provide a better error message in the case that the needed files are missing though (setup.py or I guess now setup.cfg). Is that also addressed? It didn’t look like the missing file(s) case was handled any differently in that PR, though I could be wrong. |
@cjerdonek I'm not sure you'll actually get an error if both Testing it out with the current
It seems like that should probably throw an error because required metadata is missing, like the name of the package. Not sure if requiring a valid |
Since the new feature is only in master, wouldn't this already be breaking because of |
Or oh, are you just making a point as to where the error should / would need to happen in the case of this issue? |
I just mean that neither from setuptools import setup; setup() Right now that's not an error, though I can't imagine anyone is doing it deliberately. Worst case scenario we could require that you either have a |
Would this change need to happen before you make a new release, and is it a release blocker? (It seems like it would be better to make the change beforehand so that it won't become a breaking change post-release.)
If the error handling is done this way, it would still be good to somehow include in the message that the file is missing (in the case they didn't provide a file), so that it's clearer to the user what the corrective action is. If the error just says something about "invalid metadata," it wouldn't necessarily be obvious if it's because they're missing a setup.py, etc. Ideally the error message would know whether a file was originally present (as opposed to an auto-generated one). |
One more comment: this is reminiscent of issue #1664 I submitted a PR for (#1706) where currently setuptools does tell the user there is invalid metadata by saying there is a |
It's hard for me to say. I am not sure it's particularly related to the "you may have a project without a I think the metadata validation issue is probably separate from the concern about missing files, and the only related question is if we start raising errors due to missing metadata, should we try to detect that the reason the metadata is missing is that the file that you specify the metadata in is missing. I'm not really sure it will be a sufficiently common error that it's worth doing. |
Just to clarify, the potential breaking change I was referring to is throwing an error if both
I think this would lead to a better user experience, and it seems on the easy side to implement and test (e.g. easier than the PR I recently submitted), so it seems like it should be done IMO. It can even be the same exception class in both cases (e.g.
I'd be happy to implement the former as a PR today, in which case the latter could be done as a subsequent PR. I'm not familiar enough with the code base to know how easy or hard the latter would be to do. |
In a directory with a valid pyproject.toml (that uses setuptools) but no setup.py, we still have this error. This however probably needs to be fixed in setuptools (since the error is now stemming from the setuptools PEP 517 backend).
@Casper-Oakley Would you be willing to look into making the corresponding fix over at setuptools for this? It'll need changes in
setuptools/build_meta.py
around line 82.Originally posted by @pradyunsg in pypa/pip#5926 (comment)
The text was updated successfully, but these errors were encountered: