ceph-build icon indicating copy to clipboard operation
ceph-build copied to clipboard

CMAKE compile_commands flag addition

Open Suyashd999 opened this issue 1 year ago • 14 comments

The DCMAKE_EXPORT_COMPILE_COMMANDS flag generates the compile_commands.json file which contains the exact compiler calls for all translation units of the project in machine-readable form.

This file is used by many code analysis tools, IDE(s), refactoring tools, etc.

There is no harm in having the compile_commands.json file.

We want to integrate clang-tidy into the Ceph CI for which the compile_commands.json file is necessary.

Suyashd999 avatar Jul 21 '24 12:07 Suyashd999

Can one of the admins verify this patch?

ceph-jenkins avatar Jul 21 '24 12:07 ceph-jenkins

How large is the .json file? where is it placed? Does this mean that the clang-tidy job must be run after the ceph build job? How is that going to fit in our job workflow?

dmick avatar Aug 01 '24 21:08 dmick

@dmick

How large is the .json file? where is it placed? Does this mean that the clang-tidy job must be run after the ceph build job? How is that going to fit in our job workflow?

How large is the .json file?

The .json file is about 50MB.

where is it placed?

There is no specific constraint on where it needs to be placed, we can place it anywhere, as of now I am placing it in the build directory.

Does this mean that the clang-tidy job must be run after the ceph build job?

Yes

How is that going to fit in our job workflow?

After build_debs() finishes execution it would have created the compile_commands.json and stored it in the build directory (considering we are going to store it in build) then clang-tidy will be specified the directory which has the compile_commands.json file. clang-tidy will then start its execution.

Reference: After the build_debs finishes we are starting clang-tidy

Suyashd999 avatar Aug 02 '24 02:08 Suyashd999

but...so...that means clang-tidy isn't a separate Jenkins job anymore, or a pre-check for PRs, it's a step of the actual build. That's unlike anything else in the build structure. What do we do if the build succeeds but clang-tidy fails?

I had started out assuming that clang-tidy was going to be running on ceph PRs like other checks, but now that seems very different from the flow it may need.

dmick avatar Aug 02 '24 02:08 dmick

but...so...that means clang-tidy isn't a separate Jenkins job anymore, or a pre-check for PRs, it's a step of the actual build. That's unlike anything else in the build structure. What do we do if the build succeeds but clang-tidy fails?

I had started out assuming that clang-tidy was going to be running on ceph PRs like other checks, but now that seems very different from the flow it may need.

What do we do if the build succeeds but clang-tidy fails?

It doesn't matter if clang-tidy fails, in fact we want it to "fail". The only purpose of clang-tidy is to pop warnings/errors to users.

I had started out assuming that clang-tidy was going to be running on ceph PRs like other checks,

It still is. The only change that we are making is to get the .json file which the clang-tidy needs. That .json file can only be generated by cmake. There would be no alteration to the workflow whatsoever.

Suyashd999 avatar Aug 02 '24 02:08 Suyashd999

oh, but, no...the clang-tidy job is attempting to "build_debs()" itself. That's not going to work; the build is a long resource-intensive process, and there's a lot more going on than just build_debs(); that depends on being run from a jenkins job that includes a -setup stage.

We're going to need to rethink this.

dmick avatar Aug 02 '24 03:08 dmick

oh, but, no...the clang-tidy job is attempting to "build_debs()" itself. That's not going to work; the build is a long resource-intensive process, and there's a lot more going on than just build_debs(); that depends on being run from a jenkins job that includes a -setup stage.

We're going to need to rethink this.

I see.. We can just run the cmake to get the .json file. But the issue arises when clang-tidy cannot find the boost module which is installed when building. That is why I was going on with building ceph. Is there a way to install the boost module without the traditional build? If we are able to achieve this then there is no need to build ceph at all.

Suyashd999 avatar Aug 02 '24 03:08 Suyashd999

I don't know. Also, even "just running cmake" depends on stuff in ./install-deps.sh, which for Debian builds would need to be running inside a pbuilder environment... this is totally new ground.

dmick avatar Aug 02 '24 03:08 dmick

I don't know. Also, even "just running cmake" depends on stuff in ./install-deps.sh, which for Debian builds would need to be running inside a pbuilder environment... this is totally new ground.

what should we do now? What do you think is an appropriate solution?

Suyashd999 avatar Aug 02 '24 03:08 Suyashd999

@dmick is it possible that I manually build Ceph instead of build_debs() inside the pbuilder environment? By following the steps specified in the README

Suyashd999 avatar Aug 03 '24 16:08 Suyashd999

Hi @dmick can you please check the above comments? Thanks!

Suyashd999 avatar Aug 05 '24 17:08 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)

dmick avatar Aug 05 '24 18:08 dmick

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)

@dmick We can but make check is a required check and we want clang-tidy to be a non-blocking check. Can we add clang-tidy to another job?

Suyashd999 avatar Aug 06 '24 17:08 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)

I have suggested a solution for this please check: https://github.com/ceph/ceph-build/pull/2269#issuecomment-2302112163

Suyashd999 avatar Aug 21 '24 13:08 Suyashd999