Skip to content
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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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