Older generated CMakeLists.txt file incompatible with newer Drogon Builds

[gschmottlach-xse]

I have encountered an issue recently while upgrading my project's Drogon repo to master in order to get WebSocket fixes and address memory leaks in that component.

I am developing under Ubuntu 18.04 with the default G++ 7.5.0 compiler release. With this GNU release I require C++ 17 support. When I generated this project months ago using drogon_ctl it created an initial CMakeLists.txt file which I've extended as my project grew. At that time, generating the "Makefile" using mkdir build; cd build; cmake .. generates the appropriate Makefiles for my application and the Drogon submodule. The output indicated both components had selected the C++17 compiler.

After updating Drogon to master, my application's CMakeLists.txt continued to select the C++17 compiler but Drogon now selected C++14 as the compiler option when building the framework. Although the appliction built successfully, it seg-faults whenever an attribute is added to an HttpRequest object. Specially, it seg-faults when the HttpRequest is destroyed (and dies in the ~std::any destructor). I spent over a day tracking down this issue to discover it's a problem of combining my C++17 application code with the C++14 Drogon-compiled framework.

As I mentioned previously, Drogon used to select the C++17 compiler (while using G++ 7.5.0) but as of ~July 2021 it falls back to G++14. This seems to be driven by whether it can/cannot find the standard header file for std::filesystem. The existence of this header file seems to now determine whether it selects between DROGON_CXX_STANDARD 14 or 17. Unfortunately, G++ 7.5.0 and 8.4.0 provides these features under an "experimental" path and Drogon's simple tests fails to find this feature and then uses this determination to choose a C++ version. I do not believe this is a very robust or the correct method to choose the C++ version. Certainly it can be used to select between whether to use a standard feature or a Boost replacement but not to set the C++ version.

These are my concerns:

1. Drogon made a change in it's CMakeLists.txt file which makes it incompatible with older generated CMakeLists.txt files genarated by drogon_ctl. A notice or warning in the release notes would have been helpful to know that the project level CMakeLists.txt files should be re-generated.

2. Selecting the DROGON_CXX_STANDARD version is now based in part on the availability of std::filesystem header. It seems to be very fragile and not very accommodating of older compiler versions (e.g GCC 7.5.0 or GCC 8.4.0). Although marked as experimental features, std::filesystem is still supported in these versions.

Apparently, other projects that need/want to use std::filesytem have encountered similar problems and have developed Cmake modules to correctly enable this support. I would like to refer you to one such implementation:

FindFilesystem

I hope you would consider integrating the FindFileSystem module support into Drogon's cmake build system and eliminate it as a criteria on whether C++14 or 17 is selected. Instead, I believe it should be treated as a feature to determine whether built-in (standard) support is used or the one offered through Boost. I believe this approach might significantly simplify and increase the robustness of Drogon's CMakeLists.txt code that tries to identify the appropriate namespace and include path for std::filesystem support. I am interested in your thoughts. I'd rather avoid forking this project to get the WebSocket fixes and still build my project with the C++17 compiler. Your insights and recommendations would be appreciated.

Thanks for the great framework . . .

[rbugajewski]

Thanks for your valuable feedback! These are all valid concerns that we should address in the future, but also a bug that we should fix on an earlier schedule.

[gschmottlach-xse]

I suspect the root-cause of the seg-fault in ~std::any is that Drogon is using Boost::any (since it's now selecting C++14 for GNU C++ versions 7.5.0/8.4.0) and my application was using the standard (non-Boost) std::any implementation. It's tough to share template definitions that are not necessarily compatible between the application code and the framework (Drogon) code. I think this is a larger general problem when compiler options are not consistent between the application and the framework. This is only made worse when template implementations differ since they're solely defined in a header file.

Right now it seems the Drogon CMakeLists.txt file tries to choose a compatible C++ version based on the features it needs (e.g. HAS_ANY, HAS_STRING_VIEW, HAS_COROUTINE, HAS_FILESYSTEM). This seems problematic in general since it doesn't seem to respect an application-level setting for CMAKE_CXX_STANDARD which picks the C++ standard the application might need. It's a bit of a guess what C++ version Drogon will ultimately choose. It might be better if the developer selects the C++ standard and lets Drogon tell you whether it can/cannot be compiled against that standard by seeing if the necessary features can be supported. Likewise, for things like Boost types that are shared between the application and the Drogon framework, there has to be consistency in the types that are used. Maybe the developer must explicitly choose to use Boost (or not) to gain access to a feature. This article offers some guidance on this topic with respect to cmake and setting the C++ standard.

Unfortunately, I'll probably have to fork the project in the interim to get it working again with my application so a consistent C++ environment is defined between my application and the framework. I am interested, however, in what changes you make in the future to address this. Perhaps I can later switch back to upstream branches when your work is complete.

[an-tao]

To avoid the any and string_view version conflict, I only use them in public API headers, that's say the version of them only depend on the c++ standard set by the application. Maybe there are some omissions, you could figure it out in your case.

Compatibility is a complex issue, we will find a solution, @rbugajewski @marty1885.

Thanks so much for your feedback, if you has any idea, please feel free to post here. And if you make a PR for this, it would be greatly appreciated.

[rbugajewski]

Thanks for your feedback and valuable input. I think this subject is quite complex, and has several facets we didn’t talk about yet.

This seems problematic in general since it doesn't seem to respect an application-level setting for CMAKE_CXX_STANDARD which picks the C++ standard the application might need. It's a bit of a guess what C++ version Drogon will ultimately choose. It might be better if the developer selects the C++ standard and lets Drogon tell you whether it can/cannot be compiled against that standard by seeing if the necessary features can be supported.

The discussion here is shifting into the “What is a framework and what is a library?“ territory, and I have the feeling that you are trying to use Drogon “just“ as a library (being maybe part of a bigger project), while Drogon itself positions as a framework (which are by definition slightly more intrusive, but according to this definition Drogon is the application you build around, and not the other way around).

We’ll continue to keep an eye on this topic, as we also support many different platforms with different environments. If you have any concrete ideas for improvements or maybe even a PR, I could assist there if time allows.

On another point, I think that the complexity of our CMake file is getting slightly out of hand, but I digress.

[gschmottlach-xse]

Let me preface this discussion by saying I've only been using Drogon for a few weeks but with regards to this statement:

The discussion here is shifting into the “What is a framework and what is a library?“ territory, and I have the feeling that you are trying to use Drogon “just“ as a library (being maybe part of a bigger project), while Drogon itself positions as a framework (which are by definition slightly more intrusive, but according to this definition Drogon is the application you build around, and not the other way around).

In my opinion, Drogon sits squarely as library since it does not contain a main() function and drogon_ctl can be used to create projects where Drogon can literally be treated as a separately built submodule (see the project generated CMakeLists.txt). I also see discussions of bundling Drogon as a Debian package and when used in an embedded Linux setting with a framework like Yocto, Drogon has it's own recipe and is typically built as a shared library. As such, it needs to respect the distro's C++ build options with regards to C++ versions, optimizations etc... Typically, using a framework like Yocto, there are some distro defined optimization settings that all "recipes" inherit and can then optionally tweak.

What I'm suggesting, is that instead of Drogon trying to select a C++ version based on the features it can find, let the application developer choose the C++ standard and then Drogon can enable those features that are supported or discovered by CMake. So, looking at Drogon's top-level CMakeLists.txt, instead of this

check_include_file_cxx(any HAS_ANY)
check_include_file_cxx(string_view HAS_STRING_VIEW)
check_include_file_cxx(coroutine HAS_COROUTINE)
check_include_file_cxx(filesystem HAS_FILESYSTEM)
if (HAS_ANY AND HAS_STRING_VIEW AND HAS_COROUTINE AND HAS_FILESYSTEM)
    set(DROGON_CXX_STANDARD 20)
elseif (HAS_ANY AND HAS_STRING_VIEW AND HAS_FILESYSTEM)
    set(DROGON_CXX_STANDARD 17)
else ()
    set(DROGON_CXX_STANDARD 14)
endif ()

It might more sense to just replace it with this:

set(DROGON_CXX_STANDARD ${CMAKE_CXX_STANDARD})
message(STATUS "DROGON CXX standard = ${DROGON_CXX_STANDARD}")

Let an application-level CMakeLists.txt set the CMAKE_CXX_STANDARD and Drogon will follow if it's built as a submodule. Likewise it might follow a distro default and can always complain if that version cannot be supported. It seems like a lot of the code uses the __cplusplus >= XXXXXX to determine what actual features or headers to include in the source. The DROGON_CXX_STANDARD is used to decide which elements of the Boost library to substitute for missing standard features. I think some of the complexity in the Drogon CMakeLists.txt should be pushed into custom cmake modules to "find" or offer substitutions for the features Drogon is interested in (like any, filesystem, coroutine). Perhaps these modules absorb some of the complexity and help simplify the top-level Drogon CMakeLists.txt.

[rbugajewski]

I think some of the complexity in the Drogon CMakeLists.txt should be pushed into custom cmake modules to "find" or offer substitutions for the features Drogon is interested in (like any, filesystem, coroutine). Perhaps these modules absorb some of the complexity and help simplify the top-level Drogon CMakeLists.txt.

I’m absolutely with you on this one.

[marty1885]

#993 Should handle the issue. Let me know if I missed anything

• C++ Version detection no longer checks for std::fs
• User can now specify their desired version
• Checks if c++fs is needed in DrogonConfig (thanks for the tip for FindFilesystem!)