build: set CMake as default build system #20188

tchaikov · 2024-08-17T07:43:05Z

Switch the default build system from native Ninja generation to CMake.

This is a major step in our CMake migration initiative that:

Enables CMake builds by default via --use-cmake flag
Prepares for future removal of configure.py's native build.ninja generation
Streamlines our build system to use CMake exclusively
Updates the document on build system

We can still use --no-use-cmake flag to use the legacy build system.

Note: The native Ninja generation in configure.py will be removed after a grace period to ensure smooth transition.

I compared the compilation databases generated by CMake and configure.py using this comparison script, I analyzed the differences between the two build systems. The full comparison results show some variations in compile options.
I've audited all discrepancies and can confirm that:

None of the differences impact build functionality
The variations mostly represent cleanup opportunities in the CMake configuration

Refs #2717

this change should only impact the developer's workflow, no impact to production, so no need to backport.

lersek · 2024-08-19T07:57:51Z

@tchaikov I think we need a larger documentation surgery for this.

The section you are currently modifying is under heading Example: development on Fedora 25. That "example" chapter seems irrelevant for the significant "generate ninja build files with cmake" feature. I suggest removing the current cmake references in HACKING.md altogether, and writing a more comprehensive / substantial description elsewhere.

The short example here might want to name cmake.
This document knows nothing about cmake, but it will not work when configuration occurred with cmake. For example, it contains a command ninja <mode>-build, which will not work when using cmake for configuration.
In this section there are again commands like ninja-build release, which will not work with cmake (to my understanding).

I'd like to see documentation that advertises the following command (with each part unpacked and explained):

ninja -C build {scylla,tests}:{Debug,Dev,RelWithDebInfo}

Currently, none of the above-mentioned three documents (README.md, HACKING.md, docs/dev/building.md) contain RelWithDebInfo, for example.

In fact, if I grep the entire scylla tree for --use-cmake, the only hit is in configure.py itself; there is zero documentation on it.

A new developer should be able to read a single .md file and understand the complete CMake-based workflow:

configuration,
building the scylla server in all, or in some, configs,
building the entire unit test suite, or some of the tests, in all, or in some, configs,
running the test suite (all of it, or just some of it).

We have lots of stale build documentation scattered all over the place, and little up-to-date documentation. I'd rather see all of the old build docs being deleted, and a single new document explaining { dbuild, native build } x { with cmake, without cmake }.

tchaikov · 2024-08-19T12:42:53Z

@lersek hi László, thank you for the detailed suggestion. before making the CMake-based building system the formal one, we need to provide a more informative document to cover, for instance the <target>:<config> notation. i will try to address some of your comments. as i don't intend to maintain both building system at the same time in the long run, i'd prefer removing the "without cmake" part from the my change. but i won't remove the document for the "without cmake" building process, and i will fix it when it's obvious wrong or outdated. i think the first steps would be

add the CMake based document so that it is at least be able to provide enough support to a new developer to build scylla and its tests with CMake. in which, "running the test suite (all of it, or just some of it)." should have been taken care of by test.py. so unless you think the current document does not apply to configure.py or is not sufficient, i will skip this one.
fix Migrate to the CMake build system #2717 , so that our CI starts using CMake for building scylla and tests.
revisit the related documents and restructure them if necessary

please note, i am for this proposal:

removing the current cmake references in HACKING.md altogether, and writing a more comprehensive / substantial description elsewhere.

but i wanted to start with something simple and updated. and then expand it to a more comprehensive document.

lersek · 2024-08-19T15:13:16Z

@tchaikov

as i don't intend to maintain both building system at the same time in the long run, i'd prefer removing the "without cmake" part from the my change. but i won't remove the document for the "without cmake" building process, and i will fix it when it's obvious wrong or outdated.

Sounds good. We can drop that documentation when CMake has altogether obsoleted the old build.ninja generator system.

i think the first steps would be

1. add the CMake based document so that it is at least be able to provide enough support to a new developer to build scylla and its tests with CMake.

This sounds good as well. And I do agree that, if your current pull request is the "first step of the first step", then it's fine (i.e., it should be OK to first clean up the obviously stale statements on CMake, even if we don't just yet have the complete CMake write-up in place). I guess I was concerned that this small docs update was going to be the only one, for CMake's sake. :)

in which, "running the test suite (all of it, or just some of it)." should have been taken care of by test.py. so unless you think the current document does not apply to configure.py or is not sufficient, i will skip this one.

I agree that this document covers running tests. However, it does not cover building individual tests (nor do the other three documents I referenced before); I believe. From our previous discussions, there is for example a difference between rebuilding a single test binary:

# without cmake:
ninja build/{dev,debug,release}/test/boost/sstable_test

# with cmake (full syntax):
ninja -C build test/boost/{Dev,Debug,RelWithDebInfo}/sstable_test

# with cmake (shorthand; builds the binary in all modes that were not disabled at ./configure time):
ninja -C build sstable_test

I think we should capture this somewhere; it can save a lot of time, for getting a single test binary to pass.

2. fix [Migrate to the CMake build system #2717](https://github.com/scylladb/scylladb/issues/2717) , so that our CI starts using CMake for building scylla and tests.

3. revisit the related documents and restructure them if necessary
please note, i am for this proposal:

removing the current cmake references in HACKING.md altogether, and writing a more comprehensive / substantial description elsewhere.

but i wanted to start with something simple and updated. and then expand it to a more comprehensive document.

Sounds great. Thanks!

lersek · 2024-08-28T12:07:59Z

@tchaikov do you want me to review this version? (with it being a draft)

tchaikov · 2024-08-28T12:22:02Z

@lersek just wanted to update you: i am still working on this. it turns out to be messy if we keep two copies of the almost-same instructions side-by-side. so i think probably the better way is to wait until i #20261 gets merged and then i make cmake the default in configure.py.

tchaikov · 2024-08-28T12:23:06Z

@tchaikov do you want me to review this version? (with it being a draft)

no, not until i make cmake the default. by then, the document will reflect the status quo. now it is a bit ahead of time.

and thank you for asking.

lersek · 2024-08-28T12:24:32Z

OK, ping me anytime. I agree it's better if the code settles first (and cmake becomes the default).

tchaikov · 2024-10-23T08:33:30Z

@lersek ping. the change migrating our CI to CMake has been merged.

lersek

I guess the only "change" I'm requesting is to document the CMAKE_CXX_COMPILER_LAUNCHER env var; the rest is just asking for confirmation/information. Thanks!

configure.py

HACKING.md

README.md

docs/dev/building.md

tchaikov · 2024-10-25T01:55:38Z

v2:

document CMAKE_CXX_COMPILER_LAUNCHER and CMAKE_C_COMPILER_LAUNCHER en vars
remove extraneous empty line

scylladb-promoter · 2024-10-25T06:19:29Z

🟢 CI State: SUCCESS

✅ - Build
✅ - Container Test
✅ - dtest
✅ - dtest with topology changes
✅ - dtest with tablets
✅ - Unit Tests

Build Details:

Duration: 4 hr 26 min
Builder: i-0942547bf01b36eac (m5d.12xlarge)

lersek

Awesome, thanks!

avikivity · 2024-10-27T19:55:54Z

configure.py

@@ -753,7 +753,7 @@ def find_ninja():
                        help='List all available build artifacts, that can be passed to --with')
 arg_parser.add_argument('--date-stamp', dest='date_stamp', type=str,
                        help='Set datestamp for SCYLLA-VERSION-GEN')
-arg_parser.add_argument('--use-cmake', action=argparse.BooleanOptionalAction, default=False, help='Whether to use CMake as the build system')
+arg_parser.add_argument('--use-cmake', action=argparse.BooleanOptionalAction, default=True, help='Whether to use CMake as the build system')


We need proof that cmake passes the same compile options as the traditional system.

I compared the compilation databases generated by CMake and configure.py after applying these changes:

build: include subdirectory rules in compilation database merge #21312

build: enable CMAKE_CXX_EXTENSIONS explicitly #21311

build: Remove GCC ARM warning workaround (originally added in 193d1942) #21308

Using this comparison script, I analyzed the differences between the two build systems. The full comparison results show some variations in compile options.
I've audited all discrepancies and can confirm that:

None of the differences impact build functionality

The variations mostly represent cleanup opportunities in the CMake configuration

Please review and let me know if you have any questions or concerns about these differences.

I looked at the diff and the only significant difference is this:

=== wasmtime_bindings.cc: -{'W': set(), 'I': {'build/gen'}, 'f': set(), 'D': set()} +{'W': {'error=unused-result'}, 'I': {'build/Dev/seastar/gen/include', 'seastar/include'}, 'f': {'stack-clash-protection'}, 'D': {'SEASTAR_SCHEDULING_GROUPS_COUNT=16', 'SEASTAR_LOGGER_TYPE_STDOUT', 'SEASTAR_API_LEVEL=7', 'SEASTAR_SSTRING', 'SEASTAR_TYPE_ERASE_MORE', 'WITH_GZFILEOP', 'SEASTAR_LOGGER_COMPILE_TIME_FMT', 'FMT_SHARED'}} === inc.cc: -{'W': set(), 'I': {'build/gen'}, 'f': set(), 'D': set()} +{'W': {'error=unused-result'}, 'I': {'build/Dev/seastar/gen/include', 'seastar/include'}, 'f': {'stack-clash-protection'}, 'D': {'SEASTAR_SCHEDULING_GROUPS_COUNT=16', 'SEASTAR_LOGGER_TYPE_STDOUT', 'SEASTAR_API_LEVEL=7', 'SEASTAR_SSTRING', 'SEASTAR_TYPE_ERASE_MORE', 'WITH_GZFILEOP', 'SEASTAR_LOGGER_COMPILE_TIME_FMT', 'FMT_SHARED'}}

Can you explain this?

BTW which is - and which is +?

This 'I': {'build/gen'}, is present everywhere, why is that?

@denesb Hi Botond, replied inline.

I looked at the diff and the only significant difference is this:

=== wasmtime_bindings.cc: -{'W': set(), 'I': {'build/gen'}, 'f': set(), 'D': set()} +{'W': {'error=unused-result'}, 'I': {'build/Dev/seastar/gen/include', 'seastar/include'}, 'f': {'stack-clash-protection'}, 'D': {'SEASTAR_SCHEDULING_GROUPS_COUNT=16', 'SEASTAR_LOGGER_TYPE_STDOUT', 'SEASTAR_API_LEVEL=7', 'SEASTAR_SSTRING', 'SEASTAR_TYPE_ERASE_MORE', 'WITH_GZFILEOP', 'SEASTAR_LOGGER_COMPILE_TIME_FMT', 'FMT_SHARED'}} === inc.cc: -{'W': set(), 'I': {'build/gen'}, 'f': set(), 'D': set()} +{'W': {'error=unused-result'}, 'I': {'build/Dev/seastar/gen/include', 'seastar/include'}, 'f': {'stack-clash-protection'}, 'D': {'SEASTAR_SCHEDULING_GROUPS_COUNT=16', 'SEASTAR_LOGGER_TYPE_STDOUT', 'SEASTAR_API_LEVEL=7', 'SEASTAR_SSTRING', 'SEASTAR_TYPE_ERASE_MORE', 'WITH_GZFILEOP', 'SEASTAR_LOGGER_COMPILE_TIME_FMT', 'FMT_SHARED'}}

Can you explain this?

sure. these two files are both rust's C++ bindings generated using cxxbridge.

i guess you noticed that seastar flags only exist in the line starting with +. rust binding does not use Seastar headers or link against Seastar libraries, so there is no need to build with Seastar cflags. this is also why i claimed

The variations mostly represent cleanup opportunities in the CMake configuration

in previous comment in this thread.

BTW which is - and which is +?

i wanted to mimic the format of diff when comparing two building command databases. the two building command databases are generated using configure.py and CMake repectively, and the command is

../compare-compile-commands.py -ppppp {cmake-,}compile_commands.json

so the lines started with - is the command line options only found in the rules generated by CMake, and the lines started with + are those only found in the rules generated by configure.py. as you noticed, configure.py tends to pass more cflags when building object files even when they are not used at all.

This 'I': {'build/gen'}, is present everywhere, why is that?

actually, both of them have build/gen. but i am too lazy to normalize them.

the one from CMake looks like -I/home/kefu/dev/scylladb/master -I/home/kefu/dev/scylladb/master/build/gen

the one from configure.py looks like -iquote. -iquote build/dev/gen.

Ok, it indeed looks like the generated build rules are identical where it counts. I'll queue this. 🤞

This is breaking gating, I assume because we need this change together with some change in scylla-pkg

@scylladb/scylla-maint can you please dequeue this patch? it's breaking gating

tchaikov · 2024-11-12T02:46:31Z

@scylladb/scylla-maint hello maintainers, could you help review this change?

Switch the default build system from native Ninja generation to CMake. This is a major step in our CMake migration initiative that: - Enables CMake builds by default via `--use-cmake` flag - Prepares for future removal of configure.py's native build.ninja generation - Streamlines our build system to use CMake exclusively - Updates the document on build system We can still use `--no-use-cmake` flag to use the legacy build system. Note: The native Ninja generation in configure.py will be removed after a grace period to ensure smooth transition. Refs #2717 --- this change should only impact the developer's workflow, no impact to production, so no need to backport. Closes #20188 * github.com:scylladb/scylladb: {HACKING,README}.md: update the CMake related remarks build: set CMake as default build system

avikivity · 2024-11-13T21:02:26Z

Dequeued since it broke promotion.

@tchaikov the proof it doesn't change flags should be in the cover letter, not hidden in a comment.

scylladb-promoter · 2024-12-22T10:08:37Z

🟢 CI State: SUCCESS

✅ - Build
✅ - Docker Test
✅ - dtest with topology changes
✅ - dtest
✅ - dtest with tablets
✅ - Offline-installer Artifact Tests
✅ - Unit Tests

Build Details:

Duration: 4 hr 0 min
Builder: spider2.cloudius-systems.com

tchaikov · 2024-12-22T13:42:49Z

v3:

noted that i've compared the build commands generated by configure.py and those generated by CMake building system in the commit message and in the cover letter.

tchaikov · 2024-12-22T13:58:46Z

@denesb hi Botond, could you help re-review this change? i've switched the CI pipelines to CMake in https://github.com/scylladb/scylla-pkg/commit/4df8b8b54f9e8599f367ccbb4917ff881c6452a6 .

avikivity · 2024-12-22T15:08:48Z

@tchaikov let's wait until the build system changes are merged from enterprise. These are so large they would invalidate the comparison results.

tchaikov · 2024-12-23T07:18:04Z

@tchaikov let's wait until the build system changes are merged from enterprise. These are so large they would invalidate the comparison results.

okay. let's put this change on hold then.

Switch the default build system from native Ninja generation to CMake. This is a major step in our CMake migration initiative that: - Enables CMake builds by default via `--use-cmake` flag - Prepares for future removal of configure.py's native build.ninja generation - Streamlines our build system to use CMake exclusively We can still use `--no-use-cmake` flag to use the legacy build system. The compilation databases generated by CMake and configure.py were compared. And the differences between them were analyzed, I can confirm that - None of the differences impact build functionality - The variations mostly represent cleanup opportunities in the CMake configuration Note: The native Ninja generation in configure.py will be removed after a grace period to ensure smooth transition. Refs scylladb#2717 Signed-off-by: Kefu Chai <[email protected]>

the rules generated with CMake script is feature-wise on par with the one generated directly with `configure.py`, so the existing document is now stale. in this change, we update the document to reflect the current status of CMake support. Fixes scylladb#22212 Signed-off-by: Kefu Chai <[email protected]>

tchaikov · 2025-01-10T09:54:10Z

v4:

rebased against master HEAD
added "Fixes Document how to build Scylla with cmake - like CI does #22212" tag in the commit message
updated the cover letter with the link to the updated compare result

tchaikov · 2025-01-10T09:54:15Z

@avikivity and @denesb, i

regenerate the compare result at https://gist.github.com/tchaikov/ebeadaaf6ea6bbcebc20feae10555cd8 with the latest master HEAD.
checked it. no significant issues identified.

could you take another look?

scylladb-promoter · 2025-01-10T18:23:26Z

🔴 CI State: FAILURE

✅ - Build
✅ - Docker Test
✅ - dtest with tablets
✅ - dtest
✅ - dtest with topology changes
✅ - Offline-installer Artifact Tests
❌ - Unit Tests

Build Details:

Duration: 8 hr 21 min
Builder: i-05d8fb3b868e811e5 (m5ad.8xlarge)

tchaikov added documentation Requires documentation backport/none Backport is not required area/build labels Aug 17, 2024

tchaikov requested a review from lersek August 17, 2024 07:43

tchaikov marked this pull request as draft August 19, 2024 12:29

lersek approved these changes Aug 19, 2024

View reviewed changes

tchaikov force-pushed the hacking-with-cmake branch from f84c8d3 to b72b448 Compare August 28, 2024 12:00

tchaikov force-pushed the hacking-with-cmake branch from b72b448 to ad4f30a Compare October 23, 2024 08:29

tchaikov marked this pull request as ready for review October 23, 2024 08:29

tchaikov changed the title ~~HACKING.md: update the CMake related remarks~~ build: set CMake as default build system Oct 23, 2024

tchaikov requested review from lersek and denesb October 23, 2024 08:49

lersek suggested changes Oct 24, 2024

View reviewed changes

tchaikov force-pushed the hacking-with-cmake branch from ad4f30a to be14ac4 Compare October 25, 2024 01:52

scylladbbot added the status/ci in progress label Oct 25, 2024

tchaikov requested a review from lersek October 25, 2024 02:54

scylladbbot removed the status/ci in progress label Oct 25, 2024

lersek approved these changes Oct 25, 2024

View reviewed changes

avikivity reviewed Oct 27, 2024

View reviewed changes

syuu1228 mentioned this pull request Nov 12, 2024

test: compile all unit tests into a single executable #21443

Closed

tchaikov force-pushed the hacking-with-cmake branch from be14ac4 to c941ae4 Compare December 22, 2024 06:07

scylladbbot added status/ci in progress and removed status/ci in progress labels Dec 22, 2024

tchaikov marked this pull request as draft December 23, 2024 07:18

mykaul added this to the 2025.2.0 milestone Dec 23, 2024

tchaikov marked this pull request as ready for review January 10, 2025 09:50

tchaikov force-pushed the hacking-with-cmake branch from c941ae4 to 60f96b1 Compare January 10, 2025 09:50

tchaikov force-pushed the hacking-with-cmake branch from 60f96b1 to 3a844e7 Compare January 10, 2025 09:52

tchaikov mentioned this pull request Jan 10, 2025

Document how to build Scylla with cmake - like CI does #22212

Open

scylladbbot added status/ci in progress and removed status/ci in progress labels Jan 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: set CMake as default build system #20188

build: set CMake as default build system #20188

tchaikov commented Aug 17, 2024 •

edited

Loading

lersek commented Aug 19, 2024

tchaikov commented Aug 19, 2024 •

edited

Loading

lersek commented Aug 19, 2024

lersek commented Aug 28, 2024

tchaikov commented Aug 28, 2024

tchaikov commented Aug 28, 2024 •

edited

Loading

lersek commented Aug 28, 2024

tchaikov commented Oct 23, 2024

lersek left a comment

tchaikov commented Oct 25, 2024

scylladb-promoter commented Oct 25, 2024

lersek left a comment

avikivity Oct 27, 2024

tchaikov Oct 28, 2024 •

edited

Loading

denesb Nov 13, 2024

tchaikov Nov 13, 2024 •

edited

Loading

denesb Nov 13, 2024

yaronkaikov Nov 13, 2024

yaronkaikov Nov 13, 2024

tchaikov commented Nov 12, 2024

avikivity commented Nov 13, 2024

scylladb-promoter commented Dec 22, 2024

tchaikov commented Dec 22, 2024

tchaikov commented Dec 22, 2024

avikivity commented Dec 22, 2024

tchaikov commented Dec 23, 2024

tchaikov commented Jan 10, 2025

tchaikov commented Jan 10, 2025

scylladb-promoter commented Jan 10, 2025

build: set CMake as default build system #20188

Are you sure you want to change the base?

build: set CMake as default build system #20188

Conversation

tchaikov commented Aug 17, 2024 • edited Loading

lersek commented Aug 19, 2024

tchaikov commented Aug 19, 2024 • edited Loading

lersek commented Aug 19, 2024

lersek commented Aug 28, 2024

tchaikov commented Aug 28, 2024

tchaikov commented Aug 28, 2024 • edited Loading

lersek commented Aug 28, 2024

tchaikov commented Oct 23, 2024

lersek left a comment

Choose a reason for hiding this comment

tchaikov commented Oct 25, 2024

scylladb-promoter commented Oct 25, 2024

🟢 CI State: SUCCESS

Build Details:

lersek left a comment

Choose a reason for hiding this comment

avikivity Oct 27, 2024

Choose a reason for hiding this comment

tchaikov Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

denesb Nov 13, 2024

Choose a reason for hiding this comment

tchaikov Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

denesb Nov 13, 2024

Choose a reason for hiding this comment

yaronkaikov Nov 13, 2024

Choose a reason for hiding this comment

yaronkaikov Nov 13, 2024

Choose a reason for hiding this comment

tchaikov commented Nov 12, 2024

avikivity commented Nov 13, 2024

scylladb-promoter commented Dec 22, 2024

🟢 CI State: SUCCESS

Build Details:

tchaikov commented Dec 22, 2024

tchaikov commented Dec 22, 2024

avikivity commented Dec 22, 2024

tchaikov commented Dec 23, 2024

tchaikov commented Jan 10, 2025

tchaikov commented Jan 10, 2025

scylladb-promoter commented Jan 10, 2025

🔴 CI State: FAILURE

Build Details:

tchaikov commented Aug 17, 2024 •

edited

Loading

tchaikov commented Aug 19, 2024 •

edited

Loading

tchaikov commented Aug 28, 2024 •

edited

Loading

tchaikov Oct 28, 2024 •

edited

Loading

tchaikov Nov 13, 2024 •

edited

Loading