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]

of course this only works if the build machine is an apt-based system, which it may not be using your current node labels. It might be more appropriate to install the package inside the pbuilder image in that case, too, although it probably doesn't matter too much; there are ways of adding extra packages to the pbuilder image

[dmick]

We still need an answer for this; does clang-tidy exist for RHEL-like systems? or do we want to restrict this job to only apt systems?

[Suyashd999]

It is available but for now lets keep the job to apt systems only

[ronen-fr]

How is this accomplished? won't the command be executed (and fail) on all systems?

[dmick]

The job specifies the labels for the jenkins builder it wants; we'll need to add something like 'jammy' as a label, that should suffice

[Suyashd999]

Where should I add this 'jammy' label?

[dmick]

job.node is the list of labels the builder must satisfy. You probably want jammy && small

[Suyashd999]

job.node is the list of labels the builder must satisfy. You probably want jammy && small

@dmick I have added node: jammy && small

[dmick]

I suspect this is probably provided by the ghprb plugin, and doesn't need to/shouldn't be a settable parameter

[dmick]

What's the response to this?

[Suyashd999]

I removed ghprbPullId as a settable parameter. However in others Jenkins jobs it is set as a settable parameter.

Reference:
1.

ceph-build/ceph-pull-requests-arm64/config/definitions/ceph-pull-requests-arm64.yml

Lines 13 to 16 in e8d18c7

	parameters:
	- string:
	name: ghprbPullId
	description: "the GitHub pull id, like '72' in 'ceph/pull/72'"

ceph-build/ceph-perf-pull-requests/config/definitions/ceph-perf-pull-requests.yml

Lines 137 to 140 in e8d18c7

	parameters:
	- string:
	name: ghprbPullId
	description: "the GitHub pull id, like '72' in 'ceph/pull/72'"

ceph-build/ceph-pull-requests/config/definitions/ceph-pull-requests.yml

Lines 27 to 30 in e8d18c7

	parameters:
	- string:
	name: ghprbPullId
	description: "the GitHub pull id, like '72' in 'ceph/pull/72'"

[Suyashd999]

@dmick and what about this?

[dmick]

We can leave it in for now; I don't think it's necessary but it might be useful for testing

[dmick]

Let's resolve the outstanding comments

[dmick]

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

[dmick]

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

[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.

[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? #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!