Skip to content

build(cmake): simplify instructions (cmake -B build && cmake --build build ...)#6964

Merged
ochafik merged 10 commits intoggml-org:masterfrom
ochafik:cmake-b
Apr 29, 2024
Merged

build(cmake): simplify instructions (cmake -B build && cmake --build build ...)#6964
ochafik merged 10 commits intoggml-org:masterfrom
ochafik:cmake-b

Conversation

@ochafik
Copy link
Copy Markdown
Collaborator

@ochafik ochafik commented Apr 28, 2024

  • Updated instructions & CI to skip the mkdir -p build && cd build steps using config-time flag -B and build-time flag --build

    mkdir -b build
    cd build
    cmake ..
    make # or `cmake --build . --config Release`

    Becomes:

    cmake -B build
    cmake --build build --config Release
    # --config flag is ignored for Makefile generation (default) but crucial for Release builds w/ Xcode / MSVC, as pointed out by reviewers
  • Documented how to do Debug builds

  • Removed --config Release from most cmake build commands (except for Visual Studio) as Release is the default and the --config flag is often useless anyway)

    Note: To build in Debug w/ CMake one generally needs to set -DCMAKE_BUILD_TYPE=Debug in the "config" command (--config Debug on the build command works only for Ninja Multi-Config, XCode or Visual Studio generators)

@ochafik ochafik changed the title Simplify CMake build instructions (cmake . -B build && cmake --build build is all you need) build(cmake): simplify instructions (cmake . -B build && cmake --build build is all you need) Apr 28, 2024
Comment thread README.md Outdated
@cebtenzzre
Copy link
Copy Markdown
Collaborator

The source directory defaults to the current directory when using -B. So cmake . -B build should just be cmake -B build.

ochafik and others added 2 commits April 28, 2024 17:57
Co-authored-by: Jared Van Bortel <cebtenzzre@gmail.com>
@ochafik ochafik changed the title build(cmake): simplify instructions (cmake . -B build && cmake --build build is all you need) build(cmake): simplify instructions (cmake -B build && cmake --build build is all you need) Apr 28, 2024
@ochafik
Copy link
Copy Markdown
Collaborator Author

ochafik commented Apr 28, 2024

The source directory defaults to the current directory when using -B. So cmake . -B build should just be cmake -B build.

@cebtenzzre Neat! Updated.

@ochafik ochafik marked this pull request as ready for review April 28, 2024 17:14
Comment thread README-sycl.md Outdated
Comment thread README-sycl.md Outdated
Co-authored-by: Neo Zhang Jianyu <jianyu.zhang@intel.com>
@slaren
Copy link
Copy Markdown
Member

slaren commented Apr 29, 2024

  • Removed --config Release from most cmake build commands (except for Visual Studio) as Release is the default and the --config flag is often useless anyway)

I am not sure that this is entirely correct. CMAKE_BUILD_TYPE configures the build type for single-config generators, while --config selects the build type for multi-config generators (ie. MSVC). I have been trying to find what is the default value of --config, but I have not been able to find any references. Are you sure that removing --config will still build in release with MSVC in all cases?

@slaren
Copy link
Copy Markdown
Member

slaren commented Apr 29, 2024

I just ran a quick test, and without --config Release, it will result in a debug build. Therefore I think it is important to keep the --config Release, at least in the documentation that is meant to be applicable for all the platforms.

@ochafik
Copy link
Copy Markdown
Collaborator Author

ochafik commented Apr 29, 2024

Removed --config Release from most cmake build commands (except for Visual Studio) as Release is the default and the --config flag is often useless anyway)

I am not sure that this is entirely correct

@slaren Oops I misspoke, it's the default for single-generator configs (as set here), and the default is generator dependent for the others.

I just ran a quick test, and without --config Release, it will result in a debug build. Therefore I think it is important to keep the --config Release, at least in the documentation that is meant to be applicable for all the platforms.

Thanks a lot for catching this! I just checked if setting -DCMAKE_CONFIGURATION_TYPES=Release;Debug;MinSizeRel;RelWithDebInfo would give control over the default (hoping it'd pick the first) but it's not the case at least for Xcode.

I've reverted the --config Release bit and added instructions for Debug builds on the main README, PTAL 😁

@ochafik ochafik changed the title build(cmake): simplify instructions (cmake -B build && cmake --build build is all you need) build(cmake): simplify instructions (cmake -B build && cmake --build build ...) Apr 29, 2024
Comment thread README.md Outdated
Comment thread README.md
@slaren slaren dismissed NeoZhangJianyu’s stale review April 29, 2024 15:59

Already addressed.

@ochafik ochafik merged commit b8a7a5a into ggml-org:master Apr 29, 2024
Seunghhon pushed a commit to Seunghhon/llama.cpp that referenced this pull request Apr 26, 2026
… build ...`) (ggml-org#6964)

* readme: cmake . -B build && cmake --build build

* build: fix typo

Co-authored-by: Jared Van Bortel <cebtenzzre@gmail.com>

* build: drop implicit . from cmake config command

* build: remove another superfluous .

* build: update MinGW cmake commands

* Update README-sycl.md

Co-authored-by: Neo Zhang Jianyu <jianyu.zhang@intel.com>

* build: reinstate --config Release as not the default w/ some generators + document how to build Debug

* build: revert more --config Release

* build: nit / remove -H from cmake example

* build: reword debug instructions around single/multi config split

---------

Co-authored-by: Jared Van Bortel <cebtenzzre@gmail.com>
Co-authored-by: Neo Zhang Jianyu <jianyu.zhang@intel.com>
phuongncn pushed a commit to phuongncn/llama.cpp-gx10-dgx-sparks-deepseekv4 that referenced this pull request Apr 28, 2026
… build ...`) (ggml-org#6964)

* readme: cmake . -B build && cmake --build build

* build: fix typo

Co-authored-by: Jared Van Bortel <cebtenzzre@gmail.com>

* build: drop implicit . from cmake config command

* build: remove another superfluous .

* build: update MinGW cmake commands

* Update README-sycl.md

Co-authored-by: Neo Zhang Jianyu <jianyu.zhang@intel.com>

* build: reinstate --config Release as not the default w/ some generators + document how to build Debug

* build: revert more --config Release

* build: nit / remove -H from cmake example

* build: reword debug instructions around single/multi config split

---------

Co-authored-by: Jared Van Bortel <cebtenzzre@gmail.com>
Co-authored-by: Neo Zhang Jianyu <jianyu.zhang@intel.com>
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 this pull request may close these issues.

5 participants