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

[docs] [ci] encourage use of cmake --build #6368

Merged
merged 10 commits into from
Mar 20, 2024
Merged

Conversation

jameslamb
Copy link
Collaborator

Contributes to #6350, and just generally to the goal of making it easier to develop against LightGBM.

Proposes replacing this pattern:

mkdir build
cd build
cmake -DSOME_OPTION=ON -DOTHER_OPTION=OFF ..
make -j2 _lightgbm

with this

cmake -B build -S . -DSOME_OPTION=ON -DOTHER_OPTION=OFF
cmake --build build --target _lightgbm -j2

Benefits of this change

  • simpler to read and write (no worrying about mkdir vs. mkdir -p, or cd-ing to the right place and passing a path like ..)
  • works regardless of the generator chosen (e.g. you could switch from -G "Unix Makefiles" to -G "Ninja" and not need to change the build command)

Notes for Reviewers

In case you're not familiar with this pattern in CMake, see the following:

@jameslamb jameslamb changed the title WIP: [docs] [ci] encourage use of cmake to drive more of the build WIP: [docs] [ci] encourage use of cmake --build Mar 17, 2024
@jameslamb jameslamb changed the title WIP: [docs] [ci] encourage use of cmake --build [docs] [ci] encourage use of cmake --build Mar 18, 2024
@jameslamb jameslamb marked this pull request as ready for review March 18, 2024 03:53
Copy link
Collaborator

@borchero borchero left a comment

Choose a reason for hiding this comment

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

LGTM but I'm no cmake expert 😄

@jameslamb
Copy link
Collaborator Author

Thanks @borchero !

I'm no cmake expert

Let's get one more review from @shiyu1994 / @guolinke

@guolinke
Copy link
Collaborator

guolinke commented Mar 19, 2024

A quick question: can we use make to build after this change?

@jameslamb
Copy link
Collaborator Author

jameslamb commented Mar 19, 2024

can we use make to build after this change?

Definitely! This would work:

cmake -B build -S .
make -j2 -C build _lightgbm

The cmake --build form is just slightly more general... it'll work regardless of what generator you chose. Like this:

cmake -B build -S .
cmake --build build

cmake -B build -S . -G Ninja
# this still works
#
# otherwise, would have to do 'ninja ...'
cmake --build build

@jameslamb
Copy link
Collaborator Author

Thank you both!

@jameslamb jameslamb merged commit 4332b9d into master Mar 20, 2024
41 of 42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants