format cmake files

[pca006132]

Use cmake-format for formatting, and changed some of the old set calls to list(APPEND ...) which should be nicer.

[pca006132]

This uses https://github.com/cheshirekow/cmake_format

@starseeker see if you have any comments.

The issue now is that I don't have a nice way to check for cmake format in the CI. cmake-format does not provide any error option to exit with error when the files are not properly formatted. I think the simplest thing we can do is to run it in the CI and use git diff to check for differences?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.21%. Comparing base (d437097) to head (8c1b4ea).
Report is 115 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #975      +/-   ##
==========================================
- Coverage   91.84%   88.21%   -3.63%     
==========================================
  Files          37       62      +25     
  Lines        4976     8671    +3695     
  Branches        0     1044    +1044     
==========================================
+ Hits         4570     7649    +3079     
- Misses        406     1022     +616     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[starseeker]

@pca006132 https://github.com/marketplace/actions/cmake-format-lint might be useful here? I'm not 100% sure, but it looks as if they may be checking for differences as a result of formatting. Their code is at https://github.com/neg-c/cmake-format-action

[starseeker]

Did you check https://github.com/BlankSpruce/gersemi as well? It's got a --diff option that might be handy for checking for changes.

[starseeker]

Or actually, in gersemi's case, the -c option might be exactly what we want.

[pca006132]

hmmm I can try that

[pca006132]

yeah gersemi looks better, and easier to integrate with CI

[pca006132]

This is an interesting one, INTERFACE will make the compile option only available to external code linking with our library, so it is not actually using parallelization. Changing to public fixes that.

[pca006132]

and this makes me thinking if we should always set these to some non-zero values and assert the thing should be non-zero in our code, to avoid this sort of issue (and potential typos, which we had before)

[starseeker]

Urk. That's my mistake - I was misunderstanding the PUBLIC vs. INTERFACE settings. I thought PRIVATE was just for the target, PUBLIC was for all other targets in the current CMake project linking that target, and INTERFACE meant everything... I see now that is incorrect, apologies.

For future reference (mostly for me) here's the link to CMake's docs on those three keywords:
https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#target-command-scope

[starseeker]

@pca006132 Were you thinking something like this for asserting non-zero for parallel values? starseeker@c94e8a2

[pca006132]

@starseeker regarding the TODO you placed, I cannot find where tbb said we should not be doing fetch content to download it. Can you give me a reference to the issue/pr?

[elalish]

Thanks!

[starseeker]

@starseeker regarding the TODO you placed, I cannot find where tbb said we should not be doing fetch content to download it. Can you give me a reference to the issue/pr?

I think I was conflating that with some discussions I recollected about issues building it as a static library. You can go ahead and remove that TODO.

[pca006132]

@starseeker regarding the TODO you placed, I cannot find where tbb said we should not be doing fetch content to download it. Can you give me a reference to the issue/pr?

I think I was conflating that with some discussions I recollected about issues building it as a static library. You can go ahead and remove that TODO.

Sure, we can fix that when we fix our documentation in another clean up.