[PATCH v2 0/3] postinst fixes for opgk/dpkg

[PATCH v2 0/3] postinst fixes for opgk/dpkg

[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

Changes since v1:
- Note that patches are specific for opkg/dpkg

Stefan Agner (3):
  opkg: avoid running postinst scripts twice when using systemd
  run-postinsts: for dpkg/opkg, do not rely on /etc/*-postinsts
  rootfs.py: for dpkg/opkg, 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 v2 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 v2 2/3] run-postinsts: for dpkg/opkg, 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.

The behavior when using rpm packages stays the same.

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 v2 3/3] rootfs.py: for dpkg/opkg, 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 v2 0/3] postinst fixes for opgk/dpkg

[Stefan Agner]

On 16.05.2018 11:13, Stefan Agner wrote:
> 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

Anything holding us back merging this changes?

--
Stefan

> 
> Changes since v1:
> - Note that patches are specific for opkg/dpkg
> 
> Stefan Agner (3):
>   opkg: avoid running postinst scripts twice when using systemd
>   run-postinsts: for dpkg/opkg, do not rely on /etc/*-postinsts
>   rootfs.py: for dpkg/opkg, 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

Re: [PATCH v2 0/3] postinst fixes for opgk/dpkg

[Alexander Kanavin]

On 06/04/2018 07:06 PM, Stefan Agner wrote:

> Anything holding us back merging this changes?

Please read the Yocto project status mail:

" ·         Patch review/merging was slow over the past week due to Ross 
being on vacation and changes in Richard’s employment/hardware situation 
but at least some of the backlog was resolved over the weekend and the 
hardware issues should be worked around now."

Alex

Re: [PATCH v2 0/3] postinst fixes for opgk/dpkg

[Stefan Agner]

On 04.06.2018 18:18, Alexander Kanavin wrote:
> On 06/04/2018 07:06 PM, Stefan Agner wrote:
> 
>> Anything holding us back merging this changes?
> 
> Please read the Yocto project status mail:
> 
> " ·         Patch review/merging was slow over the past week due to
> Ross being on vacation and changes in Richard’s employment/hardware
> situation but at least some of the backlog was resolved over the
> weekend and the hardware issues should be worked around now."
> 

Gentle ping on that again.

This week it read:
"Due to the milestone, patch merging is slowing to stabilize."

Also, there have been patches merged which were posted after this
patchset... It is really rather important for us since it is hard to fix
in lower layers...

So I am getting nervous whether it still makes it?

--
Stefan

Re: [PATCH v2 0/3] postinst fixes for opgk/dpkg

[Alexander Kanavin]

No need to be nervous. The patch will make it in; patches do not get
quietly rejected. If it hasn't showed up yet, it's most likely because
the patch will be tested on the autobuilder in a separate set from the
'regular patches', or (yes, this happens) it has slipped through the
cracks. In that case (I know it's impossible to tell from the
outside), just resend again.

Also, please do help us out with oe-core patch review and recipe
maintenance, particularly updating them to latest versions.


Alex

2018-06-14 16:57 GMT+03:00 Stefan Agner <stefan@agner.ch>:
> On 04.06.2018 18:18, Alexander Kanavin wrote:
>> On 06/04/2018 07:06 PM, Stefan Agner wrote:
>>
>>> Anything holding us back merging this changes?
>>
>> Please read the Yocto project status mail:
>>
>> " ·         Patch review/merging was slow over the past week due to
>> Ross being on vacation and changes in Richard’s employment/hardware
>> situation but at least some of the backlog was resolved over the
>> weekend and the hardware issues should be worked around now."
>>
>
> Gentle ping on that again.
>
> This week it read:
> "Due to the milestone, patch merging is slowing to stabilize."
>
> Also, there have been patches merged which were posted after this
> patchset... It is really rather important for us since it is hard to fix
> in lower layers...
>
> So I am getting nervous whether it still makes it?
>
> --
> Stefan
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core