cmake: Sort target_sources alphabetically #12879
Conversation
Creates a gersemi extension that sorts the elements in target_sources alphabetically (and also removes duplicates). This uses case-sensitive sorting as [1] does. The minimum version of gersemi for these extensions is 0.20 as per [2], which is below our current minimum of 0.21. [1]: bd2024e [2]: BlankSpruce/gersemi@4b21bf5
| local -a source_files=($@) | ||
| local file | ||
| local -a command=(${formatter} -c --no-cache ${source_files}) | ||
| local -a command=(${formatter} -c --no-cache ${source_files} --extensions ./build-aux/gersemi-sort.py) |
There was a problem hiding this comment.
This breaks at least two fundamental aspects of the script:
- The script can be run from any working directory and will pick up a sane selection of matching source files from it
- The script can be run from any working directory with an explicit list of files
E.g. you can switch to plugins/aja and just run run-gersemi (run-clang, etc.) from there, and it will pick up all source files and format/check them. And you can also just run run-gersemi <some obs checkout>/plugins/aja/CMakeLists.txt and it will work.
That is by design. So you cannot use a relative path to resolve the extension and use a different path to introduce it to gersemi.
There was a problem hiding this comment.
This should be packaged as a proper Python module, just like the example code.
| # Exclude modified Flatpak files | ||
| # Exclude modified Flatpak and gersemi files | ||
| build-aux/flatpak-github-action-modified-* | ||
| build-aux/__pycache__ |
There was a problem hiding this comment.
This seems unnecessary if we would package the extension as a proper PyPi package and install it into Homebrew's Python site-packages (as we probably should).
|
I'll close this until I can get the proper solution to work. |
2 participants
Description
Creates a gersemi extension that sorts the elements in
target_sourcesalphabetically (and also removes duplicates). This uses case-sensitive sorting as bd2024e does.The minimum version of gersemi for these extensions is 0.20 as per BlankSpruce/gersemi@4b21bf5, which is below our current minimum of 0.21.
Motivation and Context
Sorting of these lists comes up every so often in PR reviews (see e.g. bd2024e or #12735 (comment)). Having it get sorted automatically (and enforced by CI) should make things easier.
In theory, this could be extended to other lists like
target_link_librariesas well, but from what it looks like, those are usually sorted manually based on other criteria (internalOBS::libraries first, etc)How Has This Been Tested?
Ran the check (
./build-aux/run-gersemi -c) and saw many badly formatted files. Ran the formatter (./build-aux/run-gersemi) and the files changed (to what is in this PR). Ran the check again and had no more badly formatted files.Types of changes
Checklist: