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

add CMakeLists options to disable building CLI, installing headers #5880

Merged
merged 2 commits into from
May 27, 2023

Conversation

jameslamb
Copy link
Collaborator

Contributes to #5061.

Proposes adding 3 options to CMakeLists.txt:

  • __BUILD_FOR_PYTHON = indicate that lib_lightgbm is being built for use in a Python wheel
  • BUILD_CLI = do / do not compile the lightgbm CLI when running make or make install
  • INSTALL_HEADERS = do/do not move LightGBM's headers into CMAKE_INSTALL_PREFIX (e.g. /usr/local/include/LightGBM/*.h)

Benefits of this change

Prevents unnecessary compilation of the CLI when building the Python package with scikit-build-core.

Prevents unnecessary generation of Makefile code for targets that won't ever be used by the R and Python packages.

More Details

LightGBM's CMakeLists.txt generates Makefile(s) where make / make install has 3 effects:

  • compile lib_lightgbm.{dll/so} + install it
  • compile lightgbm (the CLI) + install it
  • install LightGBM's header files to the default system location for headers

When building for the R and Python packages, the steps about the CLI and installing headers aren't required. LightGBM currently avoids those in those contexts by running make _lightgbm instead of make or make install.

silent_call(["make", "_lightgbm", f"-I{build_dir}", "-j4"], raise_error=True,

build_args <- c("_lightgbm", make_args_from_build_script)

To address #5061, in #5759 I'm proposing that LightGBM use scikit-build-core (https://github.com/scikit-build/scikit-build-core) as its build backend.

As of this writing, that project unconditionally runs make install on the CMake-generated makefiles (docs link). With that tool, there's no way to say "just build the _lightgbm target".

This PR's changes make it possible to use LightGBM's CMakeLists.txt in a way that make and make install only build the shared library.

-v \
--install-option=--gpu \
$BUILD_DIRECTORY/dist/lightgbm-$LGB_VER.tar.gz \
|| exit -1
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes here in test.sh aren't strictly related to the CMakeLists.txt changes.

This isn't actually event changing anything about this script. Just breaking these arguments onto individual lines, to make the size of the diff in #5759 a bit smaller (and therefore those changes a bit easier to review).

@@ -15,8 +15,11 @@ set(
"Semicolon separated list of sanitizer names, e.g., 'address;leak'. \
Supported sanitizers are address, leak, undefined and thread."
)
option(BUILD_CLI "Build the 'lightbgm' command-line interface in addition to lib_lightgbm" ON)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the new options BUILD_CLI and INSTALL_HEADERS both default to ON so that this PR preserves the current default behavior on master.

@jameslamb
Copy link
Collaborator Author

jameslamb commented May 27, 2023

@guolinke @shiyu1994 can you please help with reviews on this and #5884 ? So I can hopefully continue making progress on #5061.

@jameslamb
Copy link
Collaborator Author

Thank you!

@jameslamb jameslamb merged commit 4536f43 into master May 27, 2023
@jameslamb jameslamb deleted the ci/build-control branch May 27, 2023 11:27
@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 Aug 30, 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