1120: Run cppcheck only and and report cppcheck for obmc image#33
1120: Run cppcheck only and and report cppcheck for obmc image#33baemyung wants to merge 1 commit intoibm-openbmc:1120from
Conversation
|
CPPCHECK report is on BOX - https://ibm.ent.box.com/folder/365997430870 |
a336a59 to
b7a7a83
Compare
This adds a few options to `run-unit-test-docker.sh` to deal with the
cppcheck via the enviroment variables like
- CPPCHECK_ONLY=1 to run cppcheck only without going thru the full CI
run. By default, the cppcheck is a part of full CI.
- FAIL_ON_CPPCHECK_ERROR=1 to result in CI failure if cppcheck finds the
warnings/errors. By default, it does not cause CI failure (as of now)
Example usages:
```
repo=bmcweb
WORKSPACE=$(pwd) CPPCHECK_ONLY=1 UNIT_TEST_PKG=${repo} ./openbmc-build-scripts/run-unit-test-docker.sh
```
The warnings/errors go to stdout and are also saved in a file
```
WORKSPACE/reports/cppchk.<repo>.error.log
```
In addition, this adds the following scripts to get the cppcheck reports
of the dependent repos for the given obmc-phosphor-image.
1. setup-obmc-cppcheck.sh
This is to setup/extract the dependent repos for the given branch of
obmc-phosphor-image. Its evnvironment variables are
- WORKSPACE=${pwd} the top directories that extract repositories and
outputs/reports of cppcheck
- BRANCH=<branch-tag> for obmc-phosphor-image
- REPOLIST="<list of repos>" to run cppcheck.
- MACHINE=<target> Bydefault, it is p10bmc
For example.
```
WORKSPACE=$(pwd) BRANCH=fw1120.00-1.70 ./openbmc-build-scripts/setup-obmc-cppcheck.sh
```
If necessary, this may need to run under the container instance.
2. run-obmc-cppcheck.sh
This is to run `cppcheck` tool for the previous extract repos. It uses
the similar environment variables for `setup-obmc-cppcheck.sh` except
`BRANCH` which is used for extract the repos.
- WORKSPACE=${pwd} the top directories that extract repositories and
outputs/reports of cppcheck
- REPOLIST="<list of repos>" to run cppcheck.
- MACHINE=<target> Bydefault, it is p10bmc
```
REPOLIST="pldm bmcweb" WORKSPACE=$(pwd) ./openbmc-build-scripts/run-obmc-cppcheck.sh
```
3. Sample output:
WORKSPACE/reports/cppchk.phosphor-bmc-code-mgmt.error.log:
```
Filename:Line:Column: src/bus_monitor.cpp:505:15:
id:severity: uninitMemberVar: warning:
remaining: Member variable 'SystemStatus::rebootPolicy' is not initialized in the constructor.
```
The related references about cppcheck are
- https://cppcheck.sourceforge.io/
- https://cppcheck.sourceforge.io/#documentation
- https://cppcheck.sourceforge.io/manual.pdf
Signed-off-by: Myung Bae <myungbae@us.ibm.com>
b7a7a83 to
b724e04
Compare
|
Code is updated to ignore `subprojects'. CPPCHECK-REPORT/fw1120.00-1.81/reports:
|
| # Use default repos if not passed | ||
| if [ -z "${REPOLIST}" ] | ||
| then | ||
| REPOLIST=" |
There was a problem hiding this comment.
Hard coding the same defaults in 2 places is always dangerous. Can we just combine the 2 scripts, run-obmc-cppcheck.sh and setup-obmc-cppchck.sh?
Also, we tend to not put the ".sh" on files now as it allows to change to something like python later without impacting the things calling the script.
There was a problem hiding this comment.
@geissonator
Thanks for the review.
I'll try to incorporate your suggestions later.
There was a problem hiding this comment.
I think the general concept of providing a way to only run cppcheck and failing on it is upstreamable so lets send that portion upstream.
| # Required Inputs: | ||
| # WORKSPACE: Directory which contains the extracted openbmc-build-scripts | ||
| # directory | ||
| # BRANCH: Optional, branch or tag to build from each of the |
There was a problem hiding this comment.
I think if there's no BRANCH provided the default should be to just use whatever the user has setup in openbmc.
| cd ${WORKSPACE} | ||
| if [ ! -d ./openbmc ] | ||
| then | ||
| echo git clone git@github.ibm.com:openbmc/openbmc.git |
There was a problem hiding this comment.
In general, should avoid referencing GHE.
There was a problem hiding this comment.
The commit message has a lot of the what and how but not a lot of why. Could you add some details on why we need this?
This adds a few options to
run-unit-test-docker.shto deal with the cppcheck via the enviroment variables likeCPPCHECK_ONLY=1 to run cppcheck only without going thru the full CI run. By default, the cppcheck is a part of full CI.
FAIL_ON_CPPCHECK_ERROR=1 to result in CI failure if cppcheck finds the warnings/errors. By default, it does not cause CI failure (as of now)
Example usages:
The warnings/errors go to stdout and are also saved in a file
In addition, this adds the following scripts to get the cppcheck reports of the dependent repos for the given obmc-phosphor-image.
This is to setup/extract the dependent repos for the given branch of obmc-phosphor-image. Its evnvironment variables are
For example.
If necessary, this may need to run under the container instance.
This is to run
cppchecktool for the previous extract repos. It uses the similar environment variables forsetup-obmc-cppcheck.shexceptBRANCHwhich is used for extract the repos.WORKSPACE/reports/cppchk.phosphor-bmc-code-mgmt.error.log:
The related references about cppcheck are