[c++] support building with Ninja on Linux#5910
Merged
Conversation
Collaborator
Author
|
cc-ing main authors of the integrated OpenCL wheels, in case you're interested: @jgiannuzzi @tpboudreau and thanks to @csegarragonz for the very helpful Stack Overflow answer 😊 |
Collaborator
Author
|
@guolinke @shiyu1994 could I get a review on this this week, if possible? Sorry for the |
guolinke
approved these changes
Jun 9, 2023
Collaborator
Author
|
Thank you! |
Contributor
|
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contributes to #5061.
Adds support for building with
Ninjainstead ofmakeon Linux.This is necessary (or at least, very very helpful) for integrating
scikit-build-coreas the backend for building the Python package (as I'm working on in #5759).What is
Ninja?Ninjais an open-source alternative tomake, originally developed to support building Google Chrome on Linux ("Google man open sources Chrome build system").It's often much faster than
make, in exchange for being simpler (i.e. supporting a stricter, harder-to-customize API).Because of its build speeds,
Ninjais the strongly-preferred default build tool for macOS and Linux inscikit-build-core. See https://github.com/scikit-build/scikit-build-core.How do these changes relate to supporting it?
Try the following on a Linux system, using latest
master(36fe73a).You'll see the following error.
This is because of the use of
ExternalProject_Add(... BUILD_COMMAND ...)in the CMake script used to build the integrated-OpenCL wheels. Unlike withmake, theNinjaCMake generator requires that you explicitly list the outputs of such commands, so the generated.ninjafiles will contain the appropriate references to them. See the following for documentation on that:This PR modifies that script by adding an explicit list of the expected output files.
Should we add new CI jobs?
I don't think that's necessary. Once #5759 is merged, the Python-package tests on Linux and macOS will be testing compilation with
Ninja.Notes for Reviewers
Thanks for your time and consideration. I know the progress towards fixing #5061 has been slow and has involved a lot of building/packaging changes. I really appreciate your thoughtful reviews.