Conversation
Reason for change : Modiying recipes for westeros. Test Procedure : None Priority : P1 Risks : None Signed-off-by : Sajna_NazeerK@comcast.com RDKEMW-6929 : Westeros refactoring Reason for change : Modiying recipes for westeros. Test Procedure : None Priority : P1 Risks : None Signed-off-by : Sajna_NazeerK@comcast.com RDKEMW-6929 : Westeros refactoring Reason for change : Modiying recipes for westeros. Test Procedure : None Priority : P1 Risks : None Signed-off-by : Sajna_NazeerK@comcast.com RDKEMW-8961: Integration of refactored Westeros into RDKE builds Reason for change : Modiying recipes for westeros. Test Procedure : None Priority : P1 Risks : None Signed-off-by : Sajna_NazeerK@comcast.com RDKEMW-8961: Integration of refactored Westeros into RDKE builds Reason for change : Modiying recipes for westeros. Test Procedure : None Priority : P1 Risks : None Signed-off-by : Sajna_NazeerK@comcast.com RDKEMW-8961: Integration of refactored Westeros into RDKE builds Reason for change : Modiying recipes for westeros. Test Procedure : None Priority : P1 Risks : None Signed-off-by : Sajna_NazeerK@comcast.com RDKEMW-8961: Integration of refactored Westeros into RDKE builds Reason for change : Modiying recipes for westeros. Test Procedure : None Priority : P1 Risks : None Signed-off-by : Sajna_NazeerK@comcast.com RDKEMW-8961: Integration of refactored Westeros into RDKE builds Reason for change : Modiying recipes for westeros. Test Procedure : None Priority : P1 Risks : None Signed-off-by : Sajna_NazeerK@comcast.com RDKEVD-4868: [TV] Update Westeros to 1.01.63 in RDKE Reason for change : Bring ermgr with essosresmgr lib to VL build Test Procedure : None Priority : P1 Risks : None Signed-off-by : Sajna_NazeerK@comcast.com Move to version 1.01.63 RDKEVD-4868: [TV] Update Westeros to 1.01.63 in RDKE Reason for change : Change : Fixing build issue Test Procedure : None Priority : P1 Risks : None Signed-off-by : Sajna_NazeerK@comcast.com
|
b'## Copyright scan failure |
There was a problem hiding this comment.
Pull request overview
This PR upgrades the Westeros compositor to version 2.0.0 and refactors the related recipes by removing the common westeros.inc include file and moving configuration directly into individual recipe files. It also introduces a new essosrmgr (Essos Resource Manager) component as a separate recipe.
Changes:
- Removed
westeros.incand inlined configuration into individual Westeros recipe files (westeros.bb, westeros-simpleshell.bb, westeros-simplebuffer.bb) - Modified essos.bb to use a separate essos repository and added dependencies on essosrmgr and westeros
- Added new essosrmgr.bb recipe with systemd service integration
- Updated packagegroup-oss-layer.bb to include essosrmgr package
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| recipes-graphics/westeros/westeros.bb | Removed westeros.inc include, added direct license/source configuration, fixed spelling error in summary, added header file installation |
| recipes-graphics/westeros/westeros-simpleshell.bb | Removed westeros.inc include, added direct license/source configuration, fixed spelling error in summary |
| recipes-graphics/westeros/westeros-simplebuffer.bb | Removed westeros.inc include, added direct license/source configuration, fixed spelling error in summary |
| recipes-graphics/westeros/essos.bb | Changed to use separate essos repository instead of westeros subdir, updated dependencies to include essosrmgr and westeros |
| recipes-graphics/essosrmgr/essosrmgr.bb | New recipe for Essos Resource Manager with systemd service integration |
| recipes-core/packagegroups/packagegroup-oss-layer.bb | Added essosrmgr to both included and excluded package lists |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| LIC_FILES_CHKSUM = "file://${LICENSE_LOCATION};md5=8fb65319802b0c15fc9e0835350ffa02" | ||
|
|
||
| S = "${WORKDIR}/git/simpleshell" | ||
|
|
||
| SRC_URI = "git://github.com/rdkcentral/westeros;protocol=https;nobranch=1" | ||
|
|
||
| DEPENDS = "wayland glib-2.0 wayland-native" | ||
|
|
||
| LICENSE_LOCATION = "${S}/../LICENSE" |
There was a problem hiding this comment.
LICENSE_LOCATION is defined on line 12 but used on line 4 in LIC_FILES_CHKSUM. While BitBake may handle this due to lazy evaluation, it's better practice to define variables before they are referenced for clarity and to match the pattern in other files like westeros-simplebuffer.bb.
| LIC_FILES_CHKSUM = "file://${LICENSE_LOCATION};md5=8fb65319802b0c15fc9e0835350ffa02" | ||
|
|
||
| S = "${WORKDIR}/git/simpleshell" | ||
|
|
There was a problem hiding this comment.
The recipe is missing SRCREV and PV (package version) variables that were previously defined in westeros.inc. Without SRCREV, the build system won't know which specific commit to checkout from the git repository, making builds non-reproducible. Consider adding these variables to pin to a specific version.
| SRCREV = "1234567890abcdef1234567890abcdef12345678" | |
| PV = "1.0+git${SRCPV}" |
| LIC_FILES_CHKSUM = "file://${LICENSE_LOCATION};md5=8fb65319802b0c15fc9e0835350ffa02" | ||
|
|
||
| SRC_URI = "git://github.com/rdkcentral/essos;protocol=https;nobranch=1" | ||
|
|
There was a problem hiding this comment.
The recipe is missing SRCREV and PV (package version) variables. Without SRCREV, the build system won't know which specific commit to checkout from the git repository, making builds non-reproducible. Consider adding these variables to pin to a specific version.
| # Pin to a specific git revision for reproducible builds | |
| SRCREV = "abcdef1234567890abcdef1234567890abcdef12" | |
| PV = "1.0+git${SRCPV}" |
| inherit autotools pkgconfig features_check | ||
|
|
||
| PACKAGECONFIG ??= "westeros resmgr" | ||
| PACKAGECONFIG[westeros] = "--disable-essoswesterosfree,--enable-essoswesterosfree,westeros-simpleshell virtual/westeros-soc" | ||
| PACKAGECONFIG[resmgr] = "--disable-essosresmgrfree,--enable-essosresmgrfree" | ||
|
|
||
| # Specify any options you want to pass to the configure script using EXTRA_OECONF: | ||
| EXTRA_OECONF = "" | ||
|
|
||
| acpaths = "-I cfg" | ||
|
|
||
| do_install:append() { | ||
|
|
||
| install -d ${D}${systemd_unitdir}/system | ||
| install -m 0644 ${S}/conf/ermgr.service ${D}${systemd_unitdir}/system | ||
|
|
||
| if ${@bb.utils.contains('DISTRO_FEATURES', 'use_westeros_essrmgr_uds', 'true', 'false', d)}; then | ||
| sed -i "/^Before=/ s/ audioserver.service tvserver.service$//" ${D}${systemd_unitdir}/system/ermgr.service | ||
| sed -i "/^WantedBy=/ s/ui-init.target$/wpeframework.service/" ${D}${systemd_unitdir}/system/ermgr.service | ||
| fi | ||
| # appsservice expects ERM UDS in /tmp folder | ||
| sed -i "/^Environment=\"XDG_RUNTIME_DIR/ s/run\"$/tmp\"/" ${D}${systemd_unitdir}/system/ermgr.service | ||
|
|
||
| } | ||
|
|
||
| SYSTEMD_SERVICE:${PN} += "ermgr.service" |
There was a problem hiding this comment.
The recipe uses SYSTEMD_SERVICE and installs systemd service files but does not inherit from the systemd class. The systemd class should be added to the inherit line (line 15) to ensure proper systemd integration. Without this, systemd-related variables and functions may not work correctly. See recipes-extended/mdns/mdns_1556.80.2.bb:31 for an example of proper systemd inheritance.
| @@ -1,8 +1,10 @@ | |||
| include westeros.inc | |||
|
|
|||
| SUMMARY = "This receipe compiles the westeros compositor component" | |||
There was a problem hiding this comment.
Spelling error: "receipe" should be "recipe".
| SUMMARY = "This receipe compiles the westeros compositor component" | |
| SUMMARY = "This recipe compiles the westeros compositor component" |
| LICENSE = "Apache-2.0" | ||
|
|
||
| S = "${WORKDIR}/git/simplebuffer" | ||
|
|
There was a problem hiding this comment.
The recipe is missing SRCREV and PV (package version) variables that were previously defined in westeros.inc. Without SRCREV, the build system won't know which specific commit to checkout from the git repository, making builds non-reproducible. Consider adding these variables to pin to a specific version.
| PV = "1.0+git${SRCPV}" | |
| SRCREV = "0123456789abcdef0123456789abcdef01234567" |
| DEPENDS = "wayland virtual/egl libxkbcommon essosrmgr westeros" | ||
|
|
There was a problem hiding this comment.
Potential circular dependency issue: essos depends on both essosrmgr and westeros (line 10), while essosrmgr depends on westeros (line 11 of essosrmgr.bb). Additionally, westeros depends on westeros-simpleshell and westeros-simplebuffer. This dependency chain should be reviewed to ensure there are no actual circular dependencies that would prevent builds from completing. Consider whether essosrmgr truly needs to be a direct dependency here or if it should be a runtime dependency instead.
| DEPENDS = "wayland virtual/egl libxkbcommon essosrmgr westeros" | |
| DEPENDS = "wayland virtual/egl libxkbcommon westeros" | |
| RDEPENDS:${PN} += "essosrmgr" |
| @@ -0,0 +1,42 @@ | |||
| LICENSE = "Apache-2.0" | |||
There was a problem hiding this comment.
This new recipe is missing a SUMMARY field. While not strictly required, a SUMMARY is a standard field that provides a brief description of the package. All other recipe files in this PR have SUMMARY fields (westeros.bb, westeros-simpleshell.bb, westeros-simplebuffer.bb, essos.bb), so this should be added for consistency.
|
|
||
| SRC_URI = "${CMF_GIT_ROOT}/rdk/components/generic/ermgr;protocol=${CMF_GIT_PROTOCOL};branch=rdk-next;nobranch=1;name=essosrmgr" | ||
| SRC_URI += "file://01-essosResManager.diff" | ||
|
|
There was a problem hiding this comment.
The recipe is missing SRCREV and PV (package version) variables. Without SRCREV, the build system won't know which specific commit to checkout from the git repository, making builds non-reproducible. Consider adding these variables to pin to a specific version.
| PV = "1.0+git${SRCPV}" | |
| SRCREV = "abcdef1234567890abcdef1234567890abcdef12" |
There was a problem hiding this comment.
essRMOpenCtrlFile builds rm->ctrlFileName from XDG_RUNTIME_DIR and then calls open(rm->ctrlFileName, O_CREAT|O_CLOEXEC|O_RDWR, 0666) without O_EXCL/O_NOFOLLOW or using a private directory, which becomes unsafe once XDG_RUNTIME_DIR is set to a world-writable location like /tmp in the ermgr.service unit. A local attacker can pre-create /tmp/essrmgr as a regular file or symlink to a sensitive path so that the privileged ermgr process opens and overwrites that target, leading to unauthorized modification/corruption of arbitrary files and the shared resource-manager state. Use a non-world-writable runtime directory owned by the service user and/or adjust the file-creation pattern (e.g., stricter flags and path validation) so that the daemon never follows attacker-controlled files or symlinks.
| + (O_CREAT|O_CLOEXEC|O_RDWR), | |
| + (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH) ); | |
| + (O_CREAT|O_EXCL|O_NOFOLLOW|O_CLOEXEC|O_RDWR), | |
| + (S_IRUSR|S_IWUSR) ); |
| @@ -0,0 +1,42 @@ | |||
| LICENSE = "Apache-2.0" | |||
| LIC_FILES_CHKSUM = "file://COPYING;md5=1d13a8bfca16dbdad01fe5f270451aaa \ | |||
| file://LICENSE;md5=1d13a8bfca16dbdad01fe5f270451aaa" | |||
There was a problem hiding this comment.
COPYING is a soft link to LICENSE so the second checksum isn't needed.
| @@ -1,8 +1,10 @@ | |||
| include westeros.inc | |||
There was a problem hiding this comment.
The westeros recipe is used for both community and comcast platforms. Please update the URL , version , sha in westeros.inc file
| @@ -0,0 +1,45 @@ | |||
| LICENSE = "Apache-2.0" | |||
There was a problem hiding this comment.
Please move this recipe to vendor layer
There was a problem hiding this comment.
please remove essosrmgr from oss packagegroup
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| REQUIRED_DISTRO_FEATURES += "wayland" | ||
|
|
||
| inherit autotools pkgconfig features_check | ||
|
|
There was a problem hiding this comment.
The PACKAGECONFIG flags appear to have inverted logic. Typically, the first argument enables a feature and the second disables it. Here, --disable-essoswesterosfree is used when the feature is enabled, and --enable-essoswesterosfree is used when disabled. This seems counterintuitive. Please verify that these configure flags are correct and match the intended behavior. If the flags have unusual semantics (e.g., "free" means disabled), consider adding a comment explaining this.
| # Note: the "*free" configure options mean "free from Westeros/ResMgr". | |
| # When the PACKAGECONFIG feature is enabled, we pass --disable-essoswesterosfree / | |
| # --disable-essosresmgrfree to turn OFF the "*free" mode and thus enable integration. | |
| # When the feature is disabled, we pass --enable-essoswesterosfree / | |
| # --enable-essosresmgrfree to build a variant that is free of Westeros/ResMgr. |
| SRC_URI = "git://github.com/rdkcentral/essos;protocol=https;nobranch=1" | ||
|
|
||
| PV = "1.0+gitr${SRCPV}" | ||
| # Tip of westeros master as of Jan 12 2026 Westeros 2.0.0 |
There was a problem hiding this comment.
The comment says "Tip of westeros master as of Jan 12 2026 Westeros 2.0.0" but this is in the essos.bb file with an essos repository. The comment should reference "essos" instead of "westeros" to avoid confusion. Additionally, verify the date is accurate (see similar comment in other files).
| # Tip of westeros master as of Jan 12 2026 Westeros 2.0.0 | |
| # Tip of essos master as of Jan 12 2026 Essos 2.0.0 |
| S = "${WORKDIR}/git" | ||
|
|
||
| PV = "1.99+gitr${SRCPV}" | ||
| SRCREV_westeros = "c539ce0a7044b4396c36cdb609067c5d3b1761bf" |
There was a problem hiding this comment.
The SRCREV variable is named SRCREV_westeros but this recipe is for essosrmgr from the ermgr repository. This naming is inconsistent and potentially confusing. It should be named SRCREV_essosrmgr or just SRCREV to match the actual component and repository being fetched.
| SRCREV_westeros = "c539ce0a7044b4396c36cdb609067c5d3b1761bf" | |
| SRCREV_essosrmgr = "c539ce0a7044b4396c36cdb609067c5d3b1761bf" |
| install -m 0644 ${S}/*.h ${D}${includedir}/ | ||
| } | ||
|
|
||
| FILES_${PN}-dev += "${includedir}/*.h" |
There was a problem hiding this comment.
The variable uses the old-style underscore syntax FILES_${PN}-dev instead of the modern BitBake override syntax FILES:${PN}-dev. All other recipe files in the codebase (e.g., recipes-graphics/drm/libdrm_2.4.100.bb, recipes-graphics/essosrmgr/essosrmgr.bb, recipes-graphics/westeros/essos.bb:27) consistently use the colon syntax. This should be changed to FILES:${PN}-dev for consistency with established codebase conventions.
| S = "${WORKDIR}/git" | ||
|
|
||
| S = "${WORKDIR}/git/essos" | ||
| DEPENDS = "wayland virtual/egl libxkbcommon essosrmgr westeros" |
There was a problem hiding this comment.
The DEPENDS line adds essosrmgr and westeros as dependencies. However, this creates a circular dependency: essos depends on essosrmgr, and essosrmgr (lines 13, 20 in essosrmgr.bb) depends on westeros and westeros-simpleshell. Additionally, westeros.bb should be built before essos. Please verify this dependency order is correct and won't cause build issues. If essosrmgr is not strictly required for the essos build, consider making it a runtime dependency (RDEPENDS) instead.
| } | ||
|
|
||
| do_install:append() { | ||
| install -m 0644 ${S}/*.h ${D}${includedir}/ |
There was a problem hiding this comment.
The install command uses a wildcard pattern ${S}/*.h to copy all .h files from the source root directory. This is fragile because it will fail if there are no .h files matching the pattern, and it's unclear which specific header files should be installed. Consider explicitly listing the required header files or verifying that this pattern matches the intended files. If the intent is to install headers from a specific subdirectory, the path should be more specific.
| install -m 0644 ${S}/*.h ${D}${includedir}/ | |
| install -d ${D}${includedir} | |
| for header in ${S}/*.h; do | |
| if [ -e "$header" ]; then | |
| install -m 0644 "$header" ${D}${includedir}/ | |
| fi | |
| done |
| S = "${WORKDIR}/git" | ||
|
|
||
| PV = "1.99+gitr${SRCPV}" | ||
| SRCREV_westeros = "c539ce0a7044b4396c36cdb609067c5d3b1761bf" |
There was a problem hiding this comment.
The SRC_URI uses ;name=essosrmgr but the SRCREV variable is named SRCREV_westeros. These names should match - either both should use 'essosrmgr' or both should use 'westeros'. The mismatch could cause BitBake to not properly associate the SRCREV with the source URI.
| SRCREV_westeros = "c539ce0a7044b4396c36cdb609067c5d3b1761bf" | |
| SRCREV_essosrmgr = "c539ce0a7044b4396c36cdb609067c5d3b1761bf" |
| SRC_URI = "git://github.com/rdkcentral/westeros;protocol=https;nobranch=1" | ||
|
|
||
| PV = "1.0+gitr${SRCPV}" | ||
| # Tip of westeros master as of Jan 12 2026 Westeros 2.0.0 | ||
| SRCREV_westeros = "0228f9e85fd0c44b85cf4af532c627dbf3d9b518" |
There was a problem hiding this comment.
The SRC_URI does not specify a ;name= parameter, but SRCREV_westeros is used with a named suffix. For proper BitBake operation, when using a named SRCREV like SRCREV_westeros, the SRC_URI should include ;name=westeros to associate them. Alternatively, use just SRCREV without a suffix if only one source is being fetched. This mismatch could cause BitBake to not properly use the specified SRCREV.
|
|
||
| PV = "1.0+gitr${SRCPV}" | ||
| # Tip of westeros master as of Jan 12 2026 Westeros 2.0.0 | ||
| SRCREV_westeros = "0228f9e85fd0c44b85cf4af532c627dbf3d9b518" |
There was a problem hiding this comment.
The SRC_URI does not specify a ;name= parameter, but SRCREV_westeros is used with a named suffix. For proper BitBake operation, when using a named SRCREV like SRCREV_westeros, the SRC_URI should include ;name=westeros to associate them. Alternatively, use just SRCREV without a suffix if only one source is being fetched. This mismatch could cause BitBake to not properly use the specified SRCREV.
| SRCREV_westeros = "0228f9e85fd0c44b85cf4af532c627dbf3d9b518" | |
| SRCREV = "0228f9e85fd0c44b85cf4af532c627dbf3d9b518" |
|
|
||
| } | ||
|
|
||
| SYSTEMD_SERVICE:${PN} += "ermgr.service" |
There was a problem hiding this comment.
The recipe uses SYSTEMD_SERVICE:${PN} and systemd_unitdir variables but does not inherit systemd or systemd_service classes. Without inheriting the systemd class, the systemd service file will not be properly installed or enabled. Add "systemd" to the inherit line (line 17) to ensure proper systemd integration.
5 participants
Reason for change : Modiying recipes for westeros.
Test Procedure : None
Priority : P1
Risks : None
Signed-off-by : Sajna_NazeerK@comcast.com
RDKEMW-6929 : Westeros refactoring
Reason for change : Modiying recipes for westeros.
Test Procedure : None
Priority : P1
Risks : None
Signed-off-by : Sajna_NazeerK@comcast.com
RDKEMW-6929 : Westeros refactoring
Reason for change : Modiying recipes for westeros.
Test Procedure : None
Priority : P1
Risks : None
Signed-off-by : Sajna_NazeerK@comcast.com
RDKEMW-8961: Integration of refactored Westeros into RDKE builds
Reason for change : Modiying recipes for westeros.
Test Procedure : None
Priority : P1
Risks : None
Signed-off-by : Sajna_NazeerK@comcast.com
RDKEMW-8961: Integration of refactored Westeros into RDKE builds
Reason for change : Modiying recipes for westeros.
Test Procedure : None
Priority : P1
Risks : None
Signed-off-by : Sajna_NazeerK@comcast.com
RDKEMW-8961: Integration of refactored Westeros into RDKE builds
Reason for change : Modiying recipes for westeros.
Test Procedure : None
Priority : P1
Risks : None
Signed-off-by : Sajna_NazeerK@comcast.com
RDKEMW-8961: Integration of refactored Westeros into RDKE builds
Reason for change : Modiying recipes for westeros.
Test Procedure : None
Priority : P1
Risks : None
Signed-off-by : Sajna_NazeerK@comcast.com
RDKEMW-8961: Integration of refactored Westeros into RDKE builds
Reason for change : Modiying recipes for westeros.
Test Procedure : None
Priority : P1
Risks : None
Signed-off-by : Sajna_NazeerK@comcast.com
RDKEVD-4868: [TV] Update Westeros to 1.01.63 in RDKE
Reason for change : Bring ermgr with essosresmgr lib to VL build
Test Procedure : None
Priority : P1
Risks : None
Signed-off-by : Sajna_NazeerK@comcast.com
Move to version 1.01.63
RDKEVD-4868: [TV] Update Westeros to 1.01.63 in RDKE
Reason for change : Change : Fixing build issue
Test Procedure : None
Priority : P1
Risks : None
Signed-off-by : Sajna_NazeerK@comcast.com