* [PATCH 0/3] postinsts fixes
@ 2018-05-15 14:04 Stefan Agner
2018-05-15 14:04 ` [PATCH 1/3] opkg: avoid running postinst scripts twice when using systemd Stefan Agner
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Stefan Agner @ 2018-05-15 14:04 UTC (permalink / raw)
To: openembedded-core, alexander.kanavin; +Cc: muhammad_shakeel, Stefan Agner
From: Stefan Agner <stefan.agner@toradex.com>
Hi,
This follows up on the discussion a while ago:
https://www.mail-archive.com/openembedded-core@lists.openembedded.org/msg104996.html
Patch 1 is rather simple and really fixes the main issue. The patch
by itself has been tested with the relevant self test and passes.
Patch 2/3 get rid of /etc/*-postinsts script in cases where the package
management is present. This avoids a bunch of unnecessary scripts to be
present on the rootfs. It also avoids the systemd service to be present
forever with in case no postinst scripts have been deployed:
Condition: start condition failed at Tue 2018-05-15 10:57:43 UTC; 42s ago
└─ ConditionPathExistsGlob=/etc/*-postinsts was not met
Self test executed using:
$ oe-selftest --run-tests runtime_test.Postinst.test_postinst_rootfs_and_boot
Stefan Agner (3):
opkg: avoid running postinst scripts twice when using systemd
run-postinsts: Do not rely on /etc/*-postinsts
rootfs.py: Don't install postinsts if package management is present
meta/lib/oe/rootfs.py | 3 +++
.../opkg/opkg/opkg-configure.service | 17 -----------------
meta/recipes-devtools/opkg/opkg_0.3.6.bb | 14 --------------
.../run-postinsts/run-postinsts/run-postinsts | 21 ++++++++++++---------
.../run-postinsts/run-postinsts.service | 1 -
5 files changed, 15 insertions(+), 41 deletions(-)
delete mode 100644 meta/recipes-devtools/opkg/opkg/opkg-configure.service
--
2.13.6
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] opkg: avoid running postinst scripts twice when using systemd 2018-05-15 14:04 [PATCH 0/3] postinsts fixes Stefan Agner @ 2018-05-15 14:04 ` Stefan Agner 2018-06-05 22:17 ` Christopher Larson 2018-05-15 14:04 ` [PATCH 2/3] run-postinsts: Do not rely on /etc/*-postinsts Stefan Agner ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Stefan Agner @ 2018-05-15 14:04 UTC (permalink / raw) To: openembedded-core, alexander.kanavin; +Cc: muhammad_shakeel, Stefan Agner From: Stefan Agner <stefan.agner@toradex.com> OpenEmbedded has a built-in mechanism to run postinst scripts offline at build time or, if necessary, on first boot (delayed execution). If the latter is the case and systemd is in use, two services end up doing the same thing: - opkg-configure.service starts "opkg configure" directly. - run-postinsts.service starts "/usr/sbin/run-postinsts" which runs postinst scripts stored in /etc/ipk-postinsts/ or "opkg configure" if package management is installed. Since the run-postinsts.service is also used in cases where no package management is in use, it is the primary means of handling postinsts. Get rid of the opkg-configure.service to avoid duplicate opkg configure execution. Signed-off-by: Stefan Agner <stefan.agner@toradex.com> --- meta/recipes-devtools/opkg/opkg/opkg-configure.service | 17 ----------------- meta/recipes-devtools/opkg/opkg_0.3.6.bb | 14 -------------- 2 files changed, 31 deletions(-) delete mode 100644 meta/recipes-devtools/opkg/opkg/opkg-configure.service diff --git a/meta/recipes-devtools/opkg/opkg/opkg-configure.service b/meta/recipes-devtools/opkg/opkg/opkg-configure.service deleted file mode 100644 index 432c3ddc28..0000000000 --- a/meta/recipes-devtools/opkg/opkg/opkg-configure.service +++ /dev/null @@ -1,17 +0,0 @@ -[Unit] -Description=Opkg first boot configure -DefaultDependencies=no -After=systemd-remount-fs.service systemd-tmpfiles-setup.service tmp.mount -Before=sysinit.target - -[Service] -Type=oneshot -EnvironmentFile=-@SYSCONFDIR@/default/postinst -ExecStart=-@BASE_BINDIR@/sh -c " if [ $POSTINST_LOGGING = '1' ]; then @BINDIR@/opkg configure > $LOGFILE 2>&1; else @BINDIR@/opkg configure; fi" -ExecStartPost=@BASE_BINDIR@/systemctl --no-reload disable opkg-configure.service -StandardOutput=syslog -RemainAfterExit=No - -[Install] -WantedBy=basic.target -WantedBy=sysinit.target diff --git a/meta/recipes-devtools/opkg/opkg_0.3.6.bb b/meta/recipes-devtools/opkg/opkg_0.3.6.bb index 70f20af739..579b51166c 100644 --- a/meta/recipes-devtools/opkg/opkg_0.3.6.bb +++ b/meta/recipes-devtools/opkg/opkg_0.3.6.bb @@ -12,7 +12,6 @@ DEPENDS = "libarchive" PE = "1" SRC_URI = "http://downloads.yoctoproject.org/releases/${BPN}/${BPN}-${PV}.tar.gz \ - file://opkg-configure.service \ file://opkg.conf \ file://0001-opkg_conf-create-opkg.lock-in-run-instead-of-var-run.patch \ " @@ -22,8 +21,6 @@ SRC_URI[sha256sum] = "f607f0e61be8cf8a3bbd0d2dccd9ec9e9b6c21dd4307b671c600d6eeaf inherit autotools pkgconfig systemd -SYSTEMD_SERVICE_${PN} = "opkg-configure.service" - target_localstatedir := "${localstatedir}" OPKGLIBDIR = "${target_localstatedir}/lib" @@ -46,16 +43,6 @@ do_install_append () { # We need to create the lock directory install -d ${D}${OPKGLIBDIR}/opkg - - if ${@bb.utils.contains('DISTRO_FEATURES','systemd','true','false',d)};then - install -d ${D}${systemd_unitdir}/system - install -m 0644 ${WORKDIR}/opkg-configure.service ${D}${systemd_unitdir}/system/ - sed -i -e 's,@BASE_BINDIR@,${base_bindir},g' \ - -e 's,@SYSCONFDIR@,${sysconfdir},g' \ - -e 's,@BINDIR@,${bindir},g' \ - -e 's,@SYSTEMD_UNITDIR@,${systemd_unitdir},g' \ - ${D}${systemd_unitdir}/system/opkg-configure.service - fi } RDEPENDS_${PN} = "${VIRTUAL-RUNTIME_update-alternatives} opkg-arch-config libarchive" @@ -68,7 +55,6 @@ RPROVIDES_${PN} = "opkg-collateral" PACKAGES =+ "libopkg" FILES_libopkg = "${libdir}/*.so.* ${OPKGLIBDIR}/opkg/" -FILES_${PN} += "${systemd_unitdir}/system/" BBCLASSEXTEND = "native nativesdk" -- 2.13.6 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] opkg: avoid running postinst scripts twice when using systemd 2018-05-15 14:04 ` [PATCH 1/3] opkg: avoid running postinst scripts twice when using systemd Stefan Agner @ 2018-06-05 22:17 ` Christopher Larson 0 siblings, 0 replies; 8+ messages in thread From: Christopher Larson @ 2018-06-05 22:17 UTC (permalink / raw) To: stefan Cc: Shakeel, Muhammad, stefan.agner, Patches and discussions about the oe-core layer [-- Attachment #1: Type: text/plain, Size: 4511 bytes --] Would love to see this merged, this is a long-standing issue. On Tue, May 15, 2018 at 7:05 AM Stefan Agner <stefan@agner.ch> wrote: > From: Stefan Agner <stefan.agner@toradex.com> > > OpenEmbedded has a built-in mechanism to run postinst scripts offline > at build time or, if necessary, on first boot (delayed execution). If > the latter is the case and systemd is in use, two services end up > doing the same thing: > - opkg-configure.service starts "opkg configure" directly. > - run-postinsts.service starts "/usr/sbin/run-postinsts" which runs > postinst scripts stored in /etc/ipk-postinsts/ or "opkg configure" > if package management is installed. > > Since the run-postinsts.service is also used in cases where no > package management is in use, it is the primary means of handling > postinsts. > > Get rid of the opkg-configure.service to avoid duplicate opkg > configure execution. > > Signed-off-by: Stefan Agner <stefan.agner@toradex.com> > --- > meta/recipes-devtools/opkg/opkg/opkg-configure.service | 17 > ----------------- > meta/recipes-devtools/opkg/opkg_0.3.6.bb | 14 > -------------- > 2 files changed, 31 deletions(-) > delete mode 100644 meta/recipes-devtools/opkg/opkg/opkg-configure.service > > diff --git a/meta/recipes-devtools/opkg/opkg/opkg-configure.service > b/meta/recipes-devtools/opkg/opkg/opkg-configure.service > deleted file mode 100644 > index 432c3ddc28..0000000000 > --- a/meta/recipes-devtools/opkg/opkg/opkg-configure.service > +++ /dev/null > @@ -1,17 +0,0 @@ > -[Unit] > -Description=Opkg first boot configure > -DefaultDependencies=no > -After=systemd-remount-fs.service systemd-tmpfiles-setup.service tmp.mount > -Before=sysinit.target > - > -[Service] > -Type=oneshot > -EnvironmentFile=-@SYSCONFDIR@/default/postinst > -ExecStart=-@BASE_BINDIR@/sh -c " if [ $POSTINST_LOGGING = '1' ]; then > @BINDIR@/opkg configure > $LOGFILE 2>&1; else @BINDIR@/opkg configure; fi" > -ExecStartPost=@BASE_BINDIR@/systemctl --no-reload disable > opkg-configure.service > -StandardOutput=syslog > -RemainAfterExit=No > - > -[Install] > -WantedBy=basic.target > -WantedBy=sysinit.target > diff --git a/meta/recipes-devtools/opkg/opkg_0.3.6.bb > b/meta/recipes-devtools/opkg/opkg_0.3.6.bb > index 70f20af739..579b51166c 100644 > --- a/meta/recipes-devtools/opkg/opkg_0.3.6.bb > +++ b/meta/recipes-devtools/opkg/opkg_0.3.6.bb > @@ -12,7 +12,6 @@ DEPENDS = "libarchive" > PE = "1" > > SRC_URI = " > http://downloads.yoctoproject.org/releases/${BPN}/${BPN}-${PV}.tar.gz \ > - file://opkg-configure.service \ > file://opkg.conf \ > > file://0001-opkg_conf-create-opkg.lock-in-run-instead-of-var-run.patch \ > " > @@ -22,8 +21,6 @@ SRC_URI[sha256sum] = > "f607f0e61be8cf8a3bbd0d2dccd9ec9e9b6c21dd4307b671c600d6eeaf > > inherit autotools pkgconfig systemd > > -SYSTEMD_SERVICE_${PN} = "opkg-configure.service" > - > target_localstatedir := "${localstatedir}" > OPKGLIBDIR = "${target_localstatedir}/lib" > > @@ -46,16 +43,6 @@ do_install_append () { > > # We need to create the lock directory > install -d ${D}${OPKGLIBDIR}/opkg > - > - if > ${@bb.utils.contains('DISTRO_FEATURES','systemd','true','false',d)};then > - install -d ${D}${systemd_unitdir}/system > - install -m 0644 ${WORKDIR}/opkg-configure.service > ${D}${systemd_unitdir}/system/ > - sed -i -e 's,@BASE_BINDIR@,${base_bindir},g' \ > - -e 's,@SYSCONFDIR@,${sysconfdir},g' \ > - -e 's,@BINDIR@,${bindir},g' \ > - -e 's,@SYSTEMD_UNITDIR@,${systemd_unitdir},g' \ > - > ${D}${systemd_unitdir}/system/opkg-configure.service > - fi > } > > RDEPENDS_${PN} = "${VIRTUAL-RUNTIME_update-alternatives} opkg-arch-config > libarchive" > @@ -68,7 +55,6 @@ RPROVIDES_${PN} = "opkg-collateral" > PACKAGES =+ "libopkg" > > FILES_libopkg = "${libdir}/*.so.* ${OPKGLIBDIR}/opkg/" > -FILES_${PN} += "${systemd_unitdir}/system/" > > BBCLASSEXTEND = "native nativesdk" > > -- > 2.13.6 > > -- > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-core > -- Christopher Larson kergoth at gmail dot com Founder - BitBake, OpenEmbedded, OpenZaurus Senior Software Engineer, Mentor Graphics [-- Attachment #2: Type: text/html, Size: 6285 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] run-postinsts: Do not rely on /etc/*-postinsts 2018-05-15 14:04 [PATCH 0/3] postinsts fixes Stefan Agner 2018-05-15 14:04 ` [PATCH 1/3] opkg: avoid running postinst scripts twice when using systemd Stefan Agner @ 2018-05-15 14:04 ` Stefan Agner 2018-05-15 14:04 ` [PATCH 3/3] rootfs.py: Don't install postinsts if package management is present Stefan Agner 2018-05-15 15:03 ` [PATCH 0/3] postinsts fixes Alexander Kanavin 3 siblings, 0 replies; 8+ messages in thread From: Stefan Agner @ 2018-05-15 14:04 UTC (permalink / raw) To: openembedded-core, alexander.kanavin; +Cc: muhammad_shakeel, Stefan Agner From: Stefan Agner <stefan.agner@toradex.com> Start opkg/dpkg as soon as the respective package managers status file is present, no matter whether /etc/$pm-postinsts exists. This decouples the implicit link between postinsts scripts in /etc and the package manager: Currently the package manager is only started if those scripts are present, although the package manager does not use those scripts at all! Package managers install their own set of postinst scripts. Note that using the package managers capability to execute postinst scripts is preferred for good reasons: It makes sure that the package managers database reflects that the packages have been completely installed and configured. This change allows to drop installation of the postinsts scripts when package management is present. This will be done in a separate change. Note: Before commit 5aae19959a44 ("rootfs.py: Change logic to unistall packages") rootfs.py did not install /etc/$pm-postinsts when package management is installed! The change caused YOCTO #8235 which lead to the behavior change of run-postinsts in first place. Signed-off-by: Stefan Agner <stefan.agner@toradex.com> --- .../run-postinsts/run-postinsts/run-postinsts | 21 ++++++++++++--------- .../run-postinsts/run-postinsts.service | 1 - 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/meta/recipes-devtools/run-postinsts/run-postinsts/run-postinsts b/meta/recipes-devtools/run-postinsts/run-postinsts/run-postinsts index 307feb7187..95eff04e17 100755 --- a/meta/recipes-devtools/run-postinsts/run-postinsts/run-postinsts +++ b/meta/recipes-devtools/run-postinsts/run-postinsts/run-postinsts @@ -6,7 +6,8 @@ # # The following script will run all the scriptlets found in #SYSCONFDIR#/deb-postinsts, -# #SYSCONFDIR#/ipk-postinsts or #SYSCONFDIR#/rpm-postinsts. +# #SYSCONFDIR#/ipk-postinsts or #SYSCONFDIR#/rpm-postinsts or the package manager in +# case available. # the order of this list is important, do not change! backend_list="rpm deb ipk" @@ -14,27 +15,29 @@ backend_list="rpm deb ipk" pm_installed=false for pm in $backend_list; do - pi_dir="#SYSCONFDIR#/$pm-postinsts" - - if [ ! -d $pi_dir ]; then - continue - fi - # found the package manager, it has postinsts case $pm in "deb") if [ -s "#LOCALSTATEDIR#/lib/dpkg/status" ]; then pm_installed=true + break fi ;; "ipk") if [ -s "#LOCALSTATEDIR#/lib/opkg/status" ]; then pm_installed=true + break fi ;; esac - break + + pi_dir="#SYSCONFDIR#/$pm-postinsts" + + # found postinsts directory + if [ -d $pi_dir ]; then + break + fi done remove_rcsd_link () { @@ -43,7 +46,7 @@ remove_rcsd_link () { fi } -if ! [ -d $pi_dir ]; then +if ! [ -d $pi_dir ] && ! $pm_installed; then remove_rcsd_link exit 0 fi diff --git a/meta/recipes-devtools/run-postinsts/run-postinsts/run-postinsts.service b/meta/recipes-devtools/run-postinsts/run-postinsts/run-postinsts.service index 1b71a1f8be..d42addf510 100644 --- a/meta/recipes-devtools/run-postinsts/run-postinsts/run-postinsts.service +++ b/meta/recipes-devtools/run-postinsts/run-postinsts/run-postinsts.service @@ -3,7 +3,6 @@ Description=Run pending postinsts DefaultDependencies=no After=systemd-remount-fs.service systemd-tmpfiles-setup.service tmp.mount Before=sysinit.target -ConditionPathExistsGlob=#SYSCONFDIR#/*-postinsts [Service] Type=oneshot -- 2.13.6 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] rootfs.py: Don't install postinsts if package management is present 2018-05-15 14:04 [PATCH 0/3] postinsts fixes Stefan Agner 2018-05-15 14:04 ` [PATCH 1/3] opkg: avoid running postinst scripts twice when using systemd Stefan Agner 2018-05-15 14:04 ` [PATCH 2/3] run-postinsts: Do not rely on /etc/*-postinsts Stefan Agner @ 2018-05-15 14:04 ` Stefan Agner 2018-05-15 15:03 ` [PATCH 0/3] postinsts fixes Alexander Kanavin 3 siblings, 0 replies; 8+ messages in thread From: Stefan Agner @ 2018-05-15 14:04 UTC (permalink / raw) To: openembedded-core, alexander.kanavin; +Cc: muhammad_shakeel, Stefan Agner From: Stefan Agner <stefan.agner@toradex.com> If package management is present opkg/dpkg will bring the original copy of the postinsts scripts with the metadata and will be able to handle postinsts just fine. In fact, it is preferred to let package management handle the postinsts scripts in this case since it will keep the package managers database up-to-date too. The run-postinsts scripts will make sure the package manager gets invoked instead of the scripts directly. Note: Before commit 5aae19959a44 ("rootfs.py: Change logic to unistall packages") rootfs.py did not install /etc/$pm-postinsts too. It is not clear whether that change was intentionally or just a bug. This commit fixes/reverts that aspect of the commit. Signed-off-by: Stefan Agner <stefan.agner@toradex.com> --- meta/lib/oe/rootfs.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py index f8f717c050..0a57d87f71 100644 --- a/meta/lib/oe/rootfs.py +++ b/meta/lib/oe/rootfs.py @@ -560,6 +560,9 @@ class DpkgOpkgRootfs(Rootfs): return pkg_list def _save_postinsts_common(self, dst_postinst_dir, src_postinst_dir): + if bb.utils.contains("IMAGE_FEATURES", "package-management", + True, False, self.d): + return num = 0 for p in self._get_delayed_postinsts(): bb.utils.mkdirhier(dst_postinst_dir) -- 2.13.6 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] postinsts fixes 2018-05-15 14:04 [PATCH 0/3] postinsts fixes Stefan Agner ` (2 preceding siblings ...) 2018-05-15 14:04 ` [PATCH 3/3] rootfs.py: Don't install postinsts if package management is present Stefan Agner @ 2018-05-15 15:03 ` Alexander Kanavin 2018-05-15 16:07 ` Stefan Agner 3 siblings, 1 reply; 8+ messages in thread From: Alexander Kanavin @ 2018-05-15 15:03 UTC (permalink / raw) To: Stefan Agner, openembedded-core; +Cc: muhammad_shakeel, Stefan Agner On 05/15/2018 05:04 PM, Stefan Agner wrote: > This follows up on the discussion a while ago: > https://www.mail-archive.com/openembedded-core@lists.openembedded.org/msg104996.html Took me a moment to realize all three patches are specific to opkg/dpkg and do not change how rpm (or general 'postinst handling') works. Perhaps you could amend the commit messages to emphasize that? Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] postinsts fixes 2018-05-15 15:03 ` [PATCH 0/3] postinsts fixes Alexander Kanavin @ 2018-05-15 16:07 ` Stefan Agner 2018-05-15 16:07 ` Alexander Kanavin 0 siblings, 1 reply; 8+ messages in thread From: Stefan Agner @ 2018-05-15 16:07 UTC (permalink / raw) To: Alexander Kanavin; +Cc: muhammad_shakeel, Stefan Agner, openembedded-core On 15.05.2018 17:03, Alexander Kanavin wrote: > On 05/15/2018 05:04 PM, Stefan Agner wrote: >> This follows up on the discussion a while ago: >> https://www.mail-archive.com/openembedded-core@lists.openembedded.org/msg104996.html > > Took me a moment to realize all three patches are specific to > opkg/dpkg and do not change how rpm (or general 'postinst handling') > works. Perhaps you could amend the commit messages to emphasize that? Patch 1 has opkg in the subject. Patch 2/3 mention opkg/dpkg in the commit log. Patch 3 also really edits the DpkgOpkgRootfs class, which should make it rather obvious. But I agree for patch 2 it would be good to emphasize it. How about adding this after the first paragraph: The behavior for rpm stays the same. -- Stefan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] postinsts fixes 2018-05-15 16:07 ` Stefan Agner @ 2018-05-15 16:07 ` Alexander Kanavin 0 siblings, 0 replies; 8+ messages in thread From: Alexander Kanavin @ 2018-05-15 16:07 UTC (permalink / raw) To: Stefan Agner; +Cc: muhammad_shakeel, Stefan Agner, openembedded-core On 05/15/2018 07:07 PM, Stefan Agner wrote: > On 15.05.2018 17:03, Alexander Kanavin wrote: >> On 05/15/2018 05:04 PM, Stefan Agner wrote: >>> This follows up on the discussion a while ago: >>> https://www.mail-archive.com/openembedded-core@lists.openembedded.org/msg104996.html >> >> Took me a moment to realize all three patches are specific to >> opkg/dpkg and do not change how rpm (or general 'postinst handling') >> works. Perhaps you could amend the commit messages to emphasize that? > > Patch 1 has opkg in the subject. > > Patch 2/3 mention opkg/dpkg in the commit log. > > Patch 3 also really edits the DpkgOpkgRootfs class, which should make it > rather obvious. > > But I agree for patch 2 it would be good to emphasize it. > > How about adding this after the first paragraph: > The behavior for rpm stays the same. That, and amend the commit summaries: patch 2: run-postinsts: for dpkg/opkg, do not rely on /etc/*-postinsts patch 3: rootfs.py: for dpkg/opkg, don't install postinsts if package management is present Otherwise those one liners can give the wrong impression. Alex ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-06-05 22:17 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-15 14:04 [PATCH 0/3] postinsts fixes Stefan Agner 2018-05-15 14:04 ` [PATCH 1/3] opkg: avoid running postinst scripts twice when using systemd Stefan Agner 2018-06-05 22:17 ` Christopher Larson 2018-05-15 14:04 ` [PATCH 2/3] run-postinsts: Do not rely on /etc/*-postinsts Stefan Agner 2018-05-15 14:04 ` [PATCH 3/3] rootfs.py: Don't install postinsts if package management is present Stefan Agner 2018-05-15 15:03 ` [PATCH 0/3] postinsts fixes Alexander Kanavin 2018-05-15 16:07 ` Stefan Agner 2018-05-15 16:07 ` Alexander Kanavin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.