gavv
commented
3 years ago Hi, thanks for the PR!
option 'BUILD_STATIC_LIBS' can be removed, cmake already have BUILD_SHARED_LIBS(ON by default). Use -DBUILD_SHARED_LIBS=OFF if you want build static lib
Nice! But then please also create PR for 3rdparty.py in the main repo, and check that it works as expected.
remove erasing C_FLAGS on release build. There is no 'O4' optimization level in gcc/clang and cmake already have preconfigured C_FLAGS for Release/Debug/etc builds.
Yeah, -O4 doesn't exists, it works as -O3. IIRC, default CMake release flags use -O2, and IIRC it was important to build OpenFEC with -O3, not -O2, for performance reasons. I'd prefer to follow the upstream authors intentions and keep using -O3. We can just append it to flags instead of overwriting them.
'OF_DEBUG' define move to 'openfec' target and will be derived by linking this target in debug build only
Again, if we're removing DEBUG option, we should update 3rdparty.py in the main repo. And also we should update openfec README. However, since DEBUG existed in upstream prior to fork (in contrast to BUILD_STATIC_LIBS which is specific to our fork), I think we should keep it. Otherwise, our fork will be incompatible with the upstream, and I'd like to avoid it. We can teach cmake to handle both DEBUG and CMAKE_BUILD_TYPE, so that for debug/release build you can specify either of them. Or we can keep it as is.
BUILD_SHARED_LIBS(ON by default) Use-DBUILD_SHARED_LIBS=OFFif you want build static lib