RDKE-1024 : Add target toolchain libs feed support for docker#343
RDKE-1024 : Add target toolchain libs feed support for docker#343sreejithravi086 wants to merge 6 commits intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds support for a target toolchain libraries feed for Docker builds in the RDK build system. The changes enable the system to download and use prebuilt toolchain libraries from a remote feed when building for Docker containers, as an alternative to building them locally.
Changes:
- Introduces a new
TOOLCHAIN_LAYER_ARCHarchitecture identifier for toolchain packages - Adds conditional logic to populate
IPK_FEED_URISwith a toolchain library feed when appropriate - Reclassifies several core toolchain packages (glibc, libgcc, gcc-runtime, gcc-sanitizers) to use the toolchain architecture instead of the OSS layer architecture
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| TOOLCHAIN_LAYER_ARCH = "${@get_oss_arch(d)}-toolchain" | ||
| PACKAGE_EXTRA_ARCHS:append = " ${TOOLCHAIN_LAYER_ARCH}" | ||
| TOOLCHAIN_TARGET_LIBS_URI ?= "" | ||
| IPK_FEED_URIS:append = " ${@(' %s##%s' % (d.getVar('TOOLCHAIN_LAYER_ARCH'), d.getVar('TOOLCHAIN_TARGET_LIBS_URI')) if d.getVar('TOOLCHAIN_TARGET_LIBS_URI') and (not d.getVar('ENABLE_DOCKER_TARGET_TOOLCHAIN_FEED') == '1' or not os.path.isdir(d.getVar('PREBUILT_TOOLCHAIN_TARGET_LIBS') or '')) else '')}" |
There was a problem hiding this comment.
The conditional logic in this line is complex and could be difficult to maintain. Consider refactoring this into a helper function that clearly expresses the intent. For example:
def should_add_toolchain_feed_uri(d):
"""
Add toolchain feed URI if:
- TOOLCHAIN_TARGET_LIBS_URI is defined AND
- (Docker feed is not enabled OR prebuilt directory doesn't exist)
"""
if not d.getVar('TOOLCHAIN_TARGET_LIBS_URI'):
return False
docker_feed_enabled = d.getVar('ENABLE_DOCKER_TARGET_TOOLCHAIN_FEED') == '1'
prebuilt_dir = d.getVar('PREBUILT_TOOLCHAIN_TARGET_LIBS') or ''
prebuilt_exists = os.path.isdir(prebuilt_dir)
return not docker_feed_enabled or not prebuilt_existsThis would make the code more readable and easier to test. The inline expression could then be simplified to use this helper function.
| PACKAGE_EXTRA_ARCHS:append = " ${OSS_LAYER_ARCH}" | ||
|
|
||
| TOOLCHAIN_LAYER_ARCH = "${@get_oss_arch(d)}-toolchain" | ||
| PACKAGE_EXTRA_ARCHS:append = " ${TOOLCHAIN_LAYER_ARCH}" |
There was a problem hiding this comment.
The variables ENABLE_DOCKER_TARGET_TOOLCHAIN_FEED and PREBUILT_TOOLCHAIN_TARGET_LIBS are used in the conditional logic but are not defined or documented in this file. Consider adding comments to document:
- What ENABLE_DOCKER_TARGET_TOOLCHAIN_FEED controls (expected values and behavior)
- What PREBUILT_TOOLCHAIN_TARGET_LIBS should contain (expected to be a directory path)
- The relationship between these variables and TOOLCHAIN_TARGET_LIBS_URI
This would help users understand how to configure the toolchain feed mechanism and when each approach is used.
| PACKAGE_EXTRA_ARCHS:append = " ${TOOLCHAIN_LAYER_ARCH}" | |
| PACKAGE_EXTRA_ARCHS:append = " ${TOOLCHAIN_LAYER_ARCH}" | |
| # Toolchain feed configuration: | |
| # - TOOLCHAIN_TARGET_LIBS_URI: | |
| # URI of the remote IPK feed that provides target toolchain libraries | |
| # for ${TOOLCHAIN_LAYER_ARCH}. When non-empty, this URI is appended | |
| # to IPK_FEED_URIS, unless one of the local/alternative mechanisms | |
| # below is in use. | |
| # | |
| # - ENABLE_DOCKER_TARGET_TOOLCHAIN_FEED: | |
| # When set to "1", a Docker image is expected to provide the target | |
| # toolchain libraries, so no remote TOOLCHAIN_TARGET_LIBS_URI feed | |
| # is added for ${TOOLCHAIN_LAYER_ARCH}. | |
| # | |
| # - PREBUILT_TOOLCHAIN_TARGET_LIBS: | |
| # Absolute path to a directory on the build host that contains | |
| # prebuilt target toolchain libraries. If this variable is set and | |
| # points to an existing directory (os.path.isdir(...)), the local | |
| # directory is used instead of the remote TOOLCHAIN_TARGET_LIBS_URI | |
| # feed. | |
| # | |
| # The IPK_FEED_URIS:append expression below adds TOOLCHAIN_TARGET_LIBS_URI | |
| # only when a remote feed is configured (TOOLCHAIN_TARGET_LIBS_URI != "") | |
| # and neither a Docker-provided feed nor a valid PREBUILT_TOOLCHAIN_TARGET_LIBS | |
| # directory is in use. |
| OSS_LAYER_ARCH = "${@get_oss_arch(d)}${LAYER_EXTENSION}" | ||
| PACKAGE_EXTRA_ARCHS:append = " ${OSS_LAYER_ARCH}" | ||
|
|
||
| TOOLCHAIN_LAYER_ARCH = "${@get_oss_arch(d)}-toolchain" |
There was a problem hiding this comment.
TOOLCHAIN_LAYER_ARCH does not include LAYER_EXTENSION, unlike OSS_LAYER_ARCH which does include it (line 25). This means toolchain libraries will be shared across all layer extensions. Verify this is intentional - if different LAYER_EXTENSION values represent incompatible build configurations, toolchain libraries from different extensions might have ABI incompatibilities. If this sharing is intentional to optimize storage/bandwidth, consider adding a comment explaining this design decision.
| TOOLCHAIN_LAYER_ARCH = "${@get_oss_arch(d)}-toolchain" | |
| TOOLCHAIN_LAYER_ARCH = "${@get_oss_arch(d)}${LAYER_EXTENSION}-toolchain" |
No description provided.