-
Notifications
You must be signed in to change notification settings - Fork 25
don't require ninja to come from PyPI #260
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
Conversation
In pyproject.toml, the baseline requirements for building are supposed to be listed. Many end users have a recent enough system-provided ninja, and don't need to install a new copy into every contourpy isolated build virtual environment. This is handled inside meson-python itself, which dynamically adds an additional build requirement on ninja via the backend hook `get_requires_for_build_sdist`, but only if a suitable system version is not found. This is especially helpful on systems where an OS ninja is installed, but the platform is obscure enough that there are no PyPI "ninja" wheels, thus forcing `pip install contourpy` to download an additional copy of the ninja source code and build it via cmake before beginning to install contourpy itself. This is not guaranteed to work... The ninja entry in build_requirements.txt is left alone because it's opt-in and used for CI. In CI pipelines, it may make sense to be fairly specific about which providers you want to use for any given dependency, and that's a workflow preference encoded as a workflow build command. Since it doesn't prevent people from using the baseline install command, this commit makes no value judgment regarding it.
Thanks @eli-schwartz. I'll merge as-is and probably update I note that I saw this pattern elsewhere, in |
Good point. pandas-dev/pandas#54244 |
contourpy/.github/workflows/test.yml Lines 190 to 191 in 7132cf4
As observed on the pandas tracker, "I'd prefer not to rely on the Github runners having ninja installed." thus pandas will probably keep installing ninja explicitly in CI. In particular, when building contourpy (or pandas of course) without build isolation, the need to preinstall requirements as explained in the contourpy comments means that you cannot rely on pyproject.toml build-requires, and meson-python's behavior of "automatically inject an additional build-requires for ninja, but only if it isn't installed as part of the OS" does not apply; for cases where you intend to install The simple solution is of course to always install it; in other words, avoid updating |
After consideration, what I am going to do is remove it from I don't have a section in the Originally when I was working on the switch to |
In pyproject.toml, the baseline requirements for building are supposed to be listed.
Many end users have a recent enough system-provided ninja, and don't need to install a new copy into every contourpy isolated build virtual environment. This is handled inside meson-python itself, which dynamically adds an additional build requirement on ninja via the backend hook
get_requires_for_build_sdist
, but only if a suitable system version is not found.This is especially helpful on systems where an OS ninja is installed, but the platform is obscure enough that there are no PyPI "ninja" wheels, thus forcing
pip install contourpy
to download an additional copy of the ninja source code and build it via cmake before beginning to install contourpy itself. This is not guaranteed to work...The ninja entry in build_requirements.txt is left alone because it's opt-in and used for CI. In CI pipelines, it may make sense to be fairly specific about which providers you want to use for any given dependency, and that's a workflow preference encoded as a workflow build command. Since it doesn't prevent people from using the baseline install command, this commit makes no value judgment regarding it.