RDKOSS-134: Build systemd V250#397
Conversation
Reason for the change: Configured systemd PREFERRED version to 1:250 Backport tar_1.17 recipe which is gplv2 licenced required for building systemd v250 Backport missing systemd v250 changes for RDKE Remove reference to default-time-setter.sh script in systemd-timesyncd to fix the build error Signed-off-by: mselva006c <mani_selvaraj@comcast.com>
There was a problem hiding this comment.
Pull request overview
Updates the OSS Yocto layer to build against systemd v250 by pinning the preferred systemd version, backporting a GPLv2 tar (needed under the layer’s GPLv3 restrictions), and adjusting the systemd v250 bbappend to resolve build/runtime integration issues.
Changes:
- Added a
tar_1.17GPL-2.0-only recipe (with backported patches) to replace GPLv3 tar underINCOMPATIBLE_LICENSE. - Pinned
PREFERRED_VERSION_systemdto1:250%and added OSS arch/revision entries fortar. - Updated
systemd_250.%.bbappend(incl. timesyncd/unit edits, conditional patch inclusion, dependency tweaks, and additional install-time config).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| recipes-extended/tar/tar_1.17.bb | Introduces tar 1.17 recipe and applies local patches. |
| recipes-extended/tar/tar/0001-lib-argp-parse.c-Fix-compiler-error.patch | Adds a compiler warning/error fix patch for old tar sources. |
| recipes-extended/tar/tar.inc | Common tar recipe include (autotools install, alternatives, packaging). |
| recipes-extended/tar/tar-1.17/m4extensions.patch | Autoconf compatibility patch for older tar. |
| recipes-extended/tar/tar-1.17/gcc43build.patch | GCC 4.3 inline-related build fix for old tar. |
| recipes-extended/tar/tar-1.17/avoid_heap_overflow.patch | CVE-2010-0624 mitigation for tar 1.17. |
| recipes-core/systemd/systemd_250.%.bbappend | Backports/adjusts systemd v250 integration and config during install. |
| conf/include/package_revisions_oss.inc | Pins systemd v250 and adds tar PR/arch configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| PR = "r3" | ||
|
|
There was a problem hiding this comment.
The recipe sets PR = "r3", but this layer appears to centralize package revisions via conf/include/package_revisions_oss.inc (e.g., it already adds PR:pn-tar ?= "r0"). Keeping PR in the recipe makes the conf-level PR ineffective and breaks the convention used elsewhere in this layer. Consider removing PR from the recipe and managing the revision solely through PR:pn-tar in package_revisions_oss.inc (or change that entry to an unconditional assignment if you truly need to override).
| PR = "r3" |
| #Configure NTP server (time.google.com) in timesyncd.conf | ||
| if [ -n "${@bb.utils.contains('PACKAGECONFIG', 'timesyncd', 'timesyncd', '', d)}" ]; then | ||
| sed -i -e 's/^#NTP=.*/NTP=ntp.ccp.xcal.tv/g' ${D}${sysconfdir}/systemd/timesyncd.conf | ||
| sed -i -e 's/^#NTP=.*/NTP=time.google.com/g' ${D}${sysconfdir}/systemd/timesyncd.conf | ||
| #Patch for CISCOXI4-2785: remove ProtectSystem=full from systemd-timesyncd.service |
There was a problem hiding this comment.
This changes the default NTP server configured in timesyncd.conf from the previous Comcast NTP hostname to time.google.com. The PR description mentions removing default-time-setter.sh, but doesn’t mention this behavior change; please either document this change in the PR description/rationale or keep the previous NTP setting to avoid unintended device behavior changes.
| do_install:append() { | ||
| if ${@bb.utils.contains('DISTRO_FEATURES', 'benchmark_enable', 'false', 'true', d)}; then | ||
| sed -i -e 's/Options=/Options=nosuid,nodev,noexec,/' ${D}${systemd_unitdir}/system/tmp.mount | ||
| fi | ||
|
|
||
| if ${@bb.utils.contains('DISTRO_FEATURES', 'syslog-ng', 'false', 'true', d)}; then | ||
| #Journal conf overide | ||
| sed -i -e 's/.*ForwardToSyslog=.*/#ForwardToSyslog=no/g' ${D}${sysconfdir}/systemd/journald.conf | ||
| sed -i -e 's/.*SystemMaxUse=.*/SystemMaxUse=8M/g' ${D}${sysconfdir}/systemd/journald.conf | ||
| sed -i -e 's/.*RuntimeMaxUse=.*/RuntimeMaxUse=8M/g' ${D}${sysconfdir}/systemd/journald.conf | ||
| sed -i -e 's/.*RuntimeMaxFileSize=.*/RuntimeMaxFileSize=4M/g' ${D}${sysconfdir}/systemd/journald.conf | ||
| sed -i -e 's/.*SystemMaxFileSize=.*/SystemMaxFileSize=4M/g' ${D}${sysconfdir}/systemd/journald.conf |
There was a problem hiding this comment.
The newly added do_install:append() block here modifies journald.conf and tmp.mount, but this file already has earlier do_install:append() logic that also edits journald.conf (e.g. sets ForwardToSyslog=yes, Storage=volatile, size limits). Having multiple appends making conflicting edits makes the final journald configuration order-dependent and hard to reason about. Please consolidate journald/tmp.mount modifications into a single place (or guard them with appropriate overrides) so the resulting configuration is deterministic and maintainable.
Reason for the change:
Configured systemd PREFERRED version to 1:250
Backport tar_1.17 recipe which is gplv2 licenced required for building systemd v250 Backport missing systemd v250 changes for RDKE
Remove reference to default-time-setter.sh script in systemd-timesyncd to fix the build error