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

PS-9040 Integrate clang-tidy checks for Percona server #5194

Closed
wants to merge 1 commit into from

Conversation

VarunNagaraju
Copy link
Contributor

https://perconadev.atlassian.net/browse/PS-9040

Creates a yaml file for clang-tidy workflow which is triggered on a pull request.

The workflow triggers a run of generating comiplation commands and uses the same to perform a clang-tidy check on the modified files and generates a report of the warnings in the changed code through a github action bot in the form of review comments on the pull request.

@VarunNagaraju
Copy link
Contributor Author

After integration, the clang-tidy warnings will be shown as review comments like VarunNagaraju#1. In any case, one can also choose to ignore addressing the comments and the push/merge option will NOT be disabled.

The comments will be posted only for the snippets of the code which are modified by the PR. But, clang-tidy will be run for the whole file(on all the files modified by the PR) and the clang-tidy log for that can be found in the Checks section or by clicking on the Show all checks > Static analysis > Details > Analyze in the workflow.

- name: Install clang-tidy
run: |
sudo apt-get update
sudo apt-get install -y clang clang-tidy libldap2-dev curl libcurl4-openssl-dev bison
Copy link
Collaborator

Choose a reason for hiding this comment

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

At https://github.com/VarunNagaraju/percona-server/actions/runs/7345240731/job/19998118252 I found:

CMake Warning at cmake/fido2.cmake:69 (MESSAGE):
  Cannot find development libraries.  You need to install the required
  packages:

    Debian/Ubuntu:              apt install libudev-dev
    RedHat/Fedora/Oracle Linux: yum install libudev-devel
    SuSE:                       zypper install libudev-devel

and

CMake Warning at cmake/kerberos.cmake:54 (MESSAGE):
  Cannot find KERBEROS and GSSAPI development libraries.  You need to install
  the required packages:

    Debian/Ubuntu:              apt install libkrb5-dev
    RedHat/Fedora/Oracle Linux: yum install krb5-devel
    SuSE:                       zypper install krb5-devel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will install them as well.

run: |
cmake -B build -DCMAKE_INSTALL_PREFIX=../install -DWITH_DEBUG=1 -DDOWNLOAD_BOOST=1 -DWITH_BOOST=my_boost \
-DWITH_ZLIB=bundled -DWITH_SSL=system -DWITH_AUTHENTICATION_LDAP=0 -DWITH_LZ4=bundled \
-WITH_KEYRING_VAULT=ON -DWITH_ROCKSDB=0 -DCMAKE_C_COMPILER=clang-14 -DCMAKE_CXX_COMPILER=clang++-14 \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Above you install clang clang-tidy so it's better to use clang and clang++ here.
With ubuntu-22.04 clang probably leads to clang-14 but it may not be the case in the future if we will change ubuntu-22.04 to something else.

Copy link
Collaborator

@inikep inikep Dec 29, 2023

Choose a reason for hiding this comment

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

Another option is to install clang++-17 llvm-17-dev clang-tidy-17 from http://apt.llvm.org
as we do at https://github.com/percona/percona-server/blob/8.0/azure-pipelines.yml#L396-L400
to make sure we use the newest clang-tidy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will update clang-14 and clang++-14 to clang and clang++ and let it take care of the versions.

https://perconadev.atlassian.net/browse/PS-9040

Creates a yaml file for clang-tidy workflow which is triggered
on a pull request.

The workflow triggers a run of generating comiplation commands
and uses the same to perform a clang-tidy check on the modified
files and generates a report of the warnings in the changed code
through a github action bot in the form of review comments on
the pull request.
Copy link
Collaborator

@inikep inikep left a comment

Choose a reason for hiding this comment

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

LGTM

@VarunNagaraju
Copy link
Contributor Author

Closing this PR since as a duplicate of #5196.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants