RDKEMW-14533: MRG - Coverity inclusion changes#92
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new GitHub Actions workflow intended to build the project on Ubuntu (and, per naming, support a Coverity incremental scan flow).
Changes:
- Add
.github/workflows/native_full_build.ymlto run autotools configure/build/install onubuntu-22.04. - Upload the install prefix (
output/) as a workflow artifact.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| LDFLAGS="-L$PREFIX_PATH/lib" \ | ||
| ./configure \ | ||
| --enable-gstreamer1=yes \ | ||
| --enable-pi-build=yes \ |
There was a problem hiding this comment.
This workflow configures without enabling either --enable-rpc or --enable-rbusrpc. In this repo, src/main/Makefile.am only defines btMgrBus_SOURCES inside USE_RPC_IARM / USE_RPC_RBUS conditionals, and configure.ac defaults both conditionals to false when these flags aren’t provided. As-is, the build is likely to fail due to btMgrBus being built with no sources/objects; pass the appropriate enable flag (and install any needed deps) for the intended build variant.
| --enable-pi-build=yes \ | |
| --enable-pi-build=yes \ | |
| --enable-rpc=yes \ |
| name: BT-Core Coverity Incremental Analysis Scan | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - develop | ||
| - main | ||
| pull_request: | ||
| branches: | ||
| - develop | ||
| - main | ||
|
|
||
| jobs: | ||
| native-build: | ||
| runs-on: ubuntu-22.04 | ||
|
|
There was a problem hiding this comment.
Workflow name/PR intent indicates a "Coverity Incremental Analysis Scan", but this workflow only builds and uploads artifacts and contains no Coverity invocation (e.g., cov-build/analysis/upload). Either add the missing Coverity steps or rename the workflow/job/file to match what it actually does to avoid confusion and incorrect CI expectations.
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: bluetooth_mgr-native-build | ||
| path: output |
There was a problem hiding this comment.
The workflow defines PREFIX_PATH as "${{ github.workspace }}/output", but the artifact upload step hardcodes "path: output". Use the same variable/path in both places so the upload stays correct if PREFIX_PATH changes (and to make the intent clearer).
| path: output | |
| path: ${{ env.PREFIX_PATH }} |
| name: BT-Core Coverity Incremental Analysis Scan | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - develop | ||
| - main | ||
| pull_request: |
There was a problem hiding this comment.
PR description mentions a device deepsleep crash fix and related test procedure, but the only change in this PR is a new GitHub Actions workflow. If the workflow change is the intended fix, the PR description should be updated to reflect that; otherwise the code change(s) for the crash fix appear to be missing from this PR.
| - name: Configure | ||
| run: | | ||
| mkdir -p $PREFIX_PATH | ||
| CPPFLAGS="-I$PREFIX_PATH/include -I$PREFIX_PATH/include/cjson" \ | ||
| LIBCJSON_CFLAGS="-I/usr/include/cjson" \ | ||
| LDFLAGS="-L$PREFIX_PATH/lib" \ | ||
| ./configure \ | ||
| --enable-gstreamer1=yes \ | ||
| --enable-pi-build=yes \ | ||
| --prefix=$PREFIX_PATH |
There was a problem hiding this comment.
This workflow runs ./configure without first providing libbtrCore headers/libs. configure.ac hard-requires libbtrCore (AC_CHECK_LIB(btrCore, ...) will error) and README.txt describes building/installing btcore first and passing LDFLAGS/CPPFLAGS (including -lbtrCore). Add a step to build/install btcore (or install a package providing libbtrCore) and update CPPFLAGS/LDFLAGS accordingly so the workflow can actually configure/build successfully.
PreethiLakshmi91
force-pushed
the
RDKEMW-14533_changes
branch
from
17a264b to
63e2e7d
Compare
PreethiLakshmi91
force-pushed
the
RDKEMW-14533_changes
branch
from
63e2e7d to
681bdff
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| branches: | ||
| - develop | ||
| - main | ||
|
|
There was a problem hiding this comment.
Consider explicitly setting minimal permissions: for this workflow (e.g., contents: read) to follow GitHub Actions least-privilege defaults, especially since this runs on pull_request events.
| permissions: | |
| contents: read |
| @@ -0,0 +1,72 @@ | |||
| name: BT-Mgr Coverity Incremental Analysis Scan | |||
There was a problem hiding this comment.
PR description mentions a crash fix (device deepsleep) and Coverity inclusion changes, but this PR only adds a GitHub Actions native build workflow and does not change runtime code. Please update the PR title/description (or include the missing functional changes) so reviewers and release notes accurately reflect the change.
| @@ -0,0 +1,72 @@ | |||
| name: BT-Mgr Coverity Incremental Analysis Scan | |||
There was a problem hiding this comment.
Workflow is named "BT-Mgr Coverity Incremental Analysis Scan", but there are no Coverity steps (no cov-build/cov-analyze, token/secrets, etc.). Either add the actual Coverity analysis steps or rename the workflow/job/file to reflect that it only performs a native build + artifact upload.
| name: BT-Mgr Coverity Incremental Analysis Scan | |
| name: BT-Mgr Native Full Build |
| - name: Prepare autotools | ||
| run: | | ||
| libtoolize --force | ||
| aclocal |
There was a problem hiding this comment.
aclocal is invoked twice back-to-back; the first call is redundant and can also mask the intent to include the m4 macro dir. Consider keeping only the aclocal -I m4 invocation (or use autoreconf -fi if you want the standard one-shot regeneration) to reduce CI time and confusion.
| aclocal |
PreethiLakshmi91
force-pushed
the
RDKEMW-14533_changes
branch
from
681bdff to
e9428ec
Compare
PreethiLakshmi91
force-pushed
the
RDKEMW-14533_changes
branch
from
e9428ec to
95a0545
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| git clone https://github.com/rdkcentral/bluetooth.git | ||
|
|
There was a problem hiding this comment.
git clone https://github.com/rdkcentral/bluetooth.git pulls whatever the current default branch HEAD is, which makes builds non-reproducible and creates a supply-chain risk. Prefer pinning to a specific tag/commit, or (as other workflows here do) attempting to checkout the same branch as the PR/commit (with a fallback if the branch doesn’t exist). Also consider --depth 1 to avoid cloning full history.
| git clone https://github.com/rdkcentral/bluetooth.git | |
| BTRCORE_REPO="https://github.com/rdkcentral/bluetooth.git" | |
| BTRCORE_BRANCH="${{ github.head_ref || github.ref_name }}" | |
| BTRCORE_FALLBACK_BRANCH="main" | |
| if git ls-remote --exit-code --heads "$BTRCORE_REPO" "$BTRCORE_BRANCH" >/dev/null 2>&1; then | |
| git clone --depth 1 --branch "$BTRCORE_BRANCH" "$BTRCORE_REPO" | |
| else | |
| git clone --depth 1 --branch "$BTRCORE_FALLBACK_BRANCH" "$BTRCORE_REPO" | |
| fi |
PreethiLakshmi91
force-pushed
the
RDKEMW-14533_changes
branch
from
95a0545 to
5a52553
Compare
PreethiLakshmi91
force-pushed
the
RDKEMW-14533_changes
branch
from
5a52553 to
8bd6af9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (4)
cov_build.sh:21
- cov_build.sh runs
cmake ..from inside cov_build/, but this repo has no CMakeLists.txt — it's an Autotools project (configure.ac, Makefile.am). Both the Coverity branch and the fallback branch will fail. The script needs to use autotools (autoreconf -i && ../configure && make) for both the Coverity and fallback paths.
# If Coverity is available, use real cov-build
if command -v cov-build >/dev/null 2>&1; then
echo "Coverity detected, running real analysis..."
cov-build --dir cov-int cmake ..
cov-build --dir cov-int make -j$(nproc)
else
echo "Coverity not installed. Running normal build as fallback..."
cmake ..
make -j$(nproc)
fi
cov_build.sh:16
- Calling
cov-buildtwice with the same--dir cov-intfor separatecmakeandmakeinvocations is not the standard Coverity usage. Typically you invokecov-build --dir cov-intonce around the actual compilation command (e.g. themakestep), sincecov-buildintercepts compiler invocations. Wrappingcmake(a configuration step that runs no compilers) provides no value, and running two separate cov-build invocations into the same directory can produce inconsistent intermediate state. Consider running configuration normally and then a singlecov-build --dir cov-int make -j$(nproc).
cov-build --dir cov-int cmake ..
cov-build --dir cov-int make -j$(nproc)
build_dependencies.sh:11
- Using
sudoinside a script that's also invoked from a GitHub Actions workflow (which already runs apt-get with sudo) is inconsistent and prevents the script from being used in container environments where sudo may not exist. Consider droppingsudofrom this script and letting the caller decide, or detecting whether sudo is needed.
sudo apt-get update
# Install core dependencies
sudo apt-get install -y \
cov_build.sh:21
- The job name and step descriptions advertise this as a "Coverity-style" static analysis run, but the script falls back silently to a plain
makebuild when cov-build is not on PATH (which it never will be on a stock ubuntu-22.04 runner without a separately installed Coverity toolchain). The CI will report success without performing any static analysis, giving a false sense of coverage. Either install/set up Coverity in the workflow or fail the step explicitly when cov-build is unavailable.
else
echo "Coverity not installed. Running normal build as fallback..."
cmake ..
make -j$(nproc)
fi
| # Update system | ||
| sudo apt-get update | ||
|
|
||
| # Install core dependencies | ||
| sudo apt-get install -y \ | ||
| build-essential \ | ||
| cmake \ | ||
| pkg-config \ | ||
| git \ | ||
| wget \ | ||
| libglib2.0-dev \ | ||
| libbluetooth-dev \ | ||
| libgstreamer1.0-dev \ | ||
| libgstreamer-plugins-base1.0-dev | ||
|
|
| - name: Build bluetooth_mgr | ||
| run: | | ||
| mkdir -p build | ||
| cd build | ||
| cmake .. | ||
| make -j$(nproc) | ||
|
|
||
| - name: Run Static Analysis (Coverity-style mock) | ||
| run: | | ||
| chmod +x cov_build.sh | ||
| ./cov_build.sh | ||
|
|
||
| - name: Archive Build Artifacts | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: bluetooth-mgr-build | ||
| path: build/ |
| rm -rf ${BUILD_DIR} | ||
| mkdir ${BUILD_DIR} | ||
| cd ${BUILD_DIR} | ||
|
|
||
| # If Coverity is available, use real cov-build | ||
| if command -v cov-build >/dev/null 2>&1; then | ||
| echo "Coverity detected, running real analysis..." | ||
| cov-build --dir cov-int cmake .. | ||
| cov-build --dir cov-int make -j$(nproc) | ||
| else | ||
| echo "Coverity not installed. Running normal build as fallback..." | ||
| cmake .. | ||
| make -j$(nproc) |
PreethiLakshmi91
force-pushed
the
RDKEMW-14533_changes
branch
from
8bd6af9 to
45d548b
Compare
Reason for change: Crash fix Test Procedure: Device deepsleep causing the crash Risks: Low Priority: P2 Signed-off-by: ppalan289 <preethi_palanisamy@comcast.com>
PreethiLakshmi91
force-pushed
the
RDKEMW-14533_changes
branch
from
45d548b to
277218f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
Comments suppressed due to low confidence (2)
.github/workflows/native_full_build.yml:60
- The workflow is named "BT-Mgr Coverity Incremental Analysis Scan", but no Coverity step is actually executed here — the job only installs dependencies, runs autotools, configure, and
make. There is nocov-build, nocov-analyze, no upload to Coverity Scan, andcov_build.sh(added in this same PR) is not invoked. Either rename the workflow to reflect that it is a plain native build, or add the actual Coverity analysis/upload steps so the name matches the behavior.
name: BT-Mgr Coverity Incremental Analysis Scan
on:
push:
branches:
- develop
pull_request:
branches:
- develop
jobs:
native-build:
runs-on: ubuntu-22.04
env:
PREFIX_PATH: ${{ github.workspace }}/output
steps:
- name: Checkout source
uses: actions/checkout@v4
- name: Install build dependencies
run: |
sudo apt-get update
sudo apt-get install -y \
build-essential \
autoconf \
automake \
libtool \
pkg-config \
libunwind-dev \
libglib2.0-dev \
libdbus-1-dev \
libgstreamer1.0-dev \
libgstreamer-plugins-base1.0-dev \
libbluetooth-dev \
libcjson-dev
- name: Prepare autotools
run: |
libtoolize --force
aclocal
aclocal -I m4
automake --force-missing --add-missing
autoconf
- name: Configure
run: |
mkdir -p $PREFIX_PATH
CPPFLAGS="-I$PREFIX_PATH/include -I$PREFIX_PATH/include/cjson" \
LIBCJSON_CFLAGS="-I/usr/include/cjson" \
LDFLAGS="-L$PREFIX_PATH/lib" \
./configure \
--enable-gstreamer1=yes \
--enable-pi-build=yes \
--prefix=$PREFIX_PATH
- name: Build
run: |
make -j$(nproc)
.github/workflows/native_full_build.yml:1
- The workflow file is named
native_full_build.ymlbut the workflow'sname:is "BT-Mgr Coverity Incremental Analysis Scan". The filename and the displayed workflow name describe two different things (native build vs. Coverity incremental analysis). Pick one purpose and align the filename, thename:field, and the actual steps.
name: BT-Mgr Coverity Incremental Analysis Scan
| @@ -0,0 +1,60 @@ | |||
| name: BT-Mgr Coverity Incremental Analysis Scan | |||
| if command -v cov-build >/dev/null 2>&1; then | ||
| echo "Coverity detected, running real analysis..." | ||
| cov-build --dir cov-int cmake .. | ||
| cov-build --dir cov-int make -j$(nproc) | ||
| else | ||
| echo "Coverity not installed. Running normal build as fallback..." | ||
| cmake .. | ||
| make -j$(nproc) | ||
| fi |
| # If Coverity is available, use real cov-build | ||
| if command -v cov-build >/dev/null 2>&1; then | ||
| echo "Coverity detected, running real analysis..." | ||
| cov-build --dir cov-int cmake .. | ||
| cov-build --dir cov-int make -j$(nproc) | ||
| else | ||
| echo "Coverity not installed. Running normal build as fallback..." | ||
| cmake .. |
| rm -rf ${BUILD_DIR} | ||
| mkdir ${BUILD_DIR} | ||
| cd ${BUILD_DIR} |
| sudo apt-get install -y \ | ||
| build-essential \ | ||
| cmake \ | ||
| pkg-config \ | ||
| git \ | ||
| wget \ | ||
| libglib2.0-dev \ | ||
| libbluetooth-dev \ | ||
| libgstreamer1.0-dev \ | ||
| libgstreamer-plugins-base1.0-dev \ | ||
| python3 \ | ||
| python3-pip \ | ||
| lcov |
| echo "===== Installing bluetooth_mgr dependencies =====" | ||
|
|
||
| # Update system | ||
| sudo apt-get update | ||
|
|
||
| # Install core dependencies | ||
| sudo apt-get install -y \ |
| #!/bin/bash | ||
|
|
||
| set -e | ||
|
|
||
| echo "===== Starting Coverity-like Build =====" | ||
|
|
||
| BUILD_DIR="cov_build" | ||
| rm -rf ${BUILD_DIR} | ||
| mkdir ${BUILD_DIR} | ||
| cd ${BUILD_DIR} | ||
|
|
||
| # If Coverity is available, use real cov-build | ||
| if command -v cov-build >/dev/null 2>&1; then | ||
| echo "Coverity detected, running real analysis..." | ||
| cov-build --dir cov-int cmake .. | ||
| cov-build --dir cov-int make -j$(nproc) | ||
| else | ||
| echo "Coverity not installed. Running normal build as fallback..." | ||
| cmake .. | ||
| make -j$(nproc) | ||
| fi | ||
|
|
||
| echo "===== Build Completed =====" |
| libtoolize --force | ||
| aclocal | ||
| aclocal -I m4 | ||
| automake --force-missing --add-missing | ||
| autoconf |
Reason for change: Crash fix
Test Procedure: Device deepsleep causing the crash
Risks: Low
Priority: P2