Skip to content
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

[c++] support building with Ninja on Linux #5910

Merged
merged 4 commits into from
Jun 9, 2023
Merged

[c++] support building with Ninja on Linux #5910

merged 4 commits into from
Jun 9, 2023

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Jun 6, 2023

Contributes to #5061.

Adds support for building with Ninja instead of make on Linux.

This is necessary (or at least, very very helpful) for integrating scikit-build-core as the backend for building the Python package (as I'm working on in #5759).

What is Ninja?

Ninja is an open-source alternative to make, 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, Ninja is the strongly-preferred default build tool for macOS and Linux in scikit-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).

rm -rf ./build
mkdir ./build
cd ./build
cmake \
    -G 'Ninja' \
    -D__INTEGRATE_OPENCL=ON \
    ..

ninja -j2 _lightgbm

You'll see the following error.

ninja: error: 'Boost/source/stage/lib/libboost_filesystem.a', needed by '/home/jlamb/repos/LightGBM/lib_lightgbm.so', missing and no known rule to make it

This is because of the use of ExternalProject_Add(... BUILD_COMMAND ...) in the CMake script used to build the integrated-OpenCL wheels. Unlike with make, the Ninja CMake generator requires that you explicitly list the outputs of such commands, so the generated .ninja files 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.

@jameslamb jameslamb requested a review from jmoralez June 6, 2023 05:35
@jameslamb jameslamb changed the title [c++] support building with Ninja on Linux WIP: [c++] support building with Ninja on Linux Jun 6, 2023
@jameslamb jameslamb changed the title WIP: [c++] support building with Ninja on Linux [c++] support building with Ninja on Linux Jun 8, 2023
@jameslamb jameslamb marked this pull request as ready for review June 8, 2023 03:49
@jameslamb
Copy link
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 😊

@jameslamb
Copy link
Collaborator Author

@guolinke @shiyu1994 could I get a review on this this week, if possible? Sorry for the @, but I think this is an important step towards #5061 via its support of #5759 ... and therefore an important step towards the v4.0 release (#5153).

@jameslamb
Copy link
Collaborator Author

Thank you!

@jameslamb jameslamb merged commit 1c8a7ab into master Jun 9, 2023
@jameslamb jameslamb deleted the support-ninja branch June 9, 2023 02:06
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants