Update cov_docker_script/README.md#26
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the README.md documentation for the cov_docker_script build system to introduce a new recommended approach using wrapper scripts that automatically fetch build tools from the build_tools_workflows repository, rather than committing build scripts directly to component repositories.
Changes:
- Updated GitHub Actions badge link to point to generic documentation instead of a specific implementation
- Renamed "Migration Guide" to "Component Adoption Guide" throughout the documentation
- Added comprehensive documentation for the new wrapper script approach, including templates for
run_setup_dependencies.shandrun_native_build.sh - Updated Quick Start guide to differentiate between wrapper script approach (recommended) and legacy approach
- Updated examples to use generic component names instead of moca-agent specific examples
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cov_docker_script/README.md
Outdated
|
|
||
| #### Example: moca-agent | ||
|
|
||
| See [moca-agent/cov_docker_script](https://github.com/rdkcentral/moca-agent/tree/feature/cov_native_build/cov_docker_script) for a complete working example. |
There was a problem hiding this comment.
The reference link points to a specific branch 'feature/cov_native_build' which may be temporary. If this is a feature branch that will be merged or deleted, the link will become broken. Consider updating the reference to point to the main/master branch once the feature is merged, or documenting that this is a temporary reference.
| See [moca-agent/cov_docker_script](https://github.com/rdkcentral/moca-agent/tree/feature/cov_native_build/cov_docker_script) for a complete working example. | |
| See [moca-agent/cov_docker_script](https://github.com/rdkcentral/moca-agent/tree/main/cov_docker_script) for a complete working example. |
| log "Cloning build_tools_workflows (develop)" | ||
| cd "$NATIVE_COMPONENT_DIR" | ||
| git clone -b develop "$BUILD_TOOLS_REPO_URL" || { err "Clone failed"; exit 1; } | ||
| ok "Repository cloned" |
There was a problem hiding this comment.
The clone command uses -b develop flag to specify the branch, but the repository URL doesn't verify if the 'develop' branch exists. If the develop branch doesn't exist in the build_tools_workflows repository, this command will fail. Consider adding a fallback to 'main' or 'master' branch, or making the branch configurable via an environment variable.
| log "Cloning build_tools_workflows (develop)" | |
| cd "$NATIVE_COMPONENT_DIR" | |
| git clone -b develop "$BUILD_TOOLS_REPO_URL" || { err "Clone failed"; exit 1; } | |
| ok "Repository cloned" | |
| : "${BUILD_TOOLS_BRANCH:=develop}" | |
| log "Cloning build_tools_workflows (branch: $BUILD_TOOLS_BRANCH)" | |
| cd "$NATIVE_COMPONENT_DIR" | |
| if git clone -b "$BUILD_TOOLS_BRANCH" "$BUILD_TOOLS_REPO_URL"; then | |
| ok "Repository cloned (branch: $BUILD_TOOLS_BRANCH)" | |
| else | |
| log "Branch '$BUILD_TOOLS_BRANCH' not available, attempting fallbacks ('main', then 'master')" | |
| if git ls-remote --exit-code --heads "$BUILD_TOOLS_REPO_URL" main >/dev/null 2>&1 && \ | |
| git clone -b main "$BUILD_TOOLS_REPO_URL"; then | |
| ok "Repository cloned (branch: main)" | |
| elif git ls-remote --exit-code --heads "$BUILD_TOOLS_REPO_URL" master >/dev/null 2>&1 && \ | |
| git clone -b master "$BUILD_TOOLS_REPO_URL"; then | |
| ok "Repository cloned (branch: master)" | |
| else | |
| err "Clone failed: branches '$BUILD_TOOLS_BRANCH', 'main', and 'master' are not available" | |
| exit 1 | |
| fi | |
| fi |
| if [[ -d "$BUILD_TOOLS_DIR" ]]; then | ||
| log "build_tools_workflows already exists, skipping clone" | ||
| else | ||
| log "Cloning build_tools_workflows (develop)" | ||
| cd "$NATIVE_COMPONENT_DIR" | ||
| git clone -b develop "$BUILD_TOOLS_REPO_URL" || { err "Clone failed"; exit 1; } | ||
| ok "Repository cloned" | ||
| fi |
There was a problem hiding this comment.
The run_setup_dependencies.sh wrapper script doesn't handle the case where build_tools_workflows exists but might be outdated or corrupted. Consider adding a check to verify the repository state or an option to force re-clone. Currently, it only skips cloning if the directory exists, which could lead to using stale or incomplete build tools.
| BUILD_TOOLS_REPO_URL="https://github.com/rdkcentral/build_tools_workflows" | ||
| BUILD_TOOLS_DIR="$NATIVE_COMPONENT_DIR/build_tools_workflows" | ||
| REQUIRED_SCRIPTS=("build_native.sh" "common_build_utils.sh" "common_external_build.sh" "setup_dependencies.sh") | ||
|
|
||
| # Basic logging | ||
| log() { echo "[INFO] $*"; } | ||
| ok() { echo "[OK] $*"; } | ||
| err() { echo "[ERROR] $*" >&2; } | ||
|
|
||
| echo "" | ||
| echo "===== Setup Dependencies Pipeline =====" | ||
| echo "" | ||
|
|
||
| # Clone build_tools_workflows | ||
| if [[ -d "$BUILD_TOOLS_DIR" ]]; then | ||
| log "build_tools_workflows already exists, skipping clone" | ||
| else | ||
| log "Cloning build_tools_workflows (develop)" | ||
| cd "$NATIVE_COMPONENT_DIR" | ||
| git clone -b develop "$BUILD_TOOLS_REPO_URL" || { err "Clone failed"; exit 1; } | ||
| ok "Repository cloned" | ||
| fi | ||
|
|
||
| # Verify required scripts | ||
| [[ ! -d "$BUILD_TOOLS_DIR/cov_docker_script" ]] && { err "cov_docker_script not found"; exit 1; } | ||
|
|
||
| log "Verifying required scripts..." | ||
| MISSING=() | ||
| for script in "${REQUIRED_SCRIPTS[@]}"; do | ||
| [[ -f "$BUILD_TOOLS_DIR/cov_docker_script/$script" ]] || MISSING+=("$script") | ||
| done | ||
|
|
||
| if [[ ${#MISSING[@]} -gt 0 ]]; then | ||
| err "Missing scripts: ${MISSING[*]}" | ||
| exit 1 | ||
| fi | ||
| ok "All required scripts found" | ||
|
|
||
| # Verify setup_dependencies.sh exists before running | ||
| if [[ ! -f "$BUILD_TOOLS_DIR/cov_docker_script/setup_dependencies.sh" ]]; then | ||
| err "setup_dependencies.sh not found in build_tools_workflows" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Run setup_dependencies.sh from build_tools_workflows | ||
| echo "" | ||
| log "Running setup_dependencies.sh from build_tools_workflows..." | ||
| cd "$NATIVE_COMPONENT_DIR" | ||
| "$BUILD_TOOLS_DIR/cov_docker_script/setup_dependencies.sh" "$SCRIPT_DIR/component_config.json" |
There was a problem hiding this comment.
Security concern: The wrapper script clones from a hardcoded GitHub URL without verifying the integrity of the downloaded scripts before executing them. An attacker who gains control of the build_tools_workflows repository or performs a man-in-the-middle attack could inject malicious code. Consider adding verification steps such as checking commit signatures, verifying checksums, or pinning to a specific commit hash instead of the 'develop' branch.
| rm -rf "$BUILD_TOOLS_DIR" | ||
| ok "Cleanup completed" |
There was a problem hiding this comment.
The cleanup operation uses rm -rf on a directory path constructed from variables. If any of the path components (NATIVE_COMPONENT_DIR or BUILD_TOOLS_DIR) are empty or incorrectly set, this could potentially delete unintended files. Consider adding a safety check to ensure BUILD_TOOLS_DIR is not empty and points to a valid subdirectory before deletion.
| rm -rf "$BUILD_TOOLS_DIR" | |
| ok "Cleanup completed" | |
| if [[ -n "$BUILD_TOOLS_DIR" && -d "$BUILD_TOOLS_DIR" ]]; then | |
| rm -rf "$BUILD_TOOLS_DIR" | |
| ok "Cleanup completed" | |
| else | |
| err "Skipping cleanup: BUILD_TOOLS_DIR is not set or not a directory: '$BUILD_TOOLS_DIR'" | |
| fi |
cov_docker_script/README.md
Outdated
| # - common_external_build.sh | ||
| ``` | ||
|
|
||
| **Reference:** [moca-agent/cov_docker_script](https://github.com/rdkcentral/moca-agent/blob/feature/cov_native_build/cov_docker_script) |
There was a problem hiding this comment.
The reference link points to a specific branch 'feature/cov_native_build' which may be temporary. If this is a feature branch that will be merged or deleted, the link will become broken. Consider updating the reference to point to the main/master branch once the feature is merged, or documenting that this is a temporary reference.
| **Reference:** [moca-agent/cov_docker_script](https://github.com/rdkcentral/moca-agent/blob/feature/cov_native_build/cov_docker_script) | |
| **Reference:** [moca-agent/cov_docker_script](https://github.com/rdkcentral/moca-agent/blob/main/cov_docker_script) |
| else | ||
| log "Cloning build_tools_workflows (develop)" | ||
| cd "$NATIVE_COMPONENT_DIR" | ||
| git clone -b develop "$BUILD_TOOLS_REPO_URL" || { err "Clone failed"; exit 1; } |
There was a problem hiding this comment.
The wrapper script clones the build_tools_workflows repository but doesn't specify any authentication mechanism. If the repository is private or requires authentication, the clone will fail silently with the generic error message "Clone failed". Consider adding more specific error messages that differentiate between network issues, authentication failures, and repository not found scenarios.
| git clone -b develop "$BUILD_TOOLS_REPO_URL" || { err "Clone failed"; exit 1; } | |
| if ! clone_output=$(git clone -b develop "$BUILD_TOOLS_REPO_URL" 2>&1); then | |
| status=$? | |
| if echo "$clone_output" | grep -qiE 'authentication failed|permission denied|access denied'; then | |
| err "Clone failed: authentication error while accessing '$BUILD_TOOLS_REPO_URL'." | |
| err "Ensure you have the correct credentials/SSH keys and access to the repository." | |
| elif echo "$clone_output" | grep -qiE 'not found|repository .* does not exist'; then | |
| err "Clone failed: repository not found for '$BUILD_TOOLS_REPO_URL' (branch: develop)." | |
| err "Verify that the URL and branch name are correct and that the repository exists." | |
| elif echo "$clone_output" | grep -qiE 'could not resolve host|failed to connect|network is unreachable'; then | |
| err "Clone failed: network error while trying to reach '$BUILD_TOOLS_REPO_URL'." | |
| err "Check your network connectivity and DNS settings." | |
| else | |
| err "Clone failed with exit code $status. git output:" | |
| err "$clone_output" | |
| fi | |
| exit 1 | |
| fi |
Reason for change: Update cov_docker_script README
Test Procedure: None
Risks: None
Priority: P2
Signed-off-by: Santhosh_GujulvaJagadeesh@comcast.com