Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions run-obmc-cppcheck.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
#!/bin/bash
###############################################################################
#
# This script is for running the cppcheck for obmc-phosphor-image
#
###############################################################################
#
# Required Inputs:
# WORKSPACE: Directory which contains the extracted openbmc-build-scripts
# directory
# REPOLIST: List of repos to do the cppcheck.

usage()
{
echo "usage: run-obmc-cppcheck.sh <opt>"
echo "Example:"
echo 'WORKSPACE=$(pwd) openbmc-build-scripts/run-obmc-cppcheck.sh '
echo ""
}

# Default variables
WORKSPACE=${WORKSPACE:-$(pwd)}
REPOLIST=${REPOLIST:-""}
OBMC_BUILD_SCRIPTS="openbmc-build-scripts"

#
REPOLIST=$(echo $REPOLIST)
#

# Check workspace, build scripts, and package to be unit tested exists
if [ ! -d "${WORKSPACE}" ]; then
echo "Workspace(${WORKSPACE}) doesn't exist, exiting..."
exit 1
fi
if [ ! -d "${WORKSPACE}/${OBMC_BUILD_SCRIPTS}" ]; then
echo "Package(${OBMC_BUILD_SCRIPTS}) not found in ${WORKSPACE}, exiting..."
exit 1
fi

##

# run cppcheck
function run_cppcheck()
{
cd ${WORKSPACE}
for REPONAME in ${REPOLIST}
do
UNIT_TEST_PKG=$REPONAME CPPCHECK_ONLY=1 ./openbmc-build-scripts/run-unit-test-docker.sh
done
}


### MAIN ####


# MACHINE
MACHINE=${MACHINE:-p10bmc}
export MACHINE

# environment variables
#
# List of Repos to scan.
REPOLIST=${REPOLIST:-""}

# Check the repolist config file
REPOLIST_FILE=${WORKSPACE}/obmc-cppcheck-repolist.txt
[[ -z "${REPOLIST}" && -f "${REPOLIST_FILE}" ]] && REPOLIST=$(cat ${REPOLIST_FILE})

# Use default repos if not passed
if [ -z "${REPOLIST}" ]
then
REPOLIST="
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@geissonator
Thanks for the review.

I'll try to incorporate your suggestions later.

bmcweb \
dbus-sensors \
entity-manager \
ibm-acf \
ipl \
libpldm \
obmc-console \
openpower-debug-collector \
openpower-hw-diags \
openpower-hw-isolation \
openpower-occ-control \
openpower-vpd-parser \
panel \
phosphor-bmc-code-mgmt \
phosphor-certificate-manager \
phosphor-dbus-interfaces \
phosphor-debug-collector \
phosphor-host-ipmid \
phosphor-inventory-manager \
phosphor-led-manager \
phosphor-logging \
phosphor-power \
phosphor-state-manager \
phosphor-user-manager \
pldm \
powervm-handler \
service-config-manager \
telemetry \
"
fi

echo "usage: run-obmc-cppcheck.sh"


#
cd ${WORKSPACE}/openbmc
. setup ${MACHINE}

cd ${WORKSPACE}
run_cppcheck

exit 0
10 changes: 10 additions & 0 deletions run-unit-test-docker.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
# EXTRA_UNIT_TEST_ARGS: Optional, pass arguments to unit-test.py
# INTERACTIVE: Optional, run a bash shell instead of unit-test.py
# http_proxy: Optional, run the container with proxy environment
#
# CPPCHECK_ONLY: Optional, run cppcheck only and skip all other tests
# FAIL_ON_CPPCHECK_ERROR=[0|1]: Optional, no ci failure even if the cppcheck errors by default
#

# Trace bash processing. Set -e so when a step fails, we fail the build
set -uo pipefail
Expand All @@ -47,6 +51,10 @@ NO_CPPCHECK="${NO_CPPCHECK:-}"
INTERACTIVE="${INTERACTIVE:-}"
http_proxy=${http_proxy:-}

CPPCHECK_ONLY="${CPPCHECK_ONLY:-}"
# Ignore cppcheck error by default
FAIL_ON_CPPCHECK_ERROR="${FAIL_ON_CPPCHECK_ERROR:-""}"

# Timestamp for job
echo "Unit test build started, $(date)"

Expand Down Expand Up @@ -76,6 +84,8 @@ export DOCKER_IMG_NAME
# Allow the user to pass options through to unit-test.py:
# EXTRA_UNIT_TEST_ARGS="-r 100" ...
EXTRA_UNIT_TEST_ARGS="${EXTRA_UNIT_TEST_ARGS:+,${EXTRA_UNIT_TEST_ARGS/ /,}}"
EXTRA_UNIT_TEST_ARGS+=${CPPCHECK_ONLY:+,--cppcheck-only}
[ "${FAIL_ON_CPPCHECK_ERROR}" == "1" ] && EXTRA_UNIT_TEST_ARGS+=",--fail-on-cppcheck-error"

# Unit test and parameters
if [ "${INTERACTIVE}" ]; then
Expand Down
57 changes: 57 additions & 0 deletions scripts/run-cppcheck.sh
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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?

Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#!/bin/bash
#
# Test for cppcheck filter
# Required Inputs:
# WORKSPACE: Directory which contains the extracted openbmc-build-scripts
# directory
# REPONAME: repo to run cppcheck. If not, use UNIT_TEST_PKG
# LOGDIR: Optional, log output dir. If not, WORKSPACE/logs/
# REPORTDIR: Optional, report output dir, If not, use WORKSPACE/reports/

set -uo pipefail

args=$*

[ -z "${UNIT_TEST_PKG}" ] && ( echo "UNIT_TEST_PKG is not set"; exit 1)

WORKSPACE=${WORKSPACE:-$(pwd)}
REPONAME=${REPONAME:-${UNIT_TEST_PKG}}
LOGDIR=${LOGDIR:-${WORKSPACE}/logs}
REPORTDIR=${REPORTDIR:-${WORKSPACE}/reports}
mkdir -p ${LOGDIR}
mkdir -p ${REPORTDIR}

## Note: Additional necessary opts
CPPCHECK_ADD_OPTS=-D"__cppcheck__=1"

CPPCHECK_STDOUT=${LOGDIR}/cppchk.${REPONAME}.stdout
CPPCHECK_STDERR=${LOGDIR}/cppchk.${REPONAME}.stderr
cppcheck ${CPPCHECK_ADD_OPTS} --template="{file}:{line}:{column}: {id}: {severity}: {message}" $args 2> ${CPPCHECK_STDERR} | tee ${CPPCHECK_STDOUT}

CPPCHECK_ERR_LOG=${REPORTDIR}/cppchk.${REPONAME}.error.log

FAILED=0

rm -f ${CPPCHECK_ERR_LOG}
while read -r filelinecol id severity remaining; do
if [[ "$filelinecol" == "/"* || "$filelinecol" == "subprojects/"* ]]
then
# ignore the absolute-full-file paths like /usr/include/*, or under subprojects
continue
fi
if [[ "$severity" == "warning:" || "$severity" == "error:" ]]
then
echo "Filename:Line:Column: $filelinecol" >> ${CPPCHECK_ERR_LOG}
echo "id:severity: $id $severity" >> ${CPPCHECK_ERR_LOG}
echo "remaining: $remaining" >> ${CPPCHECK_ERR_LOG}
echo "---" >> ${CPPCHECK_ERR_LOG}
FAILED=1
fi
done < ${CPPCHECK_STDERR}

if [[ $FAILED -ne 0 ]]
then
echo "cppcheck ($UNIT_TEST_PKG) failed, CPPCHECK_ERR_LOG=$CPPCHECK_ERR_LOG"
cat ${CPPCHECK_ERR_LOG}
exit 1
fi
62 changes: 59 additions & 3 deletions scripts/unit-test.py
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Original file line number Diff line number Diff line change
Expand Up @@ -379,11 +379,17 @@ def run_cppcheck():
):
return None

cppchkenv = os.environ.copy()
cppchkenv["UNIT_TEST_PKG"] = UNIT_TEST_PKG
cppchkenv["WORKSPACE"] = WORKSPACE

with TemporaryDirectory() as cpp_dir:
# http://cppcheck.sourceforge.net/manual.pdf
try:
check_call_cmd(
"cppcheck",
os.path.join(
WORKSPACE, "openbmc-build-scripts", "scripts", "run-cppcheck.sh"
),
"-j",
str(multiprocessing.cpu_count()),
"--enable=style,performance,portability,missingInclude",
Expand All @@ -396,9 +402,12 @@ def run_cppcheck():
"--library=googletest",
"--project=build/compile_commands.json",
f"--cppcheck-build-dir={cpp_dir}",
env=cppchkenv,
)
except subprocess.CalledProcessError:
print("cppcheck found errors")
if FAIL_ON_CPPCHECK_ERROR:
raise Exception("cppcheck found errors")


def is_valgrind_safe():
Expand Down Expand Up @@ -515,6 +524,8 @@ def maybe_make_coverage():
via `make check-code-coverage`. If the package does not have code coverage
testing then it just skips over this.
"""
if CPPCHECK_ONLY:
return
if not make_target_exists("check-code-coverage"):
return

Expand Down Expand Up @@ -722,13 +733,19 @@ def configure(self, build_for_testing):
check_call_cmd("./configure", *conf_flags)

def build(self):
if CPPCHECK_ONLY:
return
check_call_cmd(*make_parallel)

def install(self):
if CPPCHECK_ONLY:
return
check_call_cmd("sudo", "-n", "--", *(make_parallel + ["install"]))
check_call_cmd("sudo", "-n", "--", "ldconfig")

def test(self):
if CPPCHECK_ONLY:
return
try:
cmd = make_parallel + ["check"]
for i in range(0, args.repeat):
Expand Down Expand Up @@ -758,6 +775,8 @@ def dependencies(self):
return []

def configure(self, build_for_testing):
if CPPCHECK_ONLY:
return
self.build_for_testing = build_for_testing
if INTEGRATION_TEST:
check_call_cmd(
Expand All @@ -776,6 +795,8 @@ def configure(self, build_for_testing):
)

def build(self):
if CPPCHECK_ONLY:
return
check_call_cmd(
"cmake",
"--build",
Expand All @@ -786,14 +807,21 @@ def build(self):
)

def install(self):
if CPPCHECK_ONLY:
return
check_call_cmd("sudo", "cmake", "--install", ".")
check_call_cmd("sudo", "-n", "--", "ldconfig")

def test(self):
if CPPCHECK_ONLY:
return
if make_target_exists("test"):
check_call_cmd("ctest", ".")

def analyze(self):
if CPPCHECK_ONLY:
run_cppcheck()
return
if os.path.isfile(".clang-tidy"):
with TemporaryDirectory(prefix="build", dir=".") as build_dir:
# clang-tidy needs to run on a clang-specific build
Expand Down Expand Up @@ -959,6 +987,7 @@ def get_configure_flags(self, build_for_testing):
return meson_flags

def configure(self, build_for_testing):
# Note - configure is still needed for CPPCHECK_ONLY
meson_flags = self.get_configure_flags(build_for_testing)
try:
check_call_cmd(
Expand All @@ -971,13 +1000,19 @@ def configure(self, build_for_testing):
self.package = Meson._project_name("build")

def build(self):
if CPPCHECK_ONLY:
return
check_call_cmd("ninja", "-C", "build")

def install(self):
if CPPCHECK_ONLY:
return
check_call_cmd("sudo", "-n", "--", "ninja", "-C", "build", "install")
check_call_cmd("sudo", "-n", "--", "ldconfig")

def test(self):
if CPPCHECK_ONLY:
return
# It is useful to check various settings of the meson.build file
# for compatibility, such as meson_version checks. We shouldn't
# do this in the configure path though because it affects subprojects
Expand Down Expand Up @@ -1057,6 +1092,9 @@ def _maybe_valgrind(self):
raise Exception("Valgrind tests failed")

def analyze(self):
if CPPCHECK_ONLY:
run_cppcheck()
return
self._maybe_valgrind()

# Run clang-tidy only if the project has a configuration
Expand Down Expand Up @@ -1345,6 +1383,22 @@ def find_file(filename, basedir):
default=False,
help="Do not run cppcheck",
)
parser.add_argument(
"--fail-on-cppcheck-error",
dest="FAIL_ON_CPPCHECK_ERROR",
action="store_true",
required=False,
default=False, # No CI failure even if cppcheck error by default
help="Igonore the cppcheck errors",
)
parser.add_argument(
"--cppcheck-only",
dest="CPPCHECK_ONLY",
action="store_true",
required=False,
default=False,
help="Only run cppcheck and skip all other tests",
)
arg_inttests = parser.add_mutually_exclusive_group()
arg_inttests.add_argument(
"--integration-tests",
Expand Down Expand Up @@ -1390,6 +1444,8 @@ def find_file(filename, basedir):
WORKSPACE = args.WORKSPACE
UNIT_TEST_PKG = args.PACKAGE
TEST_ONLY = args.TEST_ONLY
CPPCHECK_ONLY = args.CPPCHECK_ONLY
FAIL_ON_CPPCHECK_ERROR = args.FAIL_ON_CPPCHECK_ERROR
NO_CPPCHECK = args.NO_CPPCHECK
INTEGRATION_TEST = args.INTEGRATION_TEST
BRANCH = args.BRANCH
Expand All @@ -1409,7 +1465,7 @@ def printline(*line):
CODE_SCAN_DIR = os.path.join(WORKSPACE, UNIT_TEST_PKG)

# Run format-code.sh, which will in turn call any repo-level formatters.
if FORMAT_CODE:
if (not CPPCHECK_ONLY) and FORMAT_CODE:
check_call_cmd(
os.path.join(
WORKSPACE, "openbmc-build-scripts", "scripts", "format-code.sh"
Expand Down Expand Up @@ -1461,7 +1517,7 @@ def printline(*line):
# Run any custom CI scripts the repo has, of which there can be
# multiple of and anywhere in the repository.
ci_scripts = find_file(["run-ci.sh", "run-ci"], CODE_SCAN_DIR)
if ci_scripts:
if (not CPPCHECK_ONLY) and ci_scripts:
os.chdir(CODE_SCAN_DIR)
for ci_script in ci_scripts:
check_call_cmd(ci_script)
Loading