[CMake] Overhaul; fix several issues#217
Conversation
Update my initial pull request, #193. With learnings from lexbor/lexbor#337. No build file generated with `export()` which is installed.
File needs to be installed.
Possible fixes.
CMake fixes.
This reverts commit 43b7040.
Update to CMake version 3.30 to get rid of [this warning](https://stackoverflow.com/questions/79146083/finding-boost-without-cmake-find-module-cmp0167). Properly quote variables according to [this](https://stackoverflow.com/questions/35847655/when-should-i-quote-cmake-variable-references). There were incosistencies with either `else()` or `else(something)`. Same is true for `endif()`. I adjusted. Use `_CURRENT_` versions of directories, see [here](https://cmake.org/cmake/help/latest/variable/CMAKE_CURRENT_SOURCE_DIR.html). Use placeholders in `mailio-config.cmake.in`. Apply [linter and formatter](https://github.com/cheshirekow/cmake_format). `MAILIO_DYN_LINK_TESTS` makes no sense without `MAILIO_BUILD_TESTS`. Fixed. `CMAKE_THREAD_PREFER_PTHREAD` does not exist. I know of the [StackOverflow question](https://stackoverflow.com/questions/5395309/how-do-i-force-cmake-to-include-pthread-option-during-compilation) but the [documentation](https://cmake.org/cmake/help/latest/module/FindThreads.html) does not contain it (anymore). Fixed. The dependencies were not well defined but rather propagated from the base target via scope `PUBLIC` which is not good practice. I reorganised and only used what is needed. In `set(DOXYGEN_OUTPUT_PATH ${DOCOUTDIR})`, `DOCOUTDIR` does not exist. Does it even make sense when `set(DOXYGEN_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/docs/mailio)` is executed before? I removed it. `configure_file()` has no `IMMEDIATE`. Removed. Use existing varialables instead of defining them for input of `configure_file()`. `version.hpp.in` uses `PROJECT_VERSION_...` instead of `mailio_VERSION_...` to make project renamings easier. Tests and examples were installed even twice. Once in the `CMakeLists.txt` and the individual files `test/CMakeLists.txt` and `examples/CMakeLists.txt`. Removed.
|
The fix turned into an overhaul. The reason was the following. There were errors like this and variables were not used and thus unnecessarily declared which contributed to the build system not being organised and making corrections really hard. General corrections
Specific corrections
To considerI am aware of file TestsEverything is backwards compatible. I tested to build which works. I also imported the created target in on of my applications which worked too. |
Change section order.
`cmake_dependent_option()` is completely useless here and even gives false impression of compatibility checks. The function basically just defines default values and toggles visibility in the GUI, https://cmake.org/cmake/help/latest/module/CMakeDependentOption.html. It does not prevent from configuring incompatible settings. I tested it.
|
Thanks for the PR. It works on Gcc/Linux but fails with Msvc/Windows, with the same CMake 3.31.3. It complains that it cannot find the Boost path although nothing has changed compared to the previous setup. Let me dig what is going on. |
|
Thanks for your general positive attitude towards this pull request. I would like to assist in this but I do not use Windows. |
Update my initial pull request, #193.
With learnings from lexbor/lexbor#337.
No build file generated with
export()which is installed.Also fixed this error, https://stackoverflow.com/questions/25676277/cmake-target-include-directories-prints-an-error-when-i-try-to-add-the-source.