Add cmake dependency only when system cmake is not available #6642
Conversation
Instead of unconditionally pulling `cmake` PyPI package in, check whether `cmake` is available first and add the dependency only when it is not available. This is the same approach as used by the `scikit-build-core` package, and it improves portability, especially given that CMake frequently requires downstream patching to work. Signed-off-by: Michał Górny <mgorny@gentoo.org>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6642 +/- ##
=======================================
Coverage 57.46% 57.46%
=======================================
Files 507 507
Lines 31584 31584
Branches 3046 3046
=======================================
Hits 18151 18151
Misses 12607 12607
Partials 826 826 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks! LGTM but the setup_requires option seems to be deprecated: https://setuptools.pypa.io/en/latest/userguide/dependency_management.html#build-system-requirement:~:text=In%20previous%20versions%20of%20setuptools%2C%20the%20setup_requires%20keyword%20performed%20a%20similar%20function%20but%20is%20now%20considered%20deprecated%20in%20favor%20of%20the%20PEP%20517%20style%20described%20above.%20To%20peek%20into%20how%20this%20legacy%20keyword%20is%20used%2C%20consult%20our%20guide%20on%20deprecated%20practice%20(WIP).
I wonder if there are better ways to do this?
|
I've filed pypa/setuptools#4806 asking for a "better way", but I don't hold my hopes high. This is pretty much a corner case. The alternative approach would be to create a custom PEP517 build backend that calls into |
|
I'm trying the alternate approach to see how hard it will end up, and will file an alternate PR with it. |
|
#6643. It's definitely more complex but not as complex as I thought. It has the extra advantage that the dependency is only added when building wheels and not when building sdists. I think the |
… version) (#6643) ### Description Instead of unconditionally pulling `cmake` PyPI package in, check whether `cmake` is available first and add the dependency only when it is not available. This is the same approach as used by the `scikit-build-core` package, and it improves portability, especially given that CMake frequently requires downstream patching to work. In this version of the patch, I've created a minimal PEP517 build backend that inherits the functions provided by setuptools, and adds the CMake dependency when necessary. This avoids using deprecated setuptools `setup_requires` argument, and it avoids adding CMake dependency when building source distributions. ### Motivation and Context Same as #6642, except using PEP517 backend instead of deprecated `setup_requires`. Signed-off-by: Michał Górny <mgorny@gentoo.org>
… version) (#6643) ### Description Instead of unconditionally pulling `cmake` PyPI package in, check whether `cmake` is available first and add the dependency only when it is not available. This is the same approach as used by the `scikit-build-core` package, and it improves portability, especially given that CMake frequently requires downstream patching to work. In this version of the patch, I've created a minimal PEP517 build backend that inherits the functions provided by setuptools, and adds the CMake dependency when necessary. This avoids using deprecated setuptools `setup_requires` argument, and it avoids adding CMake dependency when building source distributions. ### Motivation and Context Same as #6642, except using PEP517 backend instead of deprecated `setup_requires`. Signed-off-by: Michał Górny <mgorny@gentoo.org> Signed-off-by: Andreas Fehlner <fehlner@arcor.de>
… version) (onnx#6643) ### Description Instead of unconditionally pulling `cmake` PyPI package in, check whether `cmake` is available first and add the dependency only when it is not available. This is the same approach as used by the `scikit-build-core` package, and it improves portability, especially given that CMake frequently requires downstream patching to work. In this version of the patch, I've created a minimal PEP517 build backend that inherits the functions provided by setuptools, and adds the CMake dependency when necessary. This avoids using deprecated setuptools `setup_requires` argument, and it avoids adding CMake dependency when building source distributions. ### Motivation and Context Same as onnx#6642, except using PEP517 backend instead of deprecated `setup_requires`. Signed-off-by: Michał Górny <mgorny@gentoo.org> Signed-off-by: seungwoo-ji <seungwoo.ji@nuvilab.com>
Description
Instead of unconditionally pulling
cmakePyPI package in, check whethercmakeis available first and add the dependency only when it is not available. This is the same approach as used by thescikit-build-corepackage, and it improves portability, especially given that CMake frequently requires downstream patching to work.Motivation and Context
Currently, having an unconditional
cmakedependency means thatpiporuvwill install a local PyPI-packaged version ofcmaketo build the package. Besides being unnecessary, this can be particularly problematic, given that CMake is buggy in general and sometimes relies on downstream patching, that is carried in system CMake but not (yet) in PyPI's CMake. Ensuring that system CMake is used when available generally improves interoperability and consistency.