Refactor CMake to use best practices

[bruxisma]

Hello! I like this library and figured the CMake files could use a bit of a cleanup and partial modernization. Obviously I tried my best to keep it in line with 3.4 through 3.14, however I did take the liberty to break existing options to reduce the chance of a option collision. Please let me know if any other changes are required, I tried very hard to keep it readable and in some cases improve the readability for others. I did test this locally on clang and gcc, but not on windows, as I'm currently traveling and don't have access to my windows machine. Anyhow, the changes involved here include:

Using GNUInstallDirs for some (but not all) install paths

Using generator expressions supported from 3.4 onwards

Using the APPEND_STRING setting for set_property so that we do not overwrite the CMAKE_EXE_LINKER_FLAGS variable.

Collated ALL options (dependent or otherwise) into one location

Modify build options to be "namespaced" to prevent collisions when used via add_subdirectory

Clean up find_package(Python...) so that an actual target is used, rather than a variable

Moved most file(WRITE) commands to file(GENERATE). This reduces a little bit of overhead during the configure stage.

Fix having to output informational target into the root binary dir. We now use file(GENERATE) and $<TARGET_FILE> to get the correct path to the target.

If git submodules update --init has NOT been run, and the FetchContent cmake module is available, test and example dependencies will be cloned as necessary via FetchContent. This will not affect the source directory, but will place the git clones into _deps by default (This is configurable with FetchContent. Please see the cmake documentation for more info)

if CONFIGURE_DEPENDS is available for CLI11_headers, it will now be used. This could help in the future for adding or removing headers from the current library.

CTest is now provided by default, so that BUILD_TESTING is not defined, and enable_testing is not manually called. CTest takes care of this for us.

Removed unnecessary CPack variables, as it will use PROJECT_VERSION by default.

[phlptp]

I am inspired to make some similar changes in some of my other repos!

[codecov]

Codecov Report

Merging #321 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #321   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          12     12           
  Lines        3038   3038           
=====================================
  Hits         3038   3038

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ecce8f...4e0d16b. Read the comment docs.

[bruxisma]

@phlptp I just realized I did need to make sure the CPACK_PACKAGE_VERSION was set via PROJECT_VERSION, so I just did a last minute force push with lease. Once the checks pass (or don't) I'll be able to tackle any remaining issues :)

[phlptp]

@henryiii will need to do the review on this one. I haven't done much with the CMake scripts on this project other than some windows and visual studio specific stuff, but I end up doing quite a bit with CMake on other projects. I will run the changes through some build environments on my system to verify things work as expected. I think a few of the CI scripts will need some modifications particularly in regard to the C++ standard in use, @henryiii would know where better than I do.

[henryiii]

Thanks for working on this! While I'll try to review in more detail soon(ish), but would you be interested in reviewing or contributing to the Modern CMake book or the Modern CMake class I've taught (in the future, it will be proposed to Software Carpentry)?

[henryiii]

Quick note: Feel free to remove json as a submodule; that test can just be activated for modern CMake's only. It has really been irritating that json adds such a large download to the package when you recursively download and when not the main project, the extra modules are not needed. You might have an issue with https downloads, that's why all submodules are relative.

I'd be okay bumping the minimum CMake requirement a little bit for 2.0 (and maybe a tiny bit for 1.9).

[bruxisma]

Hi sorry! This got lost in a huge list of PRs because I just can't stand by and look at bad cmake anymore and its killing me so I'm just trying to fix everything at this point 🤣. Anyhow!

I'll look into what I can contribute regarding the book and workshop. Anything I can do that helps prevent mistakes from being made in CMake projects is a big plus :)

As for the changes mentions, I'm well aware of the cost of getting the json submodule. That said, for tests there's no reason why FetchContent with a git clone can't be used. For the CMake minimum version bump I'd recommend looking into 3.10, as that's the default installed on 18.04 Ubuntu (and friends), but I'll leave it up to you. The more recent the version the simpler I can make the build file. :)

I'll look into if we can do an SSH clone or something similar to prevent any networking issues, though the only time its ever been an issue for me on things like travis, I was able to create a cached directory and use that during the configure step, but I don't have the access for that sort of thing 😅

[phlptp]

For a point of reference, on several of our projects we made the decision that we would update to whatever is supported on Ubuntu 18.04 once 20.04 came out and RHEL 8.0 had been out a year. So our plan is to update the minimum to cmake 3.10, and the associated minimum compilers as well. I am looking forward to fully being able to use C++17 once that happens. I don't know that it matters as much for CLI11 since it works fine as a single header library so the cmake is somewhat less critical. Not sure when you would think to move to a newer C++ version?

[bruxisma]

@phlptp I'd prefer to make at least an INTERFACE library available for find_package calls or via add_subdirectory. This allows some CMake "package managers" to just work ™, which I think is healthy for the general cmake based project ecosystem. I'm glad y'all are at least interested in staying fairly current, it makes my life easier when I swoop in to improve a project's CMake files 🤣. C++20 is going to be a massive change to how build files are handled, and the C++ standards committee is still trying to figure out how we're going to handle it all at the tooling level. I think it will be some time before you can safely move CLI11 off of C++17. Such is life 😬

[henryiii]

Edit: that's not what this is doing. Why is this reading then writing the same property?

[bruxisma]

Oh this might have been a small check on my part and I forgot to remove the whole else() block 😅

[henryiii]

Please rebase and force-with-lease, please. CI should be fixed. You might need to run pre-commit, but unlikely.

I can do it if you want me to. We should add a github actions or azure check with the minimum CMake version to be sure this works.

[bruxisma]

Will do! Need a few days, as I've been swamped with personal life 😅