-
Notifications
You must be signed in to change notification settings - Fork 1
RDKB-63007 RDKB-63448: Update README.md for git submodule change #28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request updates the README.md documentation to reflect a migration from wrapper scripts to git submodules for managing the build_tools_workflows dependency. The change standardizes how RDK-B components integrate with the native build system by tracking build_tools_workflows as a git submodule instead of dynamically cloning it via wrapper scripts.
Changes:
- Replaces wrapper script approach with git submodule integration for build_tools_workflows
- Updates all command examples to use build_tools_workflows/cov_docker_script/ paths
- Removes extensive wrapper script documentation section (~177 lines) and replaces it with git submodule setup instructions
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Trust the workspace | ||
| git config --global --add safe.directory '*' | ||
|
|
||
| # Pull the latest changes for the native build system |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workflow uses actions/checkout@v3 without the submodules parameter, then manually runs git submodule update --init --recursive --remote on line 1181. Consider using actions/checkout@v3 with submodules: recursive to automatically check out submodules, which is cleaner and more conventional. Alternatively, document why the manual approach is preferred.
| # Pull the latest changes for the native build system | |
| # Pull the latest changes for the native build system | |
| # NOTE: We intentionally use a manual submodule update with --remote instead of | |
| # configuring actions/checkout@v3 with `submodules: recursive`. This ensures | |
| # submodules are updated to the latest remote revisions, not just the commits | |
| # pinned in the main repository, which is required by this native build system. |
| ``` | ||
|
|
||
| **Reference:** [moca-agent/cov_docker_script](https://github.com/rdkcentral/moca-agent/blob/develop/cov_docker_script) | ||
| **Reference:** [utopia/cov_docker_script](https://github.com/rdkcentral/utopia/blob/develop/cov_docker_script) |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reference has been updated from moca-agent to utopia, but the PR description does not explain why this reference was changed. Verify that the utopia repository actually contains the example structure being referenced and that it's a better reference than moca-agent for users to follow.
| **Reference:** [utopia/cov_docker_script](https://github.com/rdkcentral/utopia/blob/develop/cov_docker_script) | |
| **Reference (canonical example using this submodule-based layout):** [utopia/cov_docker_script](https://github.com/rdkcentral/utopia/blob/develop/cov_docker_script) — this component already uses the `build_tools_workflows` git submodule and mirrors the directory structure shown above. |
| ```bash | ||
| ./cov_docker_script/common_external_build.sh | ||
| # Initialize/update build_tools_workflows submodule | ||
| git submodule update --init --recursive --remote |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent usage of --remote flag in git submodule commands throughout the documentation. Some examples (lines 133, 374, 1181, 1346, 1404) include --remote while others (lines 322, 1286) do not. The --remote flag updates submodules to the latest commit on the remote branch, which can lead to non-reproducible builds. Consider standardizing on git submodule update --init --recursive without --remote for reproducible builds, or clearly document when and why --remote should be used.
| git submodule update --init --recursive --remote | |
| git submodule update --init --recursive |
| - name: Run Native Build | ||
| run: ./cov_docker_script/common_external_build.sh | ||
| run: | | ||
| # Trust the workspace |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The git configuration command git config --global --add safe.directory '*' adds all directories as safe, which could be a security concern in some environments. While this may be necessary for GitHub Actions, consider adding a comment explaining why this is needed or documenting any security implications.
| # Trust the workspace | |
| # Trust the workspace. In some GitHub-hosted container environments, Git may | |
| # consider the checked-out repository as having "dubious ownership" unless it | |
| # is marked as safe. The following command globally trusts all directories and | |
| # is intended for use only in ephemeral CI containers, not on shared or | |
| # long-lived developer machines. |
| # Build and install dependencies | ||
| chmod +x build_tools_workflows/cov_docker_script/setup_dependencies.sh | ||
| ./build_tools_workflows/cov_docker_script/setup_dependencies.sh ./cov_docker_script/component_config.json | ||
|
|
||
| # Build component |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The chmod commands in the GitHub Actions workflow may be redundant if the scripts in the build_tools_workflows repository already have executable permissions. Consider verifying whether these chmod commands are necessary, or add a comment explaining why they're needed.
| # Build and install dependencies | |
| chmod +x build_tools_workflows/cov_docker_script/setup_dependencies.sh | |
| ./build_tools_workflows/cov_docker_script/setup_dependencies.sh ./cov_docker_script/component_config.json | |
| # Build component | |
| # Build and install dependencies | |
| # Ensure the script is executable; submodule or archive checkouts may not preserve the executable bit in CI. | |
| chmod +x build_tools_workflows/cov_docker_script/setup_dependencies.sh | |
| ./build_tools_workflows/cov_docker_script/setup_dependencies.sh ./cov_docker_script/component_config.json | |
| # Build component | |
| # As above, explicitly set execute permission to avoid failures when permissions are not preserved. |
| #### Step 3: Add to .gitignore | ||
|
|
||
| Add the following to your component's `.gitignore` to exclude temporary build artifacts: | ||
|
|
||
| ```gitignore | ||
| # Dependency build artifacts | ||
| build/ | ||
| ``` | ||
|
|
||
| #### Step 4: Add build_tools_workflows as Git Submodule |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing consistency in the step numbering. The documentation shows "Step 3: Add to .gitignore" (line 294), then "Step 4: Add build_tools_workflows as Git Submodule" (line 303), but logically adding the submodule should come before adding entries to .gitignore. Consider reordering these steps so that the submodule is added first (Step 3), then .gitignore is updated (Step 4), as this follows a more natural workflow.
|
|
||
| # Create run_setup_dependencies.sh and run_native_build.sh | ||
| # (use wrapper script templates from this README) | ||
| # 3. Add build/ to .gitignore |
Copilot
AI
Feb 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "build/ to .gitignore" but the example shows echo "build/" >> .gitignore. This will append to the .gitignore file, which is correct, but consider adding a comment in the example explaining that this assumes .gitignore already exists, or showing how to create it if it doesn't exist.
| # 3. Add build/ to .gitignore | |
| # 3. Ensure .gitignore exists and add build/ to it | |
| touch .gitignore |
Reason for change: Added submodule change for Native Build in cov_docker_script/README.md file
Test Procedure: Native build workflow should pass for all components
Risks: Low
Priority: P1
Signed-off-by: Hirrangandhi_Devaraj@comcast.com