adding clang-tidy as a linter tool

[Suyashd999]

The Ceph project has a huge codebase, and it faces the risk of containing suboptimal code that could jeopardize storage reliability or induce performance bottlenecks or cause resource inefficiencies. Identifying and rectifying such code issues is important to maintain the integrity and efficiency of the Ceph storage system. clang-tidy, a powerful static analysis tool, offers a systematic approach to uncover critical issues within the codebase and generate comprehensive reports highlighting potential vulnerabilities.

Integrating clang-tidy into the Jenkins CI pipeline of Ceph would be very beneficial for the development, as any new possible vulnerabilities would be identified early on. The goal is to enhance code quality, maintain performance, and ensure the reliability of the storage system by continuously monitoring and addressing potential issues through automated static analysis.

[ceph-jenkins]

Can one of the admins verify this patch?

[dmick]

Let's change the name of LICENSE to LICENSE.clang-tidy-to-junit.py to clarify what the license applies to

[Suyashd999]

address the couple more comments and then we'll set up a test job

Hi @dmick! I think we are ready for a test job.

However there is one problem. Clang-tidy requires compile_commands.json file for analysis for which we have to pass
-DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON to CMAKE. I have create a PR for that: https://github.com/ceph/ceph-build/pull/2270

This PR will also be need to be merged.

After this we are good to go.

[Suyashd999]

address the couple more comments and then we'll set up a test job

Hi @dmick! I think we are ready for a test job.

However there is one problem. Clang-tidy requires compile_commands.json file for analysis for which we have to pass -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON to CMAKE. I have create a PR for that: #2270

This PR will also be need to be merged.

After this we are good to go.

Hey @dmick can you please check this? Thank you

[Suyashd999]

Maybe it's best to incorporate this as an optional step in the make-check job? make check already builds ceph, and will have a built ceph tree in the workspace (unlike a standalone jenkins job, which would have to redo the build process)

Hi @dmick ! After researching a bit I found out that we can use the Jenkins plugin 'Copy Artifact' which allows us to copy files and directories from different Jenkins jobs.

What we can do is allow make check to finish its execution and start our clang-tidy job. In our job it'll copy all the files from the job make check and then start clang-tidy. I was able to make the copy artifact work in my local system. The build job after its successful completion would trigger our clang-tidy job. The clang-tidy job copies all the files from the build job in the form of a tar file and extracts it.

I think this is the best approach.

Please let me know what you think.

[dmick]

That sounds like a good way to address the concerns. IIRC this involves a change to the make check job to add permissions to allow clang-tidy to use the artifacts.

[Suyashd999]

That sounds like a good way to address the concerns. IIRC this involves a change to the make check job to add permissions to allow clang-tidy to use the artifacts.

Yes you are right. You will also need to install the plugin 'Copy Artifact' Please do so

[dmick]

Copy artifact is installed. It's used in the ceph-build jobs (search for "copyartifact")

[Suyashd999]

Copy artifact is installed. It's used in the ceph-build jobs (search for "copyartifact")

@dmick Okay. I'll need to change the make-check job to archive the files right? And where would I need to add the code to automatically trigger the clang-tidy job after make-check job?

[dmick]

Yes, the make check job will both archive the files and trigger the clang-tidy job

[Suyashd999]

Yes, the make check job will both archive the files and trigger the clang-tidy job

Hi @dmick. I have created a PR for archiving Ceph and triggering the job ceph-pr-clang-tidy. Can you please take a look at this? https://github.com/ceph/ceph-build/pull/2276 Thanks!

[Suyashd999]

Yes, the make check job will both archive the files and trigger the clang-tidy job

Hi @dmick. I have created a PR for archiving Ceph and triggering the job ceph-pr-clang-tidy. Can you please take a look at this? #2276 Thanks!

Hey @dmick! could you please look into this? Thank you!

[Suyashd999]

Hi! @dmick can you check this please?

Yes, the make check job will both archive the files and trigger the clang-tidy job

Hi @dmick. I have created a PR for archiving Ceph and triggering the job ceph-pr-clang-tidy. Can you please take a look at this? #2276 Thanks!

[Suyashd999]

@yuvalif done can you please take a look now, Thank you

[yuvalif]

@yuvalif done can you please take a look now, Thank you

do you have new time measurements after all of the latest improvements?

[Suyashd999]

@yuvalif done can you please take a look now, Thank you

do you have new time measurements after all of the latest improvements?

@yuvalif Apologies for the delay in reply, after various testing, I replicated the Jenkins jobs in my local system and started up our clang-tidy-jenkins job. The job took 24 minutes to complete.

It was running the clang-tidy check bugprone-use-after-move on ALL files in the RGW

The job was divided into various stages to exactly monitor the time taken by each stage.

1. Copy artifact stage: Time: 5.1 seconds

2. Installing clang-tidy: Time: 12 seconds

3. Shallow clone ceph repo & recursive submodule update: Time: 7 minutes 20 seconds

4. Running ./do_cmake with export commands: Time: 16 seconds

5. Replace the boost and include directories from the :` Time: 4.4 seconds

6. Running clang-tidy on ALL RGW files: Time: 15 minutes

Total time: 24 minutes

@yuvalif so what should I do next

[yuvalif]

@Suyashd999 thank you for the detailed report! I think that we will have to go fo the option of running clang-tidy only on the files modified in the test. this should reduce the part that takes most time. for testing the run time, you can pick arbitrary 10 RGW files (that should represent an "average" PR).

another question. when you do the "copy artifacts" stage, why can't you copy the actual code base as well? this may take less time than doing the shallow git clone + submodule update?

[Suyashd999]

another question. when you do the "copy artifacts" stage, why can't you copy the actual code base as well? this may take less time than doing the shallow git clone + submodule update?

@yuvalif I tried copying the actual code base in the Jenkins job to save time but there are two issues, the problem arises because of 1. submodules and 2. compile_commands.json .

1. To get compile_commands.json we need to run ./do_cmake.sh -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON, The first line of do_cmake.sh is git submodule update --init --recursive --progress --recommend-shallow So we need to wait for the submodules to be updated.

2. The submodules problem can by bypassed by not running do_cmake and copying the required compile_commands.json file from our make check (ceph-pull-requests) job but compile_commands.json uses absolute path instead of relative path. So using compile_commands.json from other job doesn't work.

As of now with the configuration of clang-tidy being only run on those files which have been modified is working great. It takes roughly 7-10 minutes to complete when the number of RGW files vary from 1-10.

It takes 4-5 minutes for the submodules to be updated and 2-4 minutes for the clang-tidy to finish execution. If we are able to discard the submodules step then the job would be done within couple of minutes

[yuvalif]

1. To get compile_commands.json we need to run ./do_cmake.sh -DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON, The first line of do_cmake.sh is git submodule update --init --recursive --progress --recommend-shallow So we need to wait for the submodules to be updated.
2. The submodules problem can by bypassed by not running do_cmake and copying the required compile_commands.json file from our make check (ceph-pull-requests) job but compile_commands.json uses absolute path instead of relative path. So using compile_commands.json from other job doesn't work.

you dont need to run do_cmake to get compile_commands.json . inside the build directory run cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ..

it does exactly the same, only without the submodules

[Suyashd999]

@yuvalif Great success :tada:

With your insights I made the changes

The entire Jenkins clang-tidy job only took 2 min 32 seconds :tada:

The Jenkins job was run on ceph's latest commits

I have attached the logs, for testing purposes the git diff was modified to work on the last commit in rgw which was https://github.com/ceph/ceph/commit/50fa90b944424fb0eae2d9c6d55064b99be7971b

Please look into this, #36.txt

[yuvalif]

@yuvalif Great success 🎉

With your insights I made the changes

The entire Jenkins clang-tidy job only took 2 min 32 seconds 🎉

The Jenkins job was run on ceph's latest commits

I have attached the logs, for testing purposes the git diff was modified to work on the last commit in rgw which was ceph/ceph@50fa90b

Please look into this, #36.txt

thanks @Suyashd999 these are great news. this commit was fairly small (one file) but even if the whole process takes less than 10 min (for larget commits) I think we are good.

@dmick do you think these run times make sense? are you good with merging that?