lwlog icon indicating copy to clipboard operation
lwlog copied to clipboard

Add CMake support

Open ChristianPanov opened this issue 3 years ago • 12 comments

ChristianPanov avatar Dec 08 '21 00:12 ChristianPanov

hi!

i have done some very rough work:

cmake_minimum_required(VERSION 3.11)

project(lwlog
  LANGUAGES CXX
  VERSION 0.1
)

find_package(Threads)

option(LWLOG_BUILD_EXAMPLE "Add sandbox example to the project (default on)" ON)


file(GLOB_RECURSE sources CONFIGURE_DEPENDS src/lwlog/*.h src/lwlog/*.cpp)
add_library(lwlog STATIC ${sources})
target_include_directories(lwlog PUBLIC src)
target_compile_features(lwlog PUBLIC cxx_std_17)

source_group(TREE "${PROJECT_SOURCE_DIR}/src" FILES ${sources})

if(LWLOG_BUILD_EXAMPLE)
    add_executable(lwlog_example Sandbox/Benchmark.h Sandbox/Sandbox.cpp)
    target_compile_features(lwlog_example PUBLIC cxx_std_17)
    target_link_libraries(lwlog_example PRIVATE lwlog)
endif()

edit: updated from set_property to target_compile_features

Changes required for this cmake file to work:

  • rename lwlog to src
  • rename lwlog/src to src/lwlog
  • in Sandbox/Sandbox.cpp change #include "lwlog.h" to #include "lwlog/lwlog.h"
  • Delete project files in Sandbox (optional)
  • Delete premake files (optional)

This way the project seems to build nicely, I haven't tried yet to use it together with find_package and fetch_content. Well, it's a starting point :)

kritzikratzi avatar Dec 26 '21 00:12 kritzikratzi

target_compile_features(lwlog PUBLIC cxx_std_17)

will ensure that any targets that link to this target inherit that C++17 requirement and means you can stop manually setting target properties.

ChrisThrasher avatar Dec 26 '21 22:12 ChrisThrasher

@ChrisThrasher what's the better alternative?

kritzikratzi avatar Dec 27 '21 07:12 kritzikratzi

target_compile_features is the most modern and idiomatic solution.

ChrisThrasher avatar Dec 27 '21 16:12 ChrisThrasher

thanks! i've edited my original post to accomodate the change. any other suggestions?

@ChristianPanov what do you think? could you live with my proposed folder renaming (rename lwlog/src to src/lwlog and use src/ as base include path). if yes i'm happy to make a PR out of this.

kritzikratzi avatar Dec 27 '21 17:12 kritzikratzi

Looks like the Threads package isn't being used

ChrisThrasher avatar Dec 27 '21 17:12 ChrisThrasher

@ChrisThrasher threads are used in a few places https://github.com/ChristianPanov/lwlog/search?q=thread afaik all this does is add "-lpthread" to the linker.

ps. oh, by "isn't being used" you mean that i forgot to target_link_libraries(lwlog Threads::Threads)? sorry, tested only msvc so far an i guess it doesn't matter there.

kritzikratzi avatar Dec 27 '21 18:12 kritzikratzi

@kritzikratzi Hey, sorry for the late reply, I've been very busy these days. Yes, it looks very nice, The folder renaming is no big deal, but can you elaborate on why it's necessary? Also, I have -O2 optimizations, could you add that? Also yes, I would be very happy if you can make a PR, make sure to make it for the dev branch Thank you for the contribution!

ChristianPanov avatar Dec 27 '21 19:12 ChristianPanov

Finding the Threads package won't automatically add -lpthread to the linker line. You need to link against the Threads::Threads target to get that.

ChrisThrasher avatar Dec 27 '21 20:12 ChrisThrasher

https://github.com/ChristianPanov/lwlog/pull/21

ChrisThrasher avatar Dec 27 '21 21:12 ChrisThrasher

looks good. at least i can fetch & compile with no problems (tested in vs2019).

there is a mistmatch between docs and implementation now.

  • docs: #include "lwlog/lwlog.h"
  • reality: #include "lwlog.h"

kritzikratzi avatar Dec 28 '21 17:12 kritzikratzi

@ChristianPanov cmake takes cares of optimization already.

for instance when generating xcode project files i get the following schemes:

mkdir build_osx
cd build_osx
cmake -G Xcode ..
open lwlog.xcodeproject
Screenshot 2021-12-29 at 17 01 29

for some generators (makefile) you cannot have multiple schemes/configurations. so say you want to create a release configuration you might want to do something like:

mkdir build_linux_release
cd build_linux_release
cmake -G "Unix Makefiles" .. -DCMAKE_BUILD_TYPE=Release
make

this is mostly of interest to you and a tiny handful of contributors, most people would let cmake take care of building the library.

A) add lwlog by making a copy, or as a git submodule. tell cmake only about where you keep the dependency.

to do so you'd add roughly this to your own projects CMakeLists.txt:

...
add_subdirectory(dependencies/lwlog)
target_link_libraries(my_cool_project
  PRIVATE lwlog::lwlog
)
...

B) use fetchcontent and let cmake take care of it alltogether

# --- Fetch lwlog --------------------------------------------------------------
include(FetchContent)

set(FETCHCONTENT_UPDATES_DISCONNECTED TRUE)
FetchContent_Declare(lwlog
  GIT_REPOSITORY https://github.com/ChristianPanov/lwlog
  GIT_TAG aed908843a23487ee31aba25318aaa73b43d1d18
)

FetchContent_GetProperties(lwlog)
if(NOT lwlog_POPULATED)
  FetchContent_Populate(lwlog)
  add_subdirectory(${lwlog_SOURCE_DIR} ${lwlog_BINARY_DIR} EXCLUDE_FROM_ALL)
endif()


...
target_link_libraries(my_cool_project
  PRIVATE lwlog::lwlog
)
...

both are untested, but both should approximately work (and imho be part of the docs ^^)

kritzikratzi avatar Dec 29 '21 16:12 kritzikratzi