[PATCH scylla v1] Improve `CMakeLists.txt` for CLion

[Jesse Haber-Kucharsky]

This version reads the compilation flags for Seastar from pkg-config and uses
file globbing to register all source and header files.

In comparison to the previous version, I see all files in the project explorer
view are "active" (rather than just .cc files). I believe there are also fewer
errors reported by the editor.

Signed-off-by: Jesse Haber-Kucharsky <jhab...@scylladb.com>
---
CMakeLists.txt | 145 ++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 97 insertions(+), 48 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5538efb..b56fbab 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,55 +1,104 @@
cmake_minimum_required(VERSION 3.5)
-project(scylla)
+project(Scylla)

-if (NOT DEFINED ENV{CLION_IDE})
+if(NOT DEFINED ENV{CLION_IDE})
message(FATAL_ERROR "This CMakeLists.txt file is only valid for use in CLion")
endif()

+find_package(PkgConfig REQUIRED)
+
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=gnu++14 -DHAVE_HWLOC -DHAVE_DPDK")

-include_directories(. build/release/gen seastar)
-
-aux_source_directory(build/release/gen SOURCE_FILES)
-aux_source_directory(build/release/gen/cql3 SOURCE_FILES)
-aux_source_directory(build/release/gen/idl SOURCE_FILES)
-aux_source_directory(. SOURCE_FILES)
-aux_source_directory(api SOURCE_FILES)
-aux_source_directory(auth SOURCE_FILES)
-aux_source_directory(cql3 SOURCE_FILES)
-aux_source_directory(cql3/functions SOURCE_FILES)
-aux_source_directory(cql3/restrictions SOURCE_FILES)
-aux_source_directory(cql3/selection SOURCE_FILES)
-aux_source_directory(cql3/statements SOURCE_FILES)
-aux_source_directory(db/ SOURCE_FILES)
-aux_source_directory(db/commitlog SOURCE_FILES)
-aux_source_directory(db/index SOURCE_FILES)
-aux_source_directory(db/marshal SOURCE_FILES)
-aux_source_directory(db/view SOURCE_FILES)
-aux_source_directory(dht SOURCE_FILES)
-aux_source_directory(exceptions SOURCE_FILES)
-aux_source_directory(gms SOURCE_FILES)
-aux_source_directory(index SOURCE_FILES)
-aux_source_directory(io SOURCE_FILES)
-aux_source_directory(locator SOURCE_FILES)
-aux_source_directory(message SOURCE_FILES)
-aux_source_directory(repair SOURCE_FILES)
-aux_source_directory(seastar SOURCE_FILES)
-aux_source_directory(seastar/core SOURCE_FILES)
-aux_source_directory(seastar/http SOURCE_FILES)
-aux_source_directory(seastar/net SOURCE_FILES)
-aux_source_directory(seastar/rpc SOURCE_FILES)
-aux_source_directory(seastar/tests SOURCE_FILES)
-aux_source_directory(seastar/util SOURCE_FILES)
-aux_source_directory(service SOURCE_FILES)
-aux_source_directory(service/pager SOURCE_FILES)
-aux_source_directory(sstables SOURCE_FILES)
-aux_source_directory(streaming SOURCE_FILES)
-aux_source_directory(tests SOURCE_FILES)
-aux_source_directory(tests/perf SOURCE_FILES)
-aux_source_directory(thrift SOURCE_FILES)
-aux_source_directory(tracing SOURCE_FILES)
-aux_source_directory(transport SOURCE_FILES)
-aux_source_directory(transport/messages SOURCE_FILES)
-aux_source_directory(utils SOURCE_FILES)
-
-add_executable(scylla ${SOURCE_FILES})
+set(ENV{PKG_CONFIG_PATH} "${CMAKE_SOURCE_DIR}/seastar/build/release:$ENV{PKG_CONFIG_PATH}")
+pkg_check_modules(SEASTAR REQUIRED seastar)
+
+function(add_seastar_executable)
+ set(options "")
+ set(oneValueArgs NAME)
+ set(multiValueArgs SOURCES)
+ cmake_parse_arguments(args "${options}" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}")
+
+ add_executable(${args_NAME} ${args_SOURCES})
+
+ target_include_directories(${args_NAME} SYSTEM PUBLIC ${SEASTAR_INCLUDE_DIRS})
+ target_compile_options(${args_NAME} PUBLIC ${SEASTAR_CFLAGS})
+
+ target_link_libraries(${args_NAME}
+ ${CMAKE_THREAD_LIBS_INIT}
+ ${SEASTAR_LIBRARIES}
+ ${SEASTAR_LDFLAGS})
+endfunction(add_seastar_executable)
+
+function(add_scylla_source_directories)
+ set(options RECURSIVE)
+ set(oneValueArgs VAR)
+ set(multiValueArgs PATHS)
+ cmake_parse_arguments(args "${options}" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}")
+
+ set(globs "")
+
+ foreach(dir ${args_PATHS})
+ list(APPEND globs "${dir}/*.cc" "${dir}/*.hh")
+ endforeach(dir)
+
+ if(args_RECURSIVE)
+ set(glob_kind GLOB_RECURSE)
+ else(NOT args_RECURSIVE)
+ set(glob_kind GLOB)
+ endif(args_RECURSIVE)
+
+ file(${glob_kind} var
+ ${globs})
+
+ set (${args_VAR} ${var} PARENT_SCOPE)
+endfunction(add_scylla_source_directories)
+
+add_scylla_source_directories(
+ VAR SCYLLA_ROOT_SOURCE_FILES
+ RECURSIVE no
+ PATHS .)
+
+add_scylla_source_directories(
+ VAR SCYLLA_SUB_SOURCE_FILES
+ RECURSIVE yes
+
+ PATHS
+ api
+ auth
+ cql3
+ db
+ dht
+ exceptions
+ gms
+ index
+ io
+ locator
+ message
+ repair
+ service
+ sstables
+ streaming
+ thrift
+ tracing
+ transport
+ utils)
+
+add_scylla_source_directories(
+ VAR SCYLLA_GEN_SOURCE_FILES
+ RECURSIVE yes
+ PATHS build/release/gen)
+
+set(SCYLLA_SOURCE_FILES
+ ${SCYLLA_ROOT_SOURCE_FILES}
+ ${SCYLLA_GEN_SOURCE_FILES}
+ ${SCYLLA_SUB_SOURCE_FILES})
+
+add_seastar_executable(
+ NAME scylla
+ SOURCES ${SCYLLA_SOURCE_FILES})
+
+target_include_directories(scylla PRIVATE
+ .
+ build/release/gen
+ build/release/gen/cql3
+ build/release/gen/idl)
--
2.9.4

[Duarte Nunes]

On 20/06/2017 22:57, Jesse Haber-Kucharsky wrote:
> This version reads the compilation flags for Seastar from pkg-config and uses
> file globbing to register all source and header files.
>
> In comparison to the previous version, I see all files in the project explorer
> view are "active" (rather than just .cc files). I believe there are also fewer
> errors reported by the editor.
>
> Signed-off-by: Jesse Haber-Kucharsky <jhab...@scylladb.com>
> ---
> CMakeLists.txt | 145 ++++++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 97 insertions(+), 48 deletions(-)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 5538efb..b56fbab 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -1,55 +1,104 @@
> cmake_minimum_required(VERSION 3.5)
> -project(scylla)
> +project(Scylla)

Why the uppercase?

It would be nice if this didn't require seastar to be compiled (i.e.,
for running clion on a mac in airplanes).

tests dir seems to be missing.

> +
> +add_scylla_source_directories(
> + VAR SCYLLA_GEN_SOURCE_FILES
> + RECURSIVE yes
> + PATHS build/release/gen)

build/release/gen/cql3 seems to be missing. Maybe /idl should be there too.

> +
> +set(SCYLLA_SOURCE_FILES
> + ${SCYLLA_ROOT_SOURCE_FILES}
> + ${SCYLLA_GEN_SOURCE_FILES}
> + ${SCYLLA_SUB_SOURCE_FILES})
> +
> +add_seastar_executable(
> + NAME scylla
> + SOURCES ${SCYLLA_SOURCE_FILES})
> +
> +target_include_directories(scylla PRIVATE
> + .
> + build/release/gen
> + build/release/gen/cql3
> + build/release/gen/idl)
>

What about the seastar source files (note: I couldn't really
reverse-engineer the add_seastar_executable function)?

[Jesse Haber-Kucharsky]

Thanks for the quick comments.

On Tue, Jun 20, 2017 at 11:14:13PM +0200, Duarte Nunes wrote:
> Why the uppercase?

I suppose distinction from the `scylla` executable target, though I don't really know if this makes an appreciable difference.

> It would be nice if this didn't require seastar to be compiled (i.e., for
> running clion on a mac in airplanes).

All we need is `seastar.pc`. I confirmed (and I will document) that you can generate that via `ninja-build build/release/seastar.pc` without compiling Seastar in its entirety.

> tests dir seems to be missing.

Though, strangely, the source files are include by CLion anyway. Doesn't hurt to add it.

> build/release/gen/cql3 seems to be missing. Maybe /idl should be there too.

The `RECURSIVE yes` parameters should make the globbing descend into `cql3` and `idl`

> What about the seastar source files

These seem to be picked-up okay by CLion through the `pkg-config` flags.

> (note: I couldn't really reverse-engineer the add_seastar_executable function)?

I'll add some documentation. :-)

[Duarte Nunes]

On 20/06/2017 23:29, Jesse Haber-Kucharsky wrote:
> Thanks for the quick comments.
>
> On Tue, Jun 20, 2017 at 11:14:13PM +0200, Duarte Nunes wrote:
>> Why the uppercase?
>
> I suppose distinction from the `scylla` executable target, though I don't really know if this makes an appreciable difference.

Ah, I guess those things should be distinguishable from the context, so
no need to change.

>
>> It would be nice if this didn't require seastar to be compiled (i.e., for
>> running clion on a mac in airplanes).
>
> All we need is `seastar.pc`. I confirmed (and I will document) that you can generate that via `ninja-build build/release/seastar.pc` without compiling Seastar in its entirety.

Yeah, but I can't really get that on a mac. It's okay though, we may
move to cmake in the future and then I guess I'd be forced to keep a
CMakeLists.txt file for edit-only CLion usage anyway.

>
>> tests dir seems to be missing.
>
> Though, strangely, the source files are include by CLion anyway. Doesn't hurt to add it.

That's because glob_kind is always being set to GLOB_RECURSE. You can
check that by doing:

if(args_RECURSIVE)
message("RECURSIVE")
set(glob_kind GLOB_RECURSE)
else(NOT args_RECURSIVE)
set(glob_kind GLOB)
endif(args_RECURSIVE)

>
>> build/release/gen/cql3 seems to be missing. Maybe /idl should be there too.
>
> The `RECURSIVE yes` parameters should make the globbing descend into `cql3` and `idl`

Right.

>
>> What about the seastar source files
>
> These seem to be picked-up okay by CLion through the `pkg-config` flags.

I'm fairly certain it's because of the above bug.

>
>> (note: I couldn't really reverse-engineer the add_seastar_executable function)?
>
> I'll add some documentation. :-)
>

Thanks :)

[Jesse Haber-Kucharsky]

This version reads the compilation flags for Seastar from pkg-config and uses
file globbing to register all source and header files.

In comparison to the previous version, I see all files in the project explorer
view are "active" (rather than just .cc files). I believe there are also fewer
errors reported by the editor.

Note that we assume the project has been compiled with Ninja, or at least that
the pkg-config information for Seastar has been generated.

Signed-off-by: Jesse Haber-Kucharsky <jhab...@scylladb.com>
---

Changes since v1:
- Fix bug with accidental recursive globbing.

- Add documentation for functions and usage.

- Include Seastar source files for convenience.

---
CMakeLists.txt | 181 ++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 134 insertions(+), 47 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5538efb..8896222 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,55 +1,142 @@
+##
+## To use this file with CLion, first compile the project using the Ninja build-system.
+##
+## Alternatively, generate only the pkg-config information for Seastar (`seastar.pc`):
+##
+## $ ./configure.py [my configuration options]
+## $ ninja-build -C seastar build/$MODE/seastar.pc
+##
+
cmake_minimum_required(VERSION 3.5)
project(scylla)

+
+##
+## Add a new executable target for a Seastar application. Compilation flags for Seastar are extracted from the
+## pkg-config information.
+##
+## Use: add_seastar_executable(NAME my_app SOURCES [source1.cc source2.cc ...])
+##

+function(add_seastar_executable)
+ set(options "")
+ set(oneValueArgs NAME)
+ set(multiValueArgs SOURCES)
+ cmake_parse_arguments(args "${options}" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}")
+
+ add_executable(${args_NAME} ${args_SOURCES})
+
+ target_include_directories(${args_NAME} SYSTEM PUBLIC ${SEASTAR_INCLUDE_DIRS})
+ target_compile_options(${args_NAME} PUBLIC ${SEASTAR_CFLAGS})
+
+ target_link_libraries(${args_NAME}
+ ${CMAKE_THREAD_LIBS_INIT}
+ ${SEASTAR_LIBRARIES}
+ ${SEASTAR_LDFLAGS})
+endfunction(add_seastar_executable)
+

+##
+## Populate the names of all source and header files in the indicated paths in a designated variable.
+##
+## When RECURSIVE is specified, directories are traversed recursively.
+##
+## Use: scan_scylla_source_directories(VAR my_result_var [RECURSIVE] PATHS [path1 path2 ...])
+##
+function(scan_scylla_source_directories)

+ set(options RECURSIVE)
+ set(oneValueArgs VAR)
+ set(multiValueArgs PATHS)
+ cmake_parse_arguments(args "${options}" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}")
+
+ set(globs "")
+
+ foreach(dir ${args_PATHS})
+ list(APPEND globs "${dir}/*.cc" "${dir}/*.hh")
+ endforeach(dir)
+
+ if(args_RECURSIVE)
+ set(glob_kind GLOB_RECURSE)

+ else()
+ set(glob_kind GLOB)
+ endif()

+
+ file(${glob_kind} var
+ ${globs})
+
+ set (${args_VAR} ${var} PARENT_SCOPE)

+endfunction(scan_scylla_source_directories)
+
+## Although Seastar is an external project, it is common enough to explore the sources while doing
+## Scylla development that we'll treat the Seastar sources as part of this project for easier navigation.
+scan_scylla_source_directories(
+ VAR SEASTAR_SOURCE_FILES
+ RECURSIVE
+
+ PATHS
+ seastar/core
+ seastar/http
+ seastar/json
+ seastar/net
+ seastar/rpc
+ seastar/tests
+ seastar/util)
+
+scan_scylla_source_directories(
+ VAR SCYLLA_ROOT_SOURCE_FILES
+ PATHS .)
+
+scan_scylla_source_directories(

+ VAR SCYLLA_SUB_SOURCE_FILES
+ RECURSIVE

+
+ PATHS
+ api
+ auth
+ cql3
+ db
+ dht
+ exceptions
+ gms
+ index
+ io
+ locator
+ message
+ repair
+ service
+ sstables
+ streaming

+ tests

+ thrift
+ tracing
+ transport
+ utils)

+
+scan_scylla_source_directories(

+ VAR SCYLLA_GEN_SOURCE_FILES
+ RECURSIVE

+ PATHS build/release/gen)

+
+set(SCYLLA_SOURCE_FILES
+ ${SCYLLA_ROOT_SOURCE_FILES}
+ ${SCYLLA_GEN_SOURCE_FILES}
+ ${SCYLLA_SUB_SOURCE_FILES})
+
+add_seastar_executable(
+ NAME scylla
+

+ SOURCES
+ ${SEASTAR_SOURCE_FILES}
+ ${SCYLLA_SOURCE_FILES})

+
+target_include_directories(scylla PRIVATE
+ .

+ build/release/gen)
--
2.9.4

[Duarte Nunes]

How exactly does this affect Clion? Is it just a matter of picking up
what compilation flags were used? If so, I think there's more value in
explicitly providing them. For example, I usually don't compile with
dpdk enabled, but I've enabled -DHAVE_DPDK here; similarly, I always do
--enable-gcc6-concepts, but that's not something that should be enabled
for Clion.

If we do switch from ninja to cmake, will this make sense to include? In
that scenario, Clion should be able to compile everything.

[Jesse Haber-Kucharsky]

On Wed, Jun 21, 2017 at 03:41:50PM +0200, Duarte Nunes wrote:
> How exactly does this affect Clion? Is it just a matter of picking up what
> compilation flags were used? If so, I think there's more value in explicitly
> providing them. For example, I usually don't compile with dpdk enabled, but
> I've enabled -DHAVE_DPDK here; similarly, I always do
> --enable-gcc6-concepts, but that's not something that should be enabled for
> Clion.

Ideally, CLion should show you exactly the code that is being compiled on your system. I know that it will fold sections by default that are disabled by the pre-processor, for example.
Since CLion has its own C++ parser (I believer), concepts support does prevent us from this ideal at the moment.

> If we do switch from ninja to cmake, will this make sense to include? In
> that scenario, Clion should be able to compile everything.

Yes, `add_seastar_executable` could be used to compile a Seastar application correctly in a future if/when we switch. I've used it for Seastar applications in the past.

I'll shelve the function for now and simplify. We really just need `SEASTAR_INCLUDE_DIRS`.

[Jesse Haber-Kucharsky]

This version reads the include paths for Seastar from pkg-config and uses

file globbing to register all source and header files.

In comparison to the previous version, I see all files in the project explorer
view are "active" (rather than just .cc files). I believe there are also fewer
errors reported by the editor.

Note that we assume the project has been compiled with Ninja, or at least that

the pkg-config information for Seastar has been generated. This is already an
implicit requirement of the old version, since sources need to be generated from
the IDL and CQL grammar.

Signed-off-by: Jesse Haber-Kucharsky <jhab...@scylladb.com>
---

Changes since v2:

- Only use pkg-config information for include paths.
---
CMakeLists.txt | 154 ++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 108 insertions(+), 46 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5538efb..9e59826 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,3 +1,12 @@

+##
+## To use this file with CLion, first compile the project using the Ninja build-system.
+##
+## Alternatively, generate only the pkg-config information for Seastar (`seastar.pc`):
+##
+## $ ./configure.py [my configuration options]
+## $ ninja-build -C seastar build/$MODE/seastar.pc
+##
+
cmake_minimum_required(VERSION 3.5)
project(scylla)

@@ -5,51 +14,104 @@ if (NOT DEFINED ENV{CLION_IDE})

+## Populate the names of all source and header files in the indicated paths in a designated variable.
+##
+## When RECURSIVE is specified, directories are traversed recursively.
+##
+## Use: scan_scylla_source_directories(VAR my_result_var [RECURSIVE] PATHS [path1 path2 ...])
+##
+function (scan_scylla_source_directories)
+ set(options RECURSIVE)
+ set(oneValueArgs VAR)
+ set(multiValueArgs PATHS)
+ cmake_parse_arguments(args "${options}" "${oneValueArgs}" "${multiValueArgs}" "${ARGN}")
+
+ set(globs "")
+

+ foreach (dir ${args_PATHS})

+ list(APPEND globs "${dir}/*.cc" "${dir}/*.hh")

+ endforeach()
+
+ if (args_RECURSIVE)

+ set(glob_kind GLOB_RECURSE)
+ else()
+ set(glob_kind GLOB)
+ endif()
+
+ file(${glob_kind} var
+ ${globs})
+
+ set(${args_VAR} ${var} PARENT_SCOPE)

+endfunction()

+add_executable(scylla

+ ${SEASTAR_SOURCE_FILES}
+ ${SCYLLA_SOURCE_FILES})
+
+target_include_directories(scylla PRIVATE
+ .

+ ${SEASTAR_INCLUDE_DIRS}
+ build/release/gen)
--
2.9.4

[Duarte Nunes]

This comment is outdated, right?

[Jesse Haber-Kucharsky]

On Wed, Jun 21, 2017 at 04:56:02PM +0200, Duarte Nunes wrote:
> This comment is outdated, right?

It is, and it could be clarified.

[Jesse Haber-Kucharsky]

This version optionally reads the include paths for Seastar from pkg-config and

uses file globbing to register all source and header files.

In comparison to the previous version, I see all files in the project explorer
view are "active" (rather than just .cc files). I believe there are also fewer
errors reported by the editor.

Signed-off-by: Jesse Haber-Kucharsky <jhab...@scylladb.com>
---

Changes since v3:

- Clarify comment and don't require `seastar.pc` to be present.
---
CMakeLists.txt | 149 +++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 103 insertions(+), 46 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5538efb..edbe827 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,3 +1,7 @@
+##
+## For best results, first compile the project using the Ninja build-system.
+##
+
cmake_minimum_required(VERSION 3.5)
project(scylla)

@@ -5,51 +9,104 @@ if (NOT DEFINED ENV{CLION_IDE})

+pkg_check_modules(SEASTAR seastar)

--
2.9.4

[Vlad Zolotarov]

We also need a seastar/dpdk/lib/librte_eal/common/include

[Duarte Nunes]

That should be in the include directories below, no? Anything containing
source files from dpdk that you feel should be here?

[Duarte Nunes]

I don't see where this is defined. Probably should be "./seastar".

> + build/release/gen)
>

[Jesse Haber-Kucharsky]

On Wed, Jun 21, 2017 at 02:11:56PM -0400, Vlad Zolotarov wrote:
> We also need a seastar/dpdk/lib/librte_eal/common/include

If I'm not mistaken, those headers get populated in `seastar/build/dpdk/include`, which is included through the pkg-config module in `SEASTAR_INCLUDE_DIRS`.

[Jesse Haber-Kucharsky]

On Wed, Jun 21, 2017 at 08:40:16PM +0200, Duarte Nunes wrote:
> I don't see where this is defined. Probably should be "./seastar".

The call to `pkg_check_modules` on line 17 populates the `SEASTAR_*` variables. You can read about it here [1].

[1] https://cmake.org/cmake/help/v3.5/module/FindPkgConfig.html

[Vlad Zolotarov]

Right, missed that target_include_directories thing below - so yes, the long path above should go there (for includes like in reactor.cc).

But now when I think about it we'd rather have the DPDK's library related sources tagged too:

• seastar/dpdk/lib
• seastar/dpdk/drivers

[Jesse Haber-Kucharsky]

On Wed, Jun 21, 2017 at 2:53 PM, Vlad Zolotarov <vl...@scylladb.com> wrote:
> But now when I think about it we'd rather have the DPDK's library related
> sources tagged too:
>
> seastar/dpdk/lib
> seastar/dpdk/drivers

Can we add new source directories as part of a later patch? The
original `CMakeLists.txt` did not index those files.

[Vlad Zolotarov]

sure

[Duarte Nunes]

What if the module isn't there?

[Jesse Haber-Kucharsky]

On Wed, Jun 21, 2017 at 09:32:27PM +0200, Duarte Nunes wrote:
> On 21/06/2017 20:48, Jesse Haber-Kucharsky wrote:
> > The call to `pkg_check_modules` on line 17 populates the `SEASTAR_*` variables.
>

> What if the module isn't there?

Adding

```
# Default value. A more accurate list is populated through `pkg-config` below if `seastar.pc` is available.
set(SEASTAR_INCLUDE_DIRS "seastar")
```

just above `find_package(PkgConfig REQUIRED)` seems to work both when the pkg-config file is available and when it isn't. The benefit of the pkg-config version is that it includes generated files like those for DPDK.

Can I point to a remote tree with an amended commit, or would you prefer another patch version email?

[Duarte Nunes]

On 21/06/2017 21:50, Jesse Haber-Kucharsky wrote:
> On Wed, Jun 21, 2017 at 09:32:27PM +0200, Duarte Nunes wrote:
>> On 21/06/2017 20:48, Jesse Haber-Kucharsky wrote:
>>> The call to `pkg_check_modules` on line 17 populates the `SEASTAR_*` variables.
>>
>> What if the module isn't there?
>
> Adding
>
> ```
> # Default value. A more accurate list is populated through `pkg-config` below if `seastar.pc` is available.
> set(SEASTAR_INCLUDE_DIRS "seastar")
> ```
>
> just above `find_package(PkgConfig REQUIRED)` seems to work both when the pkg-config file is available and when it isn't. The benefit of the pkg-config version is that it includes generated files like those for DPDK.
>

Nice!

> Can I point to a remote tree with an amended commit, or would you prefer another patch version email?
>

Either way is okay (either will be just an extra e-mail :)

[Jesse Haber-Kucharsky]

On Wed, Jun 21, 2017 at 10:08:44PM +0200, Duarte Nunes wrote:
> Nice!

Great! :-)

> > Can I point to a remote tree with an amended commit, or would you prefer another patch version email?
> >
>
> Either way is okay (either will be just an extra e-mail :)

You can find the commit at [1].

[1] https://github.com/hakuch/scylla/tree/jhk/better_cmakelists/v4

[Commit Bot]

From: Jesse Haber-Kucharsky <jhab...@scylladb.com>
Committer: Jesse Haber-Kucharsky <jhab...@scylladb.com>
Branch: master

Improve `CMakeLists.txt` for CLion

This version optionally reads the include paths for Seastar from pkg-config
and
uses file globbing to register all source and header files.

In comparison to the previous version, I see all files in the project
explorer
view are "active" (rather than just .cc files). I believe there are also
fewer
errors reported by the editor.

---
diff --git a/CMakeLists.txt b/CMakeLists.txt
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,55 +1,115 @@

+##
+## For best results, first compile the project using the Ninja
build-system.
+##
+
cmake_minimum_required(VERSION 3.5)
project(scylla)

if (NOT DEFINED ENV{CLION_IDE})
message(FATAL_ERROR "This CMakeLists.txt file is only valid for use in
CLion")
endif()

+# Default value. A more accurate list is populated through `pkg-config`

below if `seastar.pc` is available.

+set(SEASTAR_INCLUDE_DIRS "seastar")
+