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] [cmake] switch to 'cmake --build'? #10699

Closed
jameslamb opened this issue Aug 13, 2024 · 1 comment · Fixed by #10717
Closed

[docs] [cmake] switch to 'cmake --build'? #10699

jameslamb opened this issue Aug 13, 2024 · 1 comment · Fixed by #10717

Comments

@jameslamb
Copy link
Contributor

Description

Many CI scripts and docs throughout this project use a pattern like this for CMake:

xgboost/doc/build.rst

Lines 130 to 133 in 4370454

mkdir build
cd build
cmake .. -DUSE_CUDA=ON
make -j4

Would you support a series of PRs (or maybe just one, depending on how involved it gets) changing all such cases to this pattern?

cmake -B build -S . -DUSE_CUDA=ON
cmake --build build --target all -j4

Benefits of this work

  • reduce friction in building
    • (2 commands instead of 4)
  • makes it easier for users to change generators
    • (e.g. to Ninja instead of GNU Make)

Approach

Replace all uses of mkdir build && cd build && cmake .. or similar with the corresponding 2 cmake commands (cmake followed by cmake --build).

Notes

I did this in LightGBM a few months ago. We haven't received any complaints about it, and I've personally found it a bit easier to work with: microsoft/LightGBM#6368

But won't be offended if you say "no thank you, we don't want this" 😅

@trivialfis
Copy link
Member

Thank you for the suggestions, sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants