RDKOSS-387: Add host_locked_sstate.bbclass - #91
Conversation
Reason for the change: This class enables leveraging a pre-populated locked sstate cache from the host machine or Docker container to avoid rebuilding native tools repeatedly, reducing overall build time. It supports both native and target components and allows configuration of the locked sstate path via HOST_LOCKED_SSTATE_PATH (default: /opt/locked_sstate). Signed-off-by: mselva006c <mani_selvaraj@comcast.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new BitBake class host_locked_sstate.bbclass to enable leveraging pre-populated locked sstate cache from the host machine or Docker container, reducing build time by avoiding repeated rebuilds of native tools.
Key Changes:
- Introduces
host_locked_sstate.bbclasswith configurable locked sstate path viaHOST_LOCKED_SSTATE_PATH(default:/opt/locked_sstate) - Implements automatic selection of appropriate locked-sigs files based on
DEFAULTTUNEandMULTILIBSconfiguration - Adds sstate mirror configuration to point to the host locked sstate cache
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # need to fix: Workaround to check for 64bit machine with multilib configuration | ||
| multilib_support = d.getVar('MULTILIBS') or "" | ||
|
|
||
| # Determine the correct sigs_file based on the configuration | ||
| if multilib_support: |
There was a problem hiding this comment.
The comment "need to fix: Workaround to check for 64bit machine with multilib configuration" indicates this is a temporary solution that needs improvement. The current logic assumes that ANY multilib configuration means ARM64, which may not always be correct. Consider implementing a more robust detection mechanism that explicitly checks for ARM64 architecture rather than relying solely on the presence of MULTILIBS.
| # need to fix: Workaround to check for 64bit machine with multilib configuration | |
| multilib_support = d.getVar('MULTILIBS') or "" | |
| # Determine the correct sigs_file based on the configuration | |
| if multilib_support: | |
| # Improved: Explicitly check for ARM64 architecture with multilib configuration | |
| multilib_support = d.getVar('MULTILIBS') or "" | |
| # Determine the correct sigs_file based on the configuration | |
| if multilib_support and ("aarch64" in default_tune or "arm64" in default_tune): |
| # - SIGGEN_LOCKEDSIGS_TASKSIG_CHECK is set to "warn" to allow flexibility. | ||
| # | ||
| #============================================================================== | ||
|
|
There was a problem hiding this comment.
Missing import statement for os module. The function get_locked_sig_file uses os.path.join() and os.path.exists() on lines 52-53, but the os module is not imported at the top of the file. This will cause a NameError when the function is executed.
| import os |
| if os.path.exists(sigs_file_path): | ||
| return sigs_file_path | ||
| else: | ||
| bb.fatal("ERROR: The expected locked sigs file(%s) is not found" %sigs_file_path) |
There was a problem hiding this comment.
The error message is redundant with "ERROR:" prefix. Since bb.fatal() already indicates a fatal error and will stop the build, adding "ERROR:" to the message is unnecessary. Consider simplifying to: bb.fatal("The expected locked sigs file (%s) is not found" % sigs_file_path). Also, add a space after "file" for readability.
| bb.fatal("ERROR: The expected locked sigs file(%s) is not found" %sigs_file_path) | |
| bb.fatal("The expected locked sigs file (%s) is not found" % sigs_file_path) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| SIG_FILE = "${@get_locked_sig_file(d)}" | ||
| include ${SIG_FILE} | ||
| SSTATE_MIRRORS += "file://.* file:///${HOST_LOCKED_SSTATE_PATH}/PATH" |
There was a problem hiding this comment.
The SSTATE_MIRRORS pattern appears incorrect. The literal string "PATH" should be replaced with the actual sstate path pattern. In BitBake, SSTATE_MIRRORS typically uses the format: "file://.* file:///${HOST_LOCKED_SSTATE_PATH}/PATH" where "PATH" is a placeholder that should be substituted with the actual path layout pattern (e.g., matching the SSTATE_DIR structure). Consider verifying this against BitBake documentation or using a tested pattern like "file://.* file:///${HOST_LOCKED_SSTATE_PATH}/" or the proper path substitution pattern.
| SSTATE_MIRRORS += "file://.* file:///${HOST_LOCKED_SSTATE_PATH}/PATH" | |
| SSTATE_MIRRORS += "file://.* file:///${HOST_LOCKED_SSTATE_PATH}/" |
| host_locked_sstate_path = d.expand('${HOST_LOCKED_SSTATE_PATH}') | ||
|
|
||
| default_tune = d.getVar('DEFAULTTUNE') | ||
| # need to fix: Workaround to check for 64bit machine with multilib configuration |
There was a problem hiding this comment.
The comment "need to fix: Workaround to check for 64bit machine with multilib configuration" indicates this is a temporary solution that should be addressed. Consider creating a ticket to track the proper implementation or removing this comment if the current approach is acceptable long-term.
| # need to fix: Workaround to check for 64bit machine with multilib configuration |
| if multilib_support: | ||
| sigs_file = "locked-sigs_rdk-arm64.inc" | ||
| elif "armv7athf-neon" in default_tune: | ||
| sigs_file = "locked-sigs_rdk-arm7a.inc" | ||
| elif "armv7vethf-neon" in default_tune: | ||
| sigs_file = "locked-sigs_rdk-arm7ve.inc" | ||
| else: | ||
| sigs_file = default_sigs_file |
There was a problem hiding this comment.
The architecture-specific filenames ("locked-sigs_rdk-arm64.inc", "locked-sigs_rdk-arm7a.inc", "locked-sigs_rdk-arm7ve.inc") are hardcoded magic strings. Consider defining these as configurable variables (e.g., LOCKED_SIGS_ARM64_FILE, LOCKED_SIGS_ARM7A_FILE) to improve maintainability and allow easier customization for different projects or configurations.
Reason for the change:
This class enables leveraging a pre-populated locked sstate cache from the host machine or Docker container to avoid rebuilding native tools repeatedly, reducing overall build time. It supports both native and target components and allows configuration of the locked sstate path via HOST_LOCKED_SSTATE_PATH (default: /opt/locked_sstate).