Conversation
There was a problem hiding this comment.
Pull request overview
Adds a generic, reusable native-build pipeline (dependencies + component build) intended to standardize “native builds outside Yocto” for enabling Coverity scans across RDK-B components.
Changes:
- Introduces bash scripts to clone/build dependency repos and install headers/libraries into a local prefix.
- Adds a native component build script (autotools/cmake), including optional patching and pre-build command execution.
- Adds an orchestrator script plus a comprehensive README describing configuration and usage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| cov_docker_script/setup_dependencies.sh | Clones dependencies from JSON, copies headers, builds via multiple build systems, and stages libraries. |
| cov_docker_script/common_external_build.sh | Orchestrates end-to-end build: dependency setup then native component build. |
| cov_docker_script/common_build_utils.sh | Shared utilities for cloning, patching, and building with autotools/cmake/meson. |
| cov_docker_script/build_native.sh | Builds the “native component” per JSON config; supports patches and pre-build commands. |
| cov_docker_script/README.md | Full documentation for adoption, configuration schema, and CI/Coverity workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Validate required tools | ||
| check_dependencies() { | ||
| local required_tools=("git" "jq" "gcc" "make") |
There was a problem hiding this comment.
The scripts call python3 (e.g., apply_patch) but check_dependencies() only verifies git, jq, gcc, and make. This causes runtime failures in otherwise-valid configurations that use patches or pre-build steps. Add python3 to the required tools (and consider checking cmake/meson/ninja/autoreconf conditionally based on the selected build type).
| local required_tools=("git" "jq" "gcc" "make") | |
| local required_tools=("git" "jq" "gcc" "make" "python3") |
| if ! python3 -c " | ||
| import sys | ||
| with open('$file', 'r') as f: | ||
| content = f.read() | ||
| content = content.replace('''$search''', '''$replace''') | ||
| with open('$file', 'w') as f: | ||
| f.write(content) | ||
| "; then |
There was a problem hiding this comment.
apply_patch embeds file/search/replace directly into a Python one-liner. If search or replace contains ''', backslashes, or other special characters, the generated Python code can break or behave incorrectly. Pass data via stdin/argv (e.g., base64 or JSON) or use a heredoc to avoid code-generation bugs.
| if ! python3 -c " | |
| import sys | |
| with open('$file', 'r') as f: | |
| content = f.read() | |
| content = content.replace('''$search''', '''$replace''') | |
| with open('$file', 'w') as f: | |
| f.write(content) | |
| "; then | |
| if ! python3 - "$file" "$search" "$replace" << 'EOF' | |
| import sys | |
| if len(sys.argv) != 4: | |
| sys.exit("Usage: script.py <file> <search> <replace>") | |
| file_path = sys.argv[1] | |
| search = sys.argv[2] | |
| replace = sys.argv[3] | |
| with open(file_path, 'r') as f: | |
| content = f.read() | |
| content = content.replace(search, replace) | |
| with open(file_path, 'w') as f: | |
| f.write(content) | |
| EOF | |
| then |
| # Ensure PKG_CONFIG_PATH is set for configure | ||
| export PKG_CONFIG_PATH="${HOME}/usr/local/lib/pkgconfig:${HOME}/usr/lib/pkgconfig:${PKG_CONFIG_PATH:-}" |
There was a problem hiding this comment.
build_autotools hardcodes ${HOME}/usr/... when setting PKG_CONFIG_PATH, which ignores USR_DIR/custom install locations established by setup_dependencies.sh. This can make dependency builds fail when users override USR_DIR. Prefer deriving the pkgconfig roots from USR_DIR (or avoid overriding PKG_CONFIG_PATH here and rely on the caller’s environment).
| # Ensure PKG_CONFIG_PATH is set for configure | |
| export PKG_CONFIG_PATH="${HOME}/usr/local/lib/pkgconfig:${HOME}/usr/lib/pkgconfig:${PKG_CONFIG_PATH:-}" | |
| # Ensure PKG_CONFIG_PATH is set for configure, honoring USR_DIR if provided | |
| local usr_root="${USR_DIR:-$HOME/usr}" | |
| export PKG_CONFIG_PATH="${usr_root}/local/lib/pkgconfig:${usr_root}/lib/pkgconfig:${PKG_CONFIG_PATH:-}" |
| local configure_flags make_targets parallel_make | ||
| configure_flags=$(jq -r ".dependencies.repos[$index].build.configure_flags // empty" "$CONFIG_FILE") | ||
| make_targets=$(jq -r ".dependencies.repos[$index].build.make_targets[]? // \"all\"" "$CONFIG_FILE" | tr '\n' ' ') | ||
| parallel_make=$(jq -r ".dependencies.repos[$index].build.parallel_make // true" "$CONFIG_FILE") |
There was a problem hiding this comment.
The jq expression .build.make_targets[]? // "all" (and similar for ninja targets) does not apply the default when the array is missing because []? yields an empty stream (not null). This leaves make_targets empty instead of defaulting to all. Use a pattern like (.dependencies.repos[$index].build.make_targets // ["all"])[] (and then join) to get a reliable default.
|
|
||
| # Make | ||
| local make_targets | ||
| make_targets=$(jq -r '.native_component.build.make_targets[]? // "all"' "$CONFIG_FILE" | tr '\n' ' ') |
There was a problem hiding this comment.
Same jq defaulting issue as in dependency builds: .native_component.build.make_targets[]? // "all" won’t emit all when make_targets is absent. Use (.native_component.build.make_targets // ["all"])[] (and join) so builds behave consistently when targets aren’t specified.
| make_targets=$(jq -r '.native_component.build.make_targets[]? // "all"' "$CONFIG_FILE" | tr '\n' ' ') | |
| make_targets=$(jq -r '(.native_component.build.make_targets // ["all"])[]' "$CONFIG_FILE" | tr '\n' ' ') |
| echo -e "$content" > "$file" | ||
| if [[ $? -ne 0 ]]; then |
There was a problem hiding this comment.
apply_patch writes created files using echo -e, which will interpret backslash escapes and can corrupt intended content (and behaves differently across shells). Use a non-escape-processing write (e.g., printf '%s') so JSON-provided content is written verbatim.
| echo -e "$content" > "$file" | |
| if [[ $? -ne 0 ]]; then | |
| if ! printf '%s' "$content" > "$file"; then |
| build_dir=$(jq -r '.native_component.build.build_dir // "build"' "$CONFIG_FILE") | ||
| cmake_flags=$(jq -r '.native_component.build.cmake_flags // empty' "$CONFIG_FILE") | ||
| cmake_flags=$(expand_path "$cmake_flags") | ||
| make_targets=$(jq -r '.native_component.build.make_targets[]? // "all"' "$CONFIG_FILE" | tr '\n' ' ') |
There was a problem hiding this comment.
Same jq defaulting issue here: .native_component.build.make_targets[]? // "all" produces an empty string when make_targets is missing. Switch to (.native_component.build.make_targets // ["all"])[] (and join) so CMake builds have a stable default target list.
| make_targets=$(jq -r '.native_component.build.make_targets[]? // "all"' "$CONFIG_FILE" | tr '\n' ' ') | |
| make_targets=$(jq -r '(.native_component.build.make_targets // ["all"])[]' "$CONFIG_FILE" | tr '\n' ' ') |
| # Expand variables safely | ||
| command=$(eval echo "$command") | ||
|
|
There was a problem hiding this comment.
command=$(eval echo "$command") performs an extra shell evaluation pass and will execute command substitutions during “expansion” (before the actual command run). Since you already execute the command later, this double-eval increases injection risk and can change behavior unexpectedly. Prefer deterministic expansion (e.g., only replacing $HOME, or using envsubst), and execute the command once.
| # Expand variables safely | |
| command=$(eval echo "$command") |
| # Setup PKG_CONFIG_PATH and LD_LIBRARY_PATH | ||
| export PKG_CONFIG_PATH="$LIB_PATH/pkgconfig:${PKG_CONFIG_PATH:-}" | ||
| export LD_LIBRARY_PATH="$LIB_PATH:${LD_LIBRARY_PATH:-}" | ||
|
|
||
| # Add common include and lib paths | ||
| export CPPFLAGS="${CPPFLAGS:-} -I$HEADER_PATH" | ||
| export CFLAGS="${CFLAGS:-} -I$HEADER_PATH" | ||
| export LDFLAGS="${LDFLAGS:-} -L$LIB_PATH" |
There was a problem hiding this comment.
common_external_build.sh runs setup_dependencies.sh as a separate process, so its exported PKG_CONFIG_PATH/LD_LIBRARY_PATH won’t persist into this script. Here, PKG_CONFIG_PATH is set to only $LIB_PATH/pkgconfig, which can miss dependency .pc files typically installed under $HOME/usr/lib/pkgconfig (and other locations). Consider constructing PKG_CONFIG_PATH/LD_LIBRARY_PATH to include both $HOME/usr/local/lib{,/pkgconfig} and $HOME/usr/lib{,/pkgconfig} (or derive from USR_DIR) so native builds can find dependency pkg-config metadata reliably.
| **Copy the directory structure from reference:** | ||
|
|
||
| ```bash | ||
| # Example structure | ||
| your-component/ | ||
| ├── cov_docker_script/ | ||
| │ ├── common_build_utils.sh # Utility functions | ||
| │ ├── setup_dependencies.sh # Dependency setup | ||
| │ ├── build_native.sh # Component build | ||
| │ ├── common_external_build.sh # Orchestrator | ||
| │ ├── component_config.json # Configuration | ||
| │ └── configure_options.conf # Build flags (optional) | ||
| ├── source/ |
There was a problem hiding this comment.
The scripts default to cov_docker_script/component_config.json, but this PR doesn’t add a component_config.json (or a template) to the directory. As-is, common_external_build.sh fails immediately when run with defaults. Add a minimal example/template component_config.json (and optionally configure_options.conf) or update the docs/scripts to make the required config file creation explicit.
Scripts for native build across all rdkcentral components to enable coverity scan