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

RFC: Add static code analyzer integration #51179

Conversation

romkell
Copy link
Contributor

@romkell romkell commented Oct 11, 2022

PR dropped and closed in favor of #51212.

@@ -0,0 +1,10 @@
{
Copy link
Contributor Author

@romkell romkell Oct 11, 2022

Choose a reason for hiding this comment

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

The file is a generated file and will be removed from the commit sooner or later.
It is here to illustrate the output format.

Question:
Why are CMAKE_CXX_COMPILER and all the other variables empty except for the CMAKE_C_COMPILER?
Did I include the sca_integration.cmake too early?
Any hints for me?

@cfriedt
Copy link
Member

cfriedt commented Oct 11, 2022

@romkell - would be happy to backport, but this would need to go into main prior to v2.7-branch

@laurenmurphyx64 laurenmurphyx64 added TSC Topics that need TSC discussion and removed TSC Topics that need TSC discussion labels Oct 11, 2022
@romkell
Copy link
Contributor Author

romkell commented Oct 12, 2022

@romkell - would be happy to backport, but this would need to go into main prior to v2.7-branch

@cfriedt With our company internal framework we are currently stuck on a 2.7 tag. Therefore I implemented against that, but tried to keep it as independent as possible to be hooked in at the right location (3.2 cmake files have been rather reorganized compared to 2.7). We will need it on 2.7-auditable-branch. My initial thought was that we would take the latest 2.7 tag as new 2.7-auditable-branch to get all the fixes. Does not work since 2.7-branch and 2.7-auditable-branch are very much out of sync due to misra fixes in auditable.
Hence I will have to take it to 2.7-auditable-branch and also main (on which I would currently have a hard time to test).

Would it be ok to create a PR to main with a not tested, half finished implementation to get the attention and feedback?

The main goal at this time is to get feedback on the mechanism and also to get some hints why basically all CMAKE__COMPILER/LINKER variables are empty (except for CMAKE_C_COMPILER).

Add a generic way to integrate any static code analyzer
into the cmake build script.
- Writing the selected toolchain paths to the sca_export.json
- Reading the static code analyzer toolchain paths from a
  sca_import.json and replacing the CMAKE_C_COMPILER
  settings with the values read from the json
- Executing static code analyzer customized scripts updating
  the analyzers configuration (with the information in sca_export.json)
- Support for a in tree and a out of tree location, allowing to
  integrate analyzers as part of Zephyr itself and as part of a
  customer environment using Zephyr as its base.

Signed-off-by: Roman Kellner <[email protected]>
@romkell romkell force-pushed the cmake_sca_integration_v2_7_2 branch from f471578 to 004d639 Compare October 12, 2022 07:34
@stephanosio
Copy link
Member

This is not a Backport task.

What this PR is proposing to do is to merge a new feature into a previous release, which is something we do not do very often (in fact, we almost never do this).

If you want to pull this into v2.7-auditable-branch, that is up to you and the Safety Committee; but, it will likely never go into the v2.7-branch or any other previous release branches.

@romkell
Copy link
Contributor Author

romkell commented Oct 12, 2022

If you want to pull this into v2.7-auditable-branch, that is up to you and the Safety Committee; but, it will likely never go into the v2.7-branch or any other previous release branches.

@stephanosio Please consider the v2.7-branch a mistake. Can we discuss the mechansim / solution proposal in this PR or do I need to create one aiming to main (I assume not getting a lot of feedback when aiming for 2.7-auditable-branch)?

@stephanosio
Copy link
Member

Can we discuss the mechansim / solution proposal in this PR or do I need to create one aiming to main (I assume not getting a lot of feedback when aiming for 2.7-auditable-branch)?

I would say creating one for main is going to be for the best.

@romkell romkell closed this Oct 12, 2022
@romkell
Copy link
Contributor Author

romkell commented Oct 12, 2022

PR dropped and closed in favor of #51212.

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

Successfully merging this pull request may close these issues.

4 participants