add targets for cmake-format

[JohelEGP]

Implements https://github.com/cheshirekow/cmake_format/issues/201.

I could use git stash to implement the equivalent targets for format and check-format. Any suggestions on better git tools for that?

This solution works on the .cmake and CMakeLists.txt files of the git repository, whether tracked or not, and uses CMAKE_MODULE_PATH to look for the cmake-format config files.

[TheLartians]

Hey thanks for getting this started! TBH I still have to try cmake-format myself to get to know best practices such as how the configuration files work and where to best put them etc.

I could use git stash to implement the equivalent targets for format and check-format. Any suggestions on better git tools for that?

About git tooling, as far as I understand it the git-clang-format.py script we currently use creates a blob file in memory to run clang-format on and create the diff against. Perhaps it can be copied and adapted for cmake-format as well?

I would also love to see a test for cmake-format as well so we can be sure the script works and runs on both unix and windows platforms. I think the GitHub Actions weren't configured for third-party PRs yet, but if you pull from master now the checks should run on this PR as well.

[JohelEGP]

Thank you. I'll look into that.

[JohelEGP]

I want to add a demo like you have in the README. I made the output coloured since the demo has it, but checking for differences between the tools in GitHub Actions, I noticed that the other's not coloured. I also noticed that it lists the changed files, but I'm not sure how much to make their output resemble. Target check-cmake-format currently outputs the first file that needs to be reformatted, whereas check-format doesn't name any.

As mentioned in the latest comments at https://github.com/cheshirekow/cmake_format/issues/201, I want to change the way the other config files are looked up. At the very least, I want to make it opt-in and default to just what cmake-format supports, like the other targets do for their tool. I'm also considering dropping that out completely and have it be something personal until it's a built-in.

[JohelEGP]

Would you like me to fixup the commits?

[JohelEGP]

Hey thanks for getting this started! TBH I still have to try cmake-format myself to get to know best practices such as how the configuration files work and where to best put them etc.

Consider that I, and probably other users of both CPM and cmake-format, would like CPM to ship with a config file with its commands, like https://github.com/johelegp/jegp/commit/670259f08072a37c1fee1aab033f4c2a9e556c46#diff-a776eb9279ea6a9b2ab5f2380798c020R12. I'm thinking of using FetchContent to make CPM available, rather than the suggested wget (or CMake equivalent with file(DOWNLOAD) that I use) to make the module and config available and up-to-date.

[TheLartians]

Would you like me to fixup the commits?

I'll squash and merge, so it's fine.

[JohelEGP]

One issue I see with the current approach is that cmake-format will miss any configuration files in nested directories. From the cmake-format docs:

If no configuration file is specified on the command line, cmake-format will attempt to find a suitable configuration for eachinputpathby checking recursively checking it's parent directory up to the root of the filesystem.

This would probably break with the format and check targets as we are currently copying and checking source files in the build directory.

I got rid of that logic, and instead opted for using include, as mentioned at https://github.com/cheshirekow/cmake_format/issues/201#issuecomment-631628511, and as can be seen at https://github.com/johelegp/jegp/commit/4dfbe8f5446ba1f53439a36ce12119fe285268c9. I haven't integrated Format.cmake into CI yet, so you can't see it working, but it works on my machine.

Consider that I, and probably other users of both CPM and cmake-format, would like CPM to ship with a config file with its commands

Hm it's a shame that cmake-format doesn't pick up the named arguments automatically. I'll have to think about what a good solution is to distributing the config. What makes this difficult is that CPM.cmake forwards any additional arguments to FetchContent, so the command declaration should somehow "inherit" FetchContent_Declare's declaration to accurately capture all possible parameters.

https://github.com/cheshirekow/cmake_format comes with additional tools. One such is cmake-genparsers, which I used to generate the CPMFindPackage output that can be glimpsed at the commit linked above. It's still limited as documented, so I had to manually add the forwarded arguments I used. But you can use cmake-genparsers to scan your system's FetchContent's and ExternalProject's commands to manually merge them with the output for CPMFindPackage and CPMAddPackage to make them as complete as possible.

I'm thinking of using FetchContent to make CPM available, rather than the suggested wget (or CMake equivalent with file(DOWNLOAD) that I use) to make the module and config available and up-to-date.

A disadvantage of downloading CPM.cmake through CMake is that it would make offline configuration impossible if it isn't already in the build directory.

I see that you are grabbing the most recent version from the master branch. It's better to download from a specific release or commit, as any potential breaking change in CPM.cmake would silently break your configuration script otherwise.

For now, I prefer to live at head.

[JohelEGP]

For the moment, all CMake diffs are done by comparing to a fixed file, and that shows on the head of the diffs. Compere git-clang-format.py's output:

--- a/test/test.cpp
+++ b/test/test.cpp

vs cmake-format.cmake's:

--- a/Users/runner/runners/2.262.1/work/Format.cmake/Format.cmake/test/CMakeLists.txt
+++ b/Users/runner/runners/2.262.1/work/Format.cmake/Format.cmake/build/format/formatted.cmake

Another difference for check-format is that it stops at the error of the first tool, so it's unknown whether the other has anything to say.

[TheLartians]

I got rid of that logic, and instead opted for using include, as mentioned at cheshirekow/cmake_format#201 (comment), and as can be seen at johelegp/jegp@4dfbe8f. I haven't integrated Format.cmake into CI yet, so you can't see it working, but it works on my machine.

Hm not sure I understand how include solves it, but a use case I was thinking of is disabling cmake-format in a specific directory, such as a third-party modules file. In clang-format this is solved by adding a specific .clang_format file to the affected directory and it will be treated differently. As I understand it the same workflow could be used with cmake-format, however not with our current approach as the checked file's location has been changed.

Maybe something like this could work? (untested)

---  execute_process(COMMAND ${CMAKE_COMMAND} -E copy ${source_cmake_file} ${formatted_cmake_file})
---   execute_process(COMMAND ${CMAKE_FORMAT_PROGRAM} -i ${formatted_cmake_file})
+++ execute_process(COMMAND ${CMAKE_FORMAT_PROGRAM} -i ${source_cmake_file} -o ${formatted_cmake_file})

[TheLartians]

For the moment, all CMake diffs are done by comparing to a fixed file, and that shows on the head of the diffs. Compere git-clang-format.py's output. Another difference for check-format is that it stops at the error of the first tool, so it's unknown whether the other has anything to say.

Should be fine for now imo

[TheLartians]

https://github.com/cheshirekow/cmake_format comes with additional tools. One such is cmake-genparsers, which I used to generate the CPMFindPackage output that can be glimpsed at the commit linked above. It's still limited as documented, so I had to manually add the forwarded arguments I used. But you can use cmake-genparsers to scan your system's FetchContent's and ExternalProject's commands to manually merge them with the output for CPMFindPackage and CPMAddPackage to make them as complete as possible.

Good to know, I'll check that out! Then we only have to remember to keep it updated with CMake in case they add additional options.

For now, I prefer to live at head.

Alright, just don't come complaining if we change something and your builds break 😜

[JohelEGP]

As I understand it the same workflow could be used with cmake-format, however not with our current approach as the checked file's location has been changed.

Ah, I forgot to adjust that part to use the correct config file.

Maybe something like this could work? (untested)

Yes, that should work.

[JohelEGP]

I need to git good to make this better and more in line with git-clang-format.py.

[TheLartians]

Looks great, thanks a lot for the contribution! I'll update the readme and create a new release for Version 1.5 after the merge. (I got confused with version numbers, 1.4 is already taken 😅) Once I have the config for CPM.cmake and I figure out how to disable formatting of a directory (relevant issue) I'll add it to the ModernCppStarter as well. :)

[JohelEGP]

Thanks a lot to you too.