RDKEMW-14533: MRG - Coverity inclusion changes#92
RDKEMW-14533: MRG - Coverity inclusion changes#92PreethiLakshmi91 wants to merge 1 commit intordkcentral:developfrom
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.
17a264b to
63e2e7d
Compare
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 |
681bdff to
e9428ec
Compare
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 |
Reason for change: Crash fix Test Procedure: Device deepsleep causing the crash Risks: Low Priority: P2 Signed-off-by: ppalan289 <preethi_palanisamy@comcast.com>
95a0545 to
5a52553
Compare
Reason for change: Crash fix
Test Procedure: Device deepsleep causing the crash
Risks: Low
Priority: P2