Use CMake to download GoogleTest as part of build

[mkilivan]

Having GoogleTest source in project is the hardest approach to keep up
to date and some organizations may not permit this method.

This change is to use CMake to download GoogleTest as part of the
build's configuration step.

It also has the benefit of reducing the number of steps to build the unit
test as unzip is not necessary anymore.

[KjellKod]

Looks good. Care to add something similar to g3sinks once this has passed review?

[mkilivan]

Sure. Happy to do that.

[KjellKod]

Looks good. The only issue I discovered was that with the gtest zip file removed it defaulted to the wrong version of c++ for the gtest compilation which will cause it to fail.

I.e. the following build steps failed

mkdir build && cd build
cmake -DADD_G3LOG_UNIT_TEST=ON .. && make

With issues in gtest.

It can be resolved by adding the c++14 to the CXX Flags on the commandline but that would be PITA.

My suggestion is that you add the following lines to CMakeLists.txt just underneath project(g3log)

# Default for g3log is C++14 support: this is equivalent to "-DCMAKE_CXX_FLAGS=-std=c++14"
add_compile_options(-std=c++14)

This would make sure that currently working compilation steps aren't impacted

[mkilivan]

GCC 4.x doesn't accept the --std=c++14 switch for C++14 code - it takes --std=c++1y instead. See here for more details.

Using CMAKE_CXX_STANDARD would be more generic and it should work cross platform.

It seems this flag has been used for the target ${G3LOG_LIBRARY} in Build.cmake file. You probably want to use a consistent C++ standard for all targets defined in a project, so setting the global the variables would be the most convenient.

# Fix behavior of CMAKE_CXX_STANDARD when targeting macOS.
if (POLICY CMP0025)
  cmake_policy(SET CMP0025 NEW)
endif (POLICY CMP0025)

project (g3log CXX)

set (CMAKE_CXX_STANDARD 14)
set (CMAKE_CXX_STANDARD_REQUIRED ON)
set (CMAKE_CXX_EXTENSIONS OFF)

Any thoughts?

[KjellKod]

Not sure what the CMake policy statement is about, please explain. I prefer fewer details from CMake as CMake is finicky enough. The shorter form necessary seems to be set(CMAKE_CXX_STANDARD 14) set(CMAKE_CXX_STANDARD_REQUIRED ON)

…

Sent from my iPhone

On Jun 3, 2020, at 5:08 AM, Murat Kilivan ***@***.***> wrote: set (CMAKE_CXX_STANDARD 14)

[mkilivan]

If you are using CMake and clang to target macOS there is a bug that can cause the CMAKE_CXX_STANDARD feature to not work. But it seems the bug is fixed and this policy is no longer needed if cmake_minimum_required(VERSION 3.0) used.

Thanks for pointing out. I'll update the patch

[KjellKod]

Thanks @mkilivan