Skip to content
New issue

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

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

Already on GitHub? # to your account

clang-tidy: Ignore the external directory #60640

Merged
merged 1 commit into from
Feb 19, 2025

Conversation

ptitjano
Copy link
Collaborator

@ptitjano ptitjano commented Feb 17, 2025

Description

The external directory contains code for external librairies. It
should not be inspected by clang-tidy.

The regex syntax does not allow to directly ignore a directory. Solve
this issue by adding a clang-tidy config file in the external
directory which disables all the checks.

See:
https://stackoverflow.com/questions/74349432/clang-tidy-exclude-specific-dir-from-analysis
See:
https://stackoverflow.com/questions/58338202/cmake-clang-tidy-disable-checking-in-directory
See: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/777/diffs

cc @troopa81 @benoitdm-oslandia

@ptitjano ptitjano added the CI/GitHub-Actions issues related to GitHub Actions label Feb 17, 2025
@ptitjano ptitjano self-assigned this Feb 17, 2025
@github-actions github-actions bot added this to the 3.42.0 milestone Feb 17, 2025
Copy link
Collaborator

@benoitdm-oslandia benoitdm-oslandia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link

github-actions bot commented Feb 17, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 0acff83)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 0acff83)

@troopa81
Copy link
Contributor

@ptitjano did you test that clang-tidy was still able to spot issue in regular folders?

@ptitjano ptitjano force-pushed the clang-tidy-ignore-external branch from b0db059 to be8b79b Compare February 17, 2025 18:45
@ptitjano
Copy link
Collaborator Author

@ptitjano did you test that clang-tidy was still able to spot issue in regular folders?

Yes, tested in #60599 for example.

@ptitjano
Copy link
Collaborator Author

@troopa81 It looks like the clang-tidy CI does not work at the moment. One get the following error on each inspected file:

error: PCH file uses an older PCH format that is no longer supported [clang-diagnostic-error]

See for example:

This has nothing to do with the change introduced here.

The external directory contains code for external librairies. It
should not be inspected by clang-tidy.

The regex syntax does not allow to directly ignore a directory. Solve
this issue by adding a `clang-tidy` config file in the external
directory which disables all the checks.

See:
https://stackoverflow.com/questions/74349432/clang-tidy-exclude-specific-dir-from-analysis
See:
https://stackoverflow.com/questions/58338202/cmake-clang-tidy-disable-checking-in-directory
See: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/777/diffs
@ptitjano ptitjano force-pushed the clang-tidy-ignore-external branch from be8b79b to 0acff83 Compare February 19, 2025 08:45
@ptitjano
Copy link
Collaborator Author

@troopa81 It looks like the clang-tidy CI does not work at the moment. One get the following error on each inspected file:

error: PCH file uses an older PCH format that is no longer supported [clang-diagnostic-error]

See for example:

* https://github.com/qgis/QGIS/actions/runs/13362091067/job/37314779975?pr=60627

* https://github.com/qgis/QGIS/actions/runs/13340766744/job/37265791702?pr=60605

* https://github.com/qgis/QGIS/actions/runs/13374511384/job/37355486087?pr=60646

This has nothing to do with the change introduced here.

This has been solved in #60659

This PR is ready now.

@ptitjano ptitjano merged commit ac21a8d into qgis:master Feb 19, 2025
31 checks passed
@ptitjano ptitjano deleted the clang-tidy-ignore-external branch February 19, 2025 10:25
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
CI/GitHub-Actions issues related to GitHub Actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants