-
Notifications
You must be signed in to change notification settings - Fork 68
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
pip check fails due to dependency on ninja #60
Comments
That seems simply expected to me, given that This will resolve itself as soon as we package
I propose to close this, since this is normal and not a problem. |
That's why I'm asking actually, see conda-forge/staged-recipes#19033. |
Thanks for doing that packaging work. I commented on your PR there.
Can you post the actual traceback you're seeing, and/or how to reproduce the problem? |
This should more or less match the used check in the recipe
|
That all makes sense. It may be the case that the How about just asking on the conda-forge PR how to bypass |
I can simply remove the |
Then maybe a patch is needed for |
Python packages installed by The issue is that |
I'm not sure why you say that |
Installing
If I can see this correctly the |
So I think that is conda-forge/ninja-feedstock#26. If the The dependency is there, and saying it's not is kinda backwards. The problem with PyPI is not that
This is never necessary anymore once |
Meson does have an optional extra, meson-python is obviously making the tradeoff that |
After some more digging in that repo, the correct fix is to create a new |
I think this is the difference between |
Update: one idea @FFY00 longer term came up with is to write a small wrapper which checks if For now, the status quo should be okay though. |
Maybe we could add a |
That does seem reasonable to me @wolfv, and perhaps better than every redistributor having to patch |
@rgommers I've opened: pypa/pip#11157 |
Thank you all for your suggestions on how to handle this. This is the old issue of the Python packaging ecosystem not integrating well with external dependencies. As Ralf mentioned, one possible solution would be to try to detect if ninja is available, by looking for a |
Yes, I don't think this is a good idea since it would mean that the package would produce a different wheel depending on what's installed on the system. One option that could make sense is to give an option for vendors to easily control the presence of the dep, something like |
Why would it produce a different wheel? EDIT: Wait. I had assumed this would be adding it as an additional get_requires_for_build_wheel return value, not a run dependency of meson-python itself. That's in fact the exact approach being taken in #175 |
Yeah, I was thinking of the current "run dependency" approach. Indeed having it returned via |
@eli-schwartz that is what is happening, the PR is just keeping the dependency for bootstrap. @mgorny we do not pin the dependency, so this is the same having a different version of the PyPI dependency. If you care about reproducibility you should be controlling the environment anyway. The issue here is that we depend on a native dependency, and the PyPI ninja package is nothing but a hack, to workaround the fact that these kind of dependencies are not handled by Python packaging tooling. From a purity standpoint, one could argue that the ninja PyPI dependency shouldn't even be there. Maybe one day we will have a way to better handle native dependencies, and then we should reconsider this situation, but for now let's settle for something that works well for the current status quo. |
If this causes issue anyway, we can also come back and reconsider. |
meson-python does not hard depend on the ninja PyPI package anymore. ninja is detected at runtime and added to the build backend dependencies only if necessary. |
Running
pip check
inside a conda environment currently fails because meson-python depends on the PyPI version of ninja and cannot detect the actually installed ninja version from conda-forge.The text was updated successfully, but these errors were encountered: