I noticed at least two styles of CMakeLists.txt for modules:
``` ❯ cat src/modules/acc/CMakeLists.txt file(GLOB acc_SRC "*.c")
add_library(acc SHARED ${acc_SRC}) ```
and
``` ❯ cat src/modules/xlog/CMakeLists.txt file(GLOB SRC_FILES "*.c")
add_library(xlog SHARED ${SRC_FILES}) # # target_link_libraries(.*PRIVATE) ```
Which one should be used? The one with `modname_SRC` (e.g., `acc_SRC`) or the one with `SRC_FILES`?
Also, I guess that the comment with `target_link_libraries()` can be removed.
The `${module_name}` is available in the CMakeLists.txt from modules, I tested for acc module with the following CMakeLists.txt content:
``` file(GLOB MODULE_SOURCES "*.c")
add_library(${module_name} SHARED ${MODULE_SOURCES}) ```
What do you think, is it good to use it in the CMakeLists.txt for all modules?
@miconda @henningw
For the `GLOB`ing it doesn't really matter, `acc_SRC` or `SRC_FILES` is just a variable name. When I started implementing this I used the `acc_SRC` but then since I was copy-pasting a lot, I just used a non module-name variable.
We can decide on one, but since we can't enforce it somehow, it will be just a guideline. `MODULE_SOURCES` seems a good candidate indeed.
Regarding the library name itself, that's a good alternative, I think, as well. We might as well promote it to the `modules/CMakLists.txt` to have it defined there and enforce the library name, and just define the sources in the CMake of each module with `target_sources`. Whether this is preferable, I don't know yet.
This might also solve the `3.10` - `3.13` version debate, since the target will be defined in the same `CMakeList` that we try to alter it. The only caveat is that `add_library` requires at least one source file [add_library docs](https://cmake.org/cmake/help/latest/command/add_library.html#add-library) and we can workaround this with `add_library(${module_name} PRIVATE "")`.
Feels a bit hacky, and not sure if there is an unexpected behavior on this.
@xkaraman: I know that technically the result is the same, but it is also about coherence/uniformity and future maintenance. At the end nothing can be enforced, but we have suggestions/recommendations that we provide to developers (the contribution guidelines, code formatting, ... ).
I won't like to go for anything hack-ish, I would prefer to go for clean and modern cmake, to be long term manageable, rather than workarounds that makes it complex and adds maintenance overhead.
I think the add_library() should stay in the module folder CMakeLists.txt, to be clearly identified there.
I think i agree on `add_library` should stay in each module `CMakelists.txt` folder, to be clearly visible.
For code formating there are tools to enforce, that's what I meant. Completely agree in the coherence/uniformity side though and we should do it of course.
``` file(GLOB MODULE_SOURCES "*.c")
add_library(${module_name} SHARED ${MODULE_SOURCES}) # More target definitions and requirements
``` seems reasonable.
Closed #4081 as completed.
Pushed changes based on above comments.
-
Daniel-Constantin Mierla -
Xenofon Karamanos