Skip to content

cmake: replace cmakelint with cmake-lint from cmakelang, fix issues #17576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 38 commits into from

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Jun 10, 2025

This linter detects formatting and naming issues with minimal amount of
noise. It seems to be an improvement over the existing linter which was
only detecting line width overruns.

Also: fix to exclude CurlTests.c from linter.

Ref: https://cmake-format.readthedocs.io/en/latest/cmake-lint.html
Ref: https://github.com/cheshirekow/cmake_format


TODO:

@vszakats vszakats added the cmake label Jun 10, 2025
@github-actions github-actions bot added tests CI Continuous Integration labels Jun 10, 2025
@vszakats vszakats marked this pull request as draft June 10, 2025 18:35
@vszakats vszakats changed the title cmake: replace cmakelint with cmake-lint from cmakelang cmake: replace cmakelint with cmake-lint from cmakelang, fix warnings Jun 10, 2025
@vszakats vszakats changed the title cmake: replace cmakelint with cmake-lint from cmakelang, fix warnings cmake: replace cmakelint with cmake-lint from cmakelang, fix warnings Jun 10, 2025
@testclutch

This comment was marked as outdated.

@vszakats vszakats marked this pull request as ready for review June 10, 2025 20:53
@vszakats vszakats changed the title cmake: replace cmakelint with cmake-lint from cmakelang, fix warnings cmake: replace cmakelint with cmake-lint from cmakelang, fix issues Jun 10, 2025
vszakats added a commit that referenced this pull request Jun 11, 2025
Delete macros from `curl_config.h.cmake` that were never set by
the CMake script: `_LARGE_FILES`, `_THREAD_SAFE`, `const`, `size_t`.

Also:
- lib/config-riscos.h: drop `#undef _LARGE_FILES`. This is an
  IBM-specific macro, no need to unset it on other platforms.

Cherry-picked from #17576

Closes #17580
vszakats added 19 commits June 11, 2025 06:33
``
CMakeLists.txt:2034,35: [W0106] String looks like a variable reference missing an open tag '{PROJECT_SOURCE_DIR'
CMakeLists.txt:2035,35: [W0106] String looks like a variable reference missing an open tag '{PROJECT_BINARY_DIR'
```
```
CMake/PickyWarnings.cmake:343,08: [C0103] Invalid directory variable name ""${_wlist}"" doesn't match `([A-Z][0-9A-Z_]+|[A-Z][A-Za-z0-9]+_FOUND|[a-z]+_SOURCES)|_[0-9a-z_]+`
```
The warning seems dubious, because `_GSS_VERSION` is defined by `pkg_search_module()`,
but either I'm misunderstanding something, or cmake-lint is not recognizing this.

```
CMake/FindGSS.cmake:145,10: [C0103] Invalid directory variable name "_GSS_VERSION" doesn't match `([A-Z][0-9A-Z_]+|[A-Z][A-Za-z0-9]+_FOUND|[a-z]+_SOURCES)|_[0-9a-z_]+`
CMake/FindGSS.cmake:273,10: [C0103] Invalid directory variable name "_GSS_VERSION" doesn't match `([A-Z][0-9A-Z_]+|[A-Z][A-Za-z0-9]+_FOUND|[a-z]+_SOURCES)|_[0-9a-z_]+`
CMake/FindGSS.cmake:279,10: [C0103] Invalid directory variable name "_GSS_VERSION" doesn't match `([A-Z][0-9A-Z_]+|[A-Z][A-Za-z0-9]+_FOUND|[a-z]+_SOURCES)|_[0-9a-z_]+`
CMake/FindGSS.cmake:285,10: [C0103] Invalid directory variable name "_GSS_VERSION" doesn't match `([A-Z][0-9A-Z_]+|[A-Z][A-Za-z0-9]+_FOUND|[a-z]+_SOURCES)|_[0-9a-z_]+`
```
vszakats added 19 commits June 11, 2025 06:33
Fixing:
```
docs/CMakeLists.txt:35,23: [C0113] Missing COMMENT in statement which allows it
docs/CMakeLists.txt:41,22: [C0113] Missing COMMENT in statement which allows it
docs/cmdline-opts/CMakeLists.txt:28,19: [C0113] Missing COMMENT in statement which allows it
docs/cmdline-opts/CMakeLists.txt:36,18: [C0113] Missing COMMENT in statement which allows it
docs/examples/CMakeLists.txt:25,18: [C0113] Missing COMMENT in statement which allows it
docs/libcurl/CMakeLists.txt:43,25: [C0113] Missing COMMENT in statement which allows it
docs/libcurl/CMakeLists.txt:67,19: [C0113] Missing COMMENT in statement which allows it
docs/libcurl/CMakeLists.txt:79,18: [C0113] Missing COMMENT in statement which allows it
docs/libcurl/opts/CMakeLists.txt:29,18: [C0113] Missing COMMENT in statement which allows it
scripts/CMakeLists.txt:33,25: [C0113] Missing COMMENT in statement which allows it
scripts/CMakeLists.txt:39,24: [C0113] Missing COMMENT in statement which allows it
scripts/CMakeLists.txt:44,25: [C0113] Missing COMMENT in statement which allows it
scripts/CMakeLists.txt:50,24: [C0113] Missing COMMENT in statement which allows it
src/CMakeLists.txt:34,04: [C0113] Missing COMMENT in statement which allows it
src/CMakeLists.txt:58,06: [C0113] Missing COMMENT in statement which allows it
tests/CMakeLists.txt:32,18: [C0113] Missing COMMENT in statement which allows it
tests/CMakeLists.txt:62,20: [C0113] Missing COMMENT in statement which allows it
tests/CMakeLists.txt:79,20: [C0113] Missing COMMENT in statement which allows it
tests/certs/CMakeLists.txt:28,19: [C0113] Missing COMMENT in statement which allows it
tests/certs/CMakeLists.txt:33,18: [C0113] Missing COMMENT in statement which allows it
tests/certs/CMakeLists.txt:36,18: [C0113] Missing COMMENT in statement which allows it
tests/http/clients/CMakeLists.txt:25,18: [C0113] Missing COMMENT in statement which allows it
tests/libtest/CMakeLists.txt:31,02: [C0113] Missing COMMENT in statement which allows it
tests/libtest/CMakeLists.txt:40,04: [C0113] Missing COMMENT in statement which allows it
tests/server/CMakeLists.txt:35,04: [C0113] Missing COMMENT in statement which allows it
tests/tunit/CMakeLists.txt:31,04: [C0113] Missing COMMENT in statement which allows it
tests/unit/CMakeLists.txt:31,04: [C0113] Missing COMMENT in statement which allows it
```
In most cases it seems to make little sense to add these.

```
    # A custom command with one output doesn't really need a comment because
    # the default "generating XXX" is a good message already.
```
Also disabled in the tool itself:
https://github.com/cheshirekow/cmake_format/blob/master/.cmake-format.py
Better this way because disable= directives apply to the whole
file, not just the following line (or lines?)
It's a `#cmakedefine VAR ${VAR}` case where the two VARs must be the same
string to work as expected.
Sadly this requires passing `--config-files` after the input files,
which needs a xargs hack, which invokes the tool for each file, making
it significantly slower.
@vszakats vszakats closed this in b761eb5 Jun 11, 2025
@vszakats vszakats deleted the cm-lint branch June 11, 2025 05:08
denandz pushed a commit to denandz/curl that referenced this pull request Jun 21, 2025
Delete macros from `curl_config.h.cmake` that were never set by
the CMake script: `_LARGE_FILES`, `_THREAD_SAFE`, `const`, `size_t`.

Also:
- lib/config-riscos.h: drop `#undef _LARGE_FILES`. This is an
  IBM-specific macro, no need to unset it on other platforms.

Cherry-picked from curl#17576

Closes curl#17580
denandz pushed a commit to denandz/curl that referenced this pull request Jun 21, 2025
…sues

This linter detects formatting and naming issues with minimal amount of
noise. It seems to be an improvement over the existing linter which was
only detecting line width overruns.

Also: fix to exclude `CurlTests.c` from linter.

Ref: https://cmake-format.readthedocs.io/en/latest/cmake-lint.html
Ref: https://github.com/cheshirekow/cmake_format

Closes curl#17576
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration cmake tests
Development

Successfully merging this pull request may close these issues.

2 participants