[PATCH 0/3] postinsts fixes

[PATCH 0/3] postinsts fixes

[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

[PATCH 1/3] opkg: avoid running postinst scripts twice when using systemd

[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

[PATCH 2/3] run-postinsts: Do not rely on /etc/*-postinsts

[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

[PATCH 3/3] rootfs.py: Don't install postinsts if package management is present

[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

Re: [PATCH 0/3] postinsts fixes

[Alexander Kanavin]

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

Re: [PATCH 0/3] postinsts fixes

[Alexander Kanavin]

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

Re: [PATCH 0/3] postinsts fixes

[Stefan Agner]

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

Re: [PATCH 1/3] opkg: avoid running postinst scripts twice when using systemd

[Christopher Larson]

[-- 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 --]