Remove warnings as errors compile flag from project CMakeLists.txt

[midget214]

Description

Deciding to treat warnings as errors should be left to the consumer compiling the library rather than being enforced by the compile flags set inside the CMakeLists.txt file. Producing no compiler warnings is a goal every developer should strive for, it is impossible to check every possible configuration that consumers of a library may have.

Currently on our environment the following warnings are being produced causing the build to fail

libCPR\src\cpr\error.cpp(167,19): error: no newline at end of file [-Werror,-Wnewline-eof]

[build]   167 | } // namespace cpr

 

libCPR\src\include\cpr/threadpool.h(85,70): error: declaration shadows a local variable [-Werror,-Wshadow-uncaptured-local]

[build]    85 |         auto task = std::make_shared<std::packaged_task<RetType()>>([fn = std::forward<Fn>(fn), args...]() mutable { return std::invoke(fn, args...); });

Example/How to Reproduce

1. Setup an environment as described in additional context section
2. Attempt to compile

Possible Fix

Follow modern CMake best practices by not turning warnings into errors inside the CMakeLists.txt file. Users / CI builds can turn this on by either setting COMPILE_WARNING_AS_ERROR variable on the command line or by setting the 'CMAKE_COMPILE_WARNING_AS_ERROR' cache variable if using presets

For info on this setting can be found here

Where did you get it from?

GitHub (branch e.g. master)

Additional Context/Your Environment

• OS: Windows 10
• Compiler: Clang for windows version 17.0.3
• Build System: Ninja
• libCPR Version: 1.11.1

[COM8]

@midget214 thanks for reporting! I agree this makes sense. Up until now I was not aware that something like this exists.
We could even combine it with PROJECT_IS_TOP_LEVEL to turn it on when cpr is the top level project.

It requires CMake 3.24, which should be fine since the oldest supported debian version is 3.25.

Would you like to create a MR for it?

[midget214]

Sure thing. Before I submit an MR I would just like to confirm a few details first.

1. You already mentioned that the cmake version requirement would need to be bumped up. In our setup that's fine as we try to keep up to date with major cmake releases. It might be worth considering a workaround for consumers who are on a lower version of cmake. The aws-sdk-cpp project for example uses a custom variable to drive warnings as errors. That does however move away from the standard way cmake does things. One option could be to use the cmake COMPILE_WARNING_AS_ERROR variable when version 3.24 or higher is present and a custom variable on lower environments. The downside to this is inconsistent behavior between cmake versions. Please let me know what you would prefer.

2. Atleast the way we use it, if warnings are forced as errors when cpr is the top level project it would still fail compilation. If you would prefer that warnings are treated as errors by default we could set the COMPILE_WARNING_AS_ERROR to ON inside the CMakeLists.txt file via the OPTION command. This would still allow consumers to override it if they need to.