All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] opkg: avoid running postinst scripts twice when using systemd
@ 2017-12-13 18:06 Stefan Agner
  2017-12-13 18:29 ` Stefan Agner
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Agner @ 2017-12-13 18:06 UTC (permalink / raw)
  To: openembedded-core; +Cc: Stefan Agner, muhammad_shakeel

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" which in turn parses
  /var/lib/opkg/status and runs the postinst scripts stored in
  /var/lib/opkg/info/.
- run-postinsts.service starts "/usr/sbin/run-postinsts" which runs
  postinst scripts stored in /etc/ipk-postinsts/. Those scripts get
  prepared by meta/lib/oe/rootfs.py (see also _save_postinsts of the
  OpkgRootfs class)

This can be seen when using a distro using ipk and systemd, then
installing opkg-keyrings and enable package management. The resulting
rootfs has both services and will run opkg-keyrings.postinst twice.

Moreover, because the two services have similar dependency, they
typically get executed almost at the same time:
  Dec 13 14:47:25 colibri-imx6 systemd[1]: Starting Opkg first boot configure...
  ...
  Dec 13 14:47:25 colibri-imx6 systemd[1]: Starting Run pending postinsts...

This leads to executions of the same postinst scripts racing with each
other! In practise update-alternative has been proven to be suspectable
to such races.

Get rid of the opkg service to avoid dupplicate postinst 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.5.bb               | 13 -------------
 2 files changed, 30 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.5.bb b/meta/recipes-devtools/opkg/opkg_0.3.5.bb
index 3e511b6bb6..fb36e8b866 100644
--- a/meta/recipes-devtools/opkg/opkg_0.3.5.bb
+++ b/meta/recipes-devtools/opkg/opkg_0.3.5.bb
@@ -22,8 +22,6 @@ SRC_URI[sha256sum] = "734bc21dea11262113fa86b928d09812618b3966f352350cf916a6ae0d
 
 inherit autotools pkgconfig systemd
 
-SYSTEMD_SERVICE_${PN} = "opkg-configure.service"
-
 target_localstatedir := "${localstatedir}"
 OPKGLIBDIR = "${target_localstatedir}/lib"
 
@@ -46,16 +44,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 +56,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] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-13 18:06 [RFC] opkg: avoid running postinst scripts twice when using systemd Stefan Agner
@ 2017-12-13 18:29 ` Stefan Agner
  2017-12-14 13:10   ` Alexander Kanavin
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Agner @ 2017-12-13 18:29 UTC (permalink / raw)
  To: openembedded-core; +Cc: muhammad_shakeel, Stefan Agner

On 2017-12-13 19:06, Stefan Agner 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" which in turn parses
>   /var/lib/opkg/status and runs the postinst scripts stored in
>   /var/lib/opkg/info/.
> - run-postinsts.service starts "/usr/sbin/run-postinsts" which runs
>   postinst scripts stored in /etc/ipk-postinsts/. Those scripts get
>   prepared by meta/lib/oe/rootfs.py (see also _save_postinsts of the
>   OpkgRootfs class)
> 
> This can be seen when using a distro using ipk and systemd, then
> installing opkg-keyrings and enable package management. The resulting
> rootfs has both services and will run opkg-keyrings.postinst twice.
> 
> Moreover, because the two services have similar dependency, they
> typically get executed almost at the same time:
>   Dec 13 14:47:25 colibri-imx6 systemd[1]: Starting Opkg first boot configure...
>   ...
>   Dec 13 14:47:25 colibri-imx6 systemd[1]: Starting Run pending postinsts...
> 
> This leads to executions of the same postinst scripts racing with each
> other! In practise update-alternative has been proven to be suspectable
> to such races.
> 
> Get rid of the opkg service to avoid dupplicate postinst execution.

With this applied there is one problem though: opkg status file still
shows the packages as just "unpacked". When running the package manager
(e.g. to install an unrelated package) opkg starts to handle the
postinst nonetheless.

So there is another issue basically. Looking at /usr/sbin/run-postinsts
I see that "opkg configure" is actually triggered if there are no
scripts in /etc/ipk-postinsts. However, meta/lib/oe/rootfs.py calls
_save_postinsts() no matter whether there is package management or no:


...
        if delayed_postinsts:
            self._save_postinsts()
            if image_rorfs:
                bb.warn("There are post install scripts "
                        "in a read-only rootfs")
...


Maybe it needs something like this?


        runtime_pkgmanage = bb.utils.contains("IMAGE_FEATURES",
"package-management",
                                              True, False, self.d)
        if delayed_postinsts and not runtime_pkgmanage:
            self._save_postinsts()
            if image_rorfs:
                bb.warn("There are post install scripts "
                        "in a read-only rootfs")



But then, this is common code for all supported package formats. It
seems that /usr/sbin/run-postinsts only supports package manager
postinst processing for deb and ipk...

Comments appreciated.

--
Stefan

> 
> 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.5.bb               | 13 -------------
>  2 files changed, 30 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.5.bb
> b/meta/recipes-devtools/opkg/opkg_0.3.5.bb
> index 3e511b6bb6..fb36e8b866 100644
> --- a/meta/recipes-devtools/opkg/opkg_0.3.5.bb
> +++ b/meta/recipes-devtools/opkg/opkg_0.3.5.bb
> @@ -22,8 +22,6 @@ SRC_URI[sha256sum] =
> "734bc21dea11262113fa86b928d09812618b3966f352350cf916a6ae0d
>  
>  inherit autotools pkgconfig systemd
>  
> -SYSTEMD_SERVICE_${PN} = "opkg-configure.service"
> -
>  target_localstatedir := "${localstatedir}"
>  OPKGLIBDIR = "${target_localstatedir}/lib"
>  
> @@ -46,16 +44,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 +56,6 @@ RPROVIDES_${PN} = "opkg-collateral"
>  PACKAGES =+ "libopkg"
>  
>  FILES_libopkg = "${libdir}/*.so.* ${OPKGLIBDIR}/opkg/"
> -FILES_${PN} += "${systemd_unitdir}/system/"
>  
>  BBCLASSEXTEND = "native nativesdk"


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-13 18:29 ` Stefan Agner
@ 2017-12-14 13:10   ` Alexander Kanavin
  2017-12-14 13:11     ` Stefan Agner
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Kanavin @ 2017-12-14 13:10 UTC (permalink / raw)
  To: Stefan Agner, openembedded-core; +Cc: muhammad_shakeel, Stefan Agner

On 12/13/2017 08:29 PM, Stefan Agner wrote:

> Maybe it needs something like this?
> 
> 
>          runtime_pkgmanage = bb.utils.contains("IMAGE_FEATURES",
> "package-management",
>                                                True, False, self.d)
>          if delayed_postinsts and not runtime_pkgmanage:
>              self._save_postinsts()
>              if image_rorfs:
>                  bb.warn("There are post install scripts "
>                          "in a read-only rootfs")

I don't think this is correct. Some postinsts are intentionally deferred 
to first boot, and they need to be run regardless of whether the image 
supports runtime package management or not.


Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 13:10   ` Alexander Kanavin
@ 2017-12-14 13:11     ` Stefan Agner
  2017-12-14 13:52       ` Alexander Kanavin
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Agner @ 2017-12-14 13:11 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: muhammad_shakeel, Stefan Agner, openembedded-core

On 2017-12-14 14:10, Alexander Kanavin wrote:
> On 12/13/2017 08:29 PM, Stefan Agner wrote:
> 
>> Maybe it needs something like this?
>>
>>
>>          runtime_pkgmanage = bb.utils.contains("IMAGE_FEATURES",
>> "package-management",
>>                                                True, False, self.d)
>>          if delayed_postinsts and not runtime_pkgmanage:
>>              self._save_postinsts()
>>              if image_rorfs:
>>                  bb.warn("There are post install scripts "
>>                          "in a read-only rootfs")
> 
> I don't think this is correct. Some postinsts are intentionally
> deferred to first boot, and they need to be run regardless of whether
> the image supports runtime package management or not.

Yes I know, the mentioned opkg-keyrings is such a case.

If there is no package management, then scripts get deployed via
/etc/*-postinsts, if package management is available then it will take
care of it.

--
Stefan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 13:11     ` Stefan Agner
@ 2017-12-14 13:52       ` Alexander Kanavin
  2017-12-14 13:59         ` Stefan Agner
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Kanavin @ 2017-12-14 13:52 UTC (permalink / raw)
  To: Stefan Agner; +Cc: muhammad_shakeel, Stefan Agner, openembedded-core

On 12/14/2017 03:11 PM, Stefan Agner wrote:
>
>> I don't think this is correct. Some postinsts are intentionally
>> deferred to first boot, and they need to be run regardless of whether
>> the image supports runtime package management or not.
> 
> Yes I know, the mentioned opkg-keyrings is such a case.
> 
> If there is no package management, then scripts get deployed via
> /etc/*-postinsts, if package management is available then it will take
> care of it.

 From reading the code, that only happens when using rpm. In other cases 
you need to execute _save_postinsts(), regardless of whether package 
management is available on image or not.

Whatever changes you try, do run oe-selftest's 
'test_postinst_rootfs_and_boot' test against them and make sure it 
doesn't regress.


Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 13:52       ` Alexander Kanavin
@ 2017-12-14 13:59         ` Stefan Agner
  2017-12-14 14:24           ` Alexander Kanavin
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Agner @ 2017-12-14 13:59 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: muhammad_shakeel, Stefan Agner, openembedded-core

On 2017-12-14 14:52, Alexander Kanavin wrote:
> On 12/14/2017 03:11 PM, Stefan Agner wrote:
>>
>>> I don't think this is correct. Some postinsts are intentionally
>>> deferred to first boot, and they need to be run regardless of whether
>>> the image supports runtime package management or not.
>>
>> Yes I know, the mentioned opkg-keyrings is such a case.
>>
>> If there is no package management, then scripts get deployed via
>> /etc/*-postinsts, if package management is available then it will take
>> care of it.
> 
> From reading the code, that only happens when using rpm. In other
> cases you need to execute _save_postinsts(), regardless of whether
> package management is available on image or not.

That at least contradicts my testing with opkg/ipk.

If /etc/ipk-postinsts is not there and /var/lib/opkg/status is there
(package management installed), then run-postinst runs "opkg configure",
which takes care of postinsts just fine.

Reading the code at least let me to believe that this is true for deb as
well.

Not sure how it works with rpm though. How do you came to the conclusion
that this only happens when using rpm?

> 
> Whatever changes you try, do run oe-selftest's
> 'test_postinst_rootfs_and_boot' test against them and make sure it
> doesn't regress.

Yeah will do when I have a solution which I am happy with.

Before that, I rather prefer understanding the whole issue and try to
come up with a solution that works on first regression test (rather than
try and error)...

--
Stefan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 13:59         ` Stefan Agner
@ 2017-12-14 14:24           ` Alexander Kanavin
  2017-12-14 14:41             ` Stefan Agner
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Kanavin @ 2017-12-14 14:24 UTC (permalink / raw)
  To: Stefan Agner; +Cc: Stefan Agner, openembedded-core

On 12/14/2017 03:59 PM, Stefan Agner wrote:

> That at least contradicts my testing with opkg/ipk.
> 
> If /etc/ipk-postinsts is not there and /var/lib/opkg/status is there
> (package management installed), then run-postinst runs "opkg configure",
> which takes care of postinsts just fine.
> 
> Reading the code at least let me to believe that this is true for deb as
> well.
> 
> Not sure how it works with rpm though. How do you came to the conclusion
> that this only happens when using rpm?

I think there was confusion between what happens at image creation time 
vs what happens at first boot :)

So what's the issue again that your RFC patch doesn't resolve? I read 
your followup and I still don't get it.

Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 14:24           ` Alexander Kanavin
@ 2017-12-14 14:41             ` Stefan Agner
  2017-12-14 14:55               ` Alexander Kanavin
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Agner @ 2017-12-14 14:41 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: Stefan Agner, openembedded-core

On 2017-12-14 15:24, Alexander Kanavin wrote:
> On 12/14/2017 03:59 PM, Stefan Agner wrote:
> 
>> That at least contradicts my testing with opkg/ipk.
>>
>> If /etc/ipk-postinsts is not there and /var/lib/opkg/status is there
>> (package management installed), then run-postinst runs "opkg configure",
>> which takes care of postinsts just fine.
>>
>> Reading the code at least let me to believe that this is true for deb as
>> well.
>>
>> Not sure how it works with rpm though. How do you came to the conclusion
>> that this only happens when using rpm?
> 
> I think there was confusion between what happens at image creation
> time vs what happens at first boot :)
> 
> So what's the issue again that your RFC patch doesn't resolve? I read
> your followup and I still don't get it.

I think removing the Opkg first boot systemd service (as the initial
patch does) is the correct first step.

However, it currently still leads to a second copy of the postinst
scripts in /etc if package management is enabled.

I am pretty sure that adding if delayed_postinsts and not
runtime_pkgmanage: should resolve the problem for ipk/deb fully: With
that run-postinsts will run "opkg configure"/"dpkg --configure -a"
respectively when package management is installed, and those command
will run all postinst correctly.

So for me the only question is what will happen with rpm... Will package
managment do something on first boot? And if yes, what mechanism?

If rpm needs /etc/*-postinsts even with package management, then maybe
we need to add:

if delayed_postinsts and (not runtime_pkgmanage or rpm):
    self._save_postinsts()

--
Stefan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 14:41             ` Stefan Agner
@ 2017-12-14 14:55               ` Alexander Kanavin
  2017-12-14 14:57                 ` Stefan Agner
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Kanavin @ 2017-12-14 14:55 UTC (permalink / raw)
  To: Stefan Agner; +Cc: Stefan Agner, openembedded-core

On 12/14/2017 04:41 PM, Stefan Agner wrote:

> I think removing the Opkg first boot systemd service (as the initial
> patch does) is the correct first step.
> 
> However, it currently still leads to a second copy of the postinst
> scripts in /etc if package management is enabled.
> I am pretty sure that adding if delayed_postinsts and not
> runtime_pkgmanage: should resolve the problem for ipk/deb fully: With
> that run-postinsts will run "opkg configure"/"dpkg --configure -a"
> respectively when package management is installed, and those command
> will run all postinst correctly.

Why is the second copy a problem? Can you elaborate? Don't describe the 
fix, describe the issue.

 From what I can see, run-postinsts will execute opkg configure if opkg 
is available, or run the scripts directly if opkg is not available - 
which is fine. Why do you need to avoid saving the scripts in /etc if 
opkg is installed?

Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 14:55               ` Alexander Kanavin
@ 2017-12-14 14:57                 ` Stefan Agner
  2017-12-14 15:14                   ` Alexander Kanavin
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Agner @ 2017-12-14 14:57 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: Stefan Agner, openembedded-core

On 2017-12-14 15:55, Alexander Kanavin wrote:
> On 12/14/2017 04:41 PM, Stefan Agner wrote:
> 
>> I think removing the Opkg first boot systemd service (as the initial
>> patch does) is the correct first step.
>>
>> However, it currently still leads to a second copy of the postinst
>> scripts in /etc if package management is enabled.
>> I am pretty sure that adding if delayed_postinsts and not
>> runtime_pkgmanage: should resolve the problem for ipk/deb fully: With
>> that run-postinsts will run "opkg configure"/"dpkg --configure -a"
>> respectively when package management is installed, and those command
>> will run all postinst correctly.
> 
> Why is the second copy a problem? Can you elaborate? Don't describe
> the fix, describe the issue.
> 
> From what I can see, run-postinsts will execute opkg configure if opkg
> is available, or run the scripts directly if opkg is not available -
> which is fine. Why do you need to avoid saving the scripts in /etc if
> opkg is installed?

No, it will call the scripts in /etc/*-postinsts (it bails out of the
for loop if the post inst dir is there...)

Then opkg status file still shows the packages as just "unpacked". When
running the package manager (e.g. to install an unrelated package) opkg
starts to handle the postinst again...

Rather than updating the package managers database etc I rather feel we
should just not do the OE specific postinst if package management can
handle it.

--
Stefan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 14:57                 ` Stefan Agner
@ 2017-12-14 15:14                   ` Alexander Kanavin
  2017-12-14 16:46                     ` Stefan Agner
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Kanavin @ 2017-12-14 15:14 UTC (permalink / raw)
  To: Stefan Agner; +Cc: Stefan Agner, openembedded-core

On 12/14/2017 04:57 PM, Stefan Agner wrote:
> On 2017-12-14 15:55, Alexander Kanavin wrote:
>> On 12/14/2017 04:41 PM, Stefan Agner wrote:
>>
>>> I think removing the Opkg first boot systemd service (as the initial
>>> patch does) is the correct first step.
>>>
>>> However, it currently still leads to a second copy of the postinst
>>> scripts in /etc if package management is enabled.
>>> I am pretty sure that adding if delayed_postinsts and not
>>> runtime_pkgmanage: should resolve the problem for ipk/deb fully: With
>>> that run-postinsts will run "opkg configure"/"dpkg --configure -a"
>>> respectively when package management is installed, and those command
>>> will run all postinst correctly.
>>
>> Why is the second copy a problem? Can you elaborate? Don't describe
>> the fix, describe the issue.
>>
>>  From what I can see, run-postinsts will execute opkg configure if opkg
>> is available, or run the scripts directly if opkg is not available -
>> which is fine. Why do you need to avoid saving the scripts in /etc if
>> opkg is installed?
> 
> No, it will call the scripts in /etc/*-postinsts (it bails out of the
> for loop if the post inst dir is there...)

Let's look at the code. Can you point out what the faulty code path is?

# the order of this list is important, do not change!
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
                         fi
                         ;;

                 "ipk")
                         if [ -s "#LOCALSTATEDIR#/lib/opkg/status" ]; then
                                 pm_installed=true
                         fi
                         ;;
         esac
         break
done

....

if $pm_installed; then
         case $pm in
                 "ipk")
                         eval opkg configure $append_log
                         ;;

                 "deb")
                         eval dpkg --configure -a $append_log
                         ;;
         esac
else
         exec_postinst_scriptlets
fi

# since all postinstalls executed successfully, remove the postinstalls 
directory
# and the rcS.d link
if [ $remove_pi_dir = 1 ]; then
         rm -rf $pi_dir
         remove_rcsd_link
fi


Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 15:14                   ` Alexander Kanavin
@ 2017-12-14 16:46                     ` Stefan Agner
  2017-12-14 16:59                       ` Alexander Kanavin
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Agner @ 2017-12-14 16:46 UTC (permalink / raw)
  To: Alexander Kanavin, jussi.kukkonen; +Cc: Stefan Agner, openembedded-core

On 2017-12-14 16:14, Alexander Kanavin wrote:
> On 12/14/2017 04:57 PM, Stefan Agner wrote:
>> On 2017-12-14 15:55, Alexander Kanavin wrote:
>>> On 12/14/2017 04:41 PM, Stefan Agner wrote:
>>>
>>>> I think removing the Opkg first boot systemd service (as the initial
>>>> patch does) is the correct first step.
>>>>
>>>> However, it currently still leads to a second copy of the postinst
>>>> scripts in /etc if package management is enabled.
>>>> I am pretty sure that adding if delayed_postinsts and not
>>>> runtime_pkgmanage: should resolve the problem for ipk/deb fully: With
>>>> that run-postinsts will run "opkg configure"/"dpkg --configure -a"
>>>> respectively when package management is installed, and those command
>>>> will run all postinst correctly.
>>>
>>> Why is the second copy a problem? Can you elaborate? Don't describe
>>> the fix, describe the issue.
>>>
>>>  From what I can see, run-postinsts will execute opkg configure if opkg
>>> is available, or run the scripts directly if opkg is not available -
>>> which is fine. Why do you need to avoid saving the scripts in /etc if
>>> opkg is installed?
>>
>> No, it will call the scripts in /etc/*-postinsts (it bails out of the
>> for loop if the post inst dir is there...)
> 
> Let's look at the code. Can you point out what the faulty code path is?
> 
> # the order of this list is important, do not change!
> 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


I need to admit that parts of my testing was using morty, and I looked
at the code which was on that platform.

That logic got changed... It used to looked like that in morty:
http://git.openembedded.org/openembedded-core/tree/meta/recipes-devtools/run-postinsts/run-postinsts/run-postinsts?h=morty#n16

The change has been made due to the same issue I mentioned above:
https://bugzilla.yoctoproject.org/show_bug.cgi?id=10478


So yes, it actually works right now. The systemd service still seems
superfluous though, since run-postinsts does the opkg configure.


But then, digging a bit deeper, I come to following findings:
IMHO, Jussi came to the wrong conclusion that the break is the bug
(Comment #4 in the above bug). The "break" was intentionally: The
intention was to let run-postinsts handle the scripts *always* if there
were script in the rootfs.

And a little bit earlier meta/lib/oe/rootfs.py did exectly what I
proposed:
http://git.openembedded.org/openembedded-core/tree/meta/lib/oe/rootfs.py?id=b198a189228648057c3be7d068598f50841b3bf9#n131

-> It only installed postinst if package management was disabled and
delayed postinsts were necessary.

I think the order of events was that the removing with package
management got broken with this commit:
http://git.openembedded.org/openembedded-core/commit/meta/lib/oe/rootfs.py?id=5aae19959a443c6ac4b0feef10715c8acf3c6376

Judging from the commit log that was not intentionally.

So from then on, we installed /etc/*-postinsts even when using package
management...

At that lead to the the issue #10478, which fixed the issue at
runtime...


Anyway, I think it is not sensible to deploy scripts we just delete on
first boot when we could have not deployed them in first place!

Also, we rely on some scripts in /etc/*-postinsts to be present to call
the package manager, although the package manger has no direct relation
to those scripts...


Suggestion:
- Still remove systemd opkg service as proposed, since run-postinsts
gets run with systemd and should/will handle package manager just fine
- Change run-postinsts to behave sane: Detect package management by
checking /var/lib/dpkg/status and /var/lib/opkg/status only (remove the
if [ ! -d $pi_dir ]; then continue; fi...)
- Stop deploying /etc/*-postinsts when package management is installed

--
Stefan


> 
>         # found the package manager, it has postinsts
>         case $pm in
>                 "deb")
>                         if [ -s "#LOCALSTATEDIR#/lib/dpkg/status" ]; then
>                                 pm_installed=true
>                         fi
>                         ;;
> 
>                 "ipk")
>                         if [ -s "#LOCALSTATEDIR#/lib/opkg/status" ]; then
>                                 pm_installed=true
>                         fi
>                         ;;
>         esac
>         break
> done
> 
> ....
> 
> if $pm_installed; then
>         case $pm in
>                 "ipk")
>                         eval opkg configure $append_log
>                         ;;
> 
>                 "deb")
>                         eval dpkg --configure -a $append_log
>                         ;;
>         esac
> else
>         exec_postinst_scriptlets
> fi
> 
> # since all postinstalls executed successfully, remove the
> postinstalls directory
> # and the rcS.d link
> if [ $remove_pi_dir = 1 ]; then
>         rm -rf $pi_dir
>         remove_rcsd_link
> fi
> 
> 
> Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 16:46                     ` Stefan Agner
@ 2017-12-14 16:59                       ` Alexander Kanavin
  2017-12-14 17:16                         ` Stefan Agner
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Kanavin @ 2017-12-14 16:59 UTC (permalink / raw)
  To: Stefan Agner; +Cc: Stefan Agner, openembedded-core

On 12/14/2017 06:46 PM, Stefan Agner wrote:

> Suggestion:
> - Still remove systemd opkg service as proposed, since run-postinsts
> gets run with systemd and should/will handle package manager just fine

Yes.

> - Change run-postinsts to behave sane: Detect package management by
> checking /var/lib/dpkg/status and /var/lib/opkg/status only (remove the
> if [ ! -d $pi_dir ]; then continue; fi...)

Maybe; you need to explain this carefully in the commit message.

> - Stop deploying /etc/*-postinsts when package management is installed

Probably not; rpm does not have a facility for 'delayed postinst 
execution' the way dpkg/opkg do, and this would create more confusion 
and special-casing than achieve any real improvement imo.

Jussi left Intel several months ago.


Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 16:59                       ` Alexander Kanavin
@ 2017-12-14 17:16                         ` Stefan Agner
  2017-12-14 17:28                           ` Alexander Kanavin
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Agner @ 2017-12-14 17:16 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: Stefan Agner, openembedded-core

On 2017-12-14 17:59, Alexander Kanavin wrote:
> On 12/14/2017 06:46 PM, Stefan Agner wrote:
> 
>> Suggestion:
>> - Still remove systemd opkg service as proposed, since run-postinsts
>> gets run with systemd and should/will handle package manager just fine
> 
> Yes.
> 
>> - Change run-postinsts to behave sane: Detect package management by
>> checking /var/lib/dpkg/status and /var/lib/opkg/status only (remove the
>> if [ ! -d $pi_dir ]; then continue; fi...)
> 
> Maybe; you need to explain this carefully in the commit message.
> 
>> - Stop deploying /etc/*-postinsts when package management is installed
> 
> Probably not; rpm does not have a facility for 'delayed postinst
> execution' the way dpkg/opkg do, and this would create more confusion
> and special-casing than achieve any real improvement imo.

Are you sure that rpm has no such facility? Maybe there is some other
mechanism around...

If rpm really does not handle it, would mean that delayed postinst would
have been broken back when we did not ship /etc/*-postinsts:
http://git.openembedded.org/openembedded-core/commit/meta/lib/oe/rootfs.py?id=5aae19959a443c6ac4b0feef10715c8acf3c6376

If there is special case handling needed it is not particularly nice due
to the additional condition.

But IMHO still better, it can easily be explained in rootfs.py with a
comment like:

    # deb/ipk package management is able to handle postinsts on first
boot.
    if ... 

I will do some tests with rpm and send a patchset.

> 
> Jussi left Intel several months ago.

I see.

--
Stefan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 17:16                         ` Stefan Agner
@ 2017-12-14 17:28                           ` Alexander Kanavin
  2017-12-14 17:40                             ` Stefan Agner
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Kanavin @ 2017-12-14 17:28 UTC (permalink / raw)
  To: Stefan Agner; +Cc: Stefan Agner, openembedded-core

On 12/14/2017 07:16 PM, Stefan Agner wrote:

> Are you sure that rpm has no such facility? Maybe there is some other
> mechanism around...

You are welcome to find it. rpm does not distinguish between unpacking 
and configuring steps, and just does everything in a single installation 
step.

> If rpm really does not handle it, would mean that delayed postinst would
> have been broken back when we did not ship /etc/*-postinsts:
> http://git.openembedded.org/openembedded-core/commit/meta/lib/oe/rootfs.py?id=5aae19959a443c6ac4b0feef10715c8acf3c6376

Rpm had a special way to save postinstall scripts (due to the same 
problem I described just above), and still does, which means 
_save_postinsts() did (and does) nothing in rpm case.

> If there is special case handling needed it is not particularly nice due
> to the additional condition.
> 
> But IMHO still better, it can easily be explained in rootfs.py with a
> comment like:
> 
>      # deb/ipk package management is able to handle postinsts on first
> boot.

Please no; the class in question is the generic Rootfs(), and I do not 
want to add packagemanager-specific clauses there.

Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 17:28                           ` Alexander Kanavin
@ 2017-12-14 17:40                             ` Stefan Agner
  2017-12-14 17:49                               ` Alexander Kanavin
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Agner @ 2017-12-14 17:40 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: Stefan Agner, openembedded-core

On 2017-12-14 18:28, Alexander Kanavin wrote:
> On 12/14/2017 07:16 PM, Stefan Agner wrote:
> 
>> Are you sure that rpm has no such facility? Maybe there is some other
>> mechanism around...
> 
> You are welcome to find it. rpm does not distinguish between unpacking
> and configuring steps, and just does everything in a single
> installation step.
> 
>> If rpm really does not handle it, would mean that delayed postinst would
>> have been broken back when we did not ship /etc/*-postinsts:
>> http://git.openembedded.org/openembedded-core/commit/meta/lib/oe/rootfs.py?id=5aae19959a443c6ac4b0feef10715c8acf3c6376
> 
> Rpm had a special way to save postinstall scripts (due to the same
> problem I described just above), and still does, which means
> _save_postinsts() did (and does) nothing in rpm case.
> 

Oh, I see, well that simplifies it, doesn't it? E.g.

    # If package managers support postinsts and the package manager is
present on the
    # rootfs, then it will handle postinsts just fine, no need to deploy
scripts again.
    if delayed_postinsts and not runtime_pkgmanage:
        self._save_postinsts()

And with that it will be as it used to be before the above commit, and
the way it should be.

--
Stefan

>> If there is special case handling needed it is not particularly nice due
>> to the additional condition.
>>
>> But IMHO still better, it can easily be explained in rootfs.py with a
>> comment like:
>>
>>      # deb/ipk package management is able to handle postinsts on first
>> boot.
> 
> Please no; the class in question is the generic Rootfs(), and I do not
> want to add packagemanager-specific clauses there.
> 
> Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 17:40                             ` Stefan Agner
@ 2017-12-14 17:49                               ` Alexander Kanavin
  2017-12-14 18:04                                 ` Alexander Kanavin
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Kanavin @ 2017-12-14 17:49 UTC (permalink / raw)
  To: Stefan Agner; +Cc: Stefan Agner, openembedded-core

On 12/14/2017 07:40 PM, Stefan Agner wrote:

> Oh, I see, well that simplifies it, doesn't it? E.g.
> 
>      # If package managers support postinsts and the package manager is
> present on the
>      # rootfs, then it will handle postinsts just fine, no need to deploy
> scripts again.
>      if delayed_postinsts and not runtime_pkgmanage:
>          self._save_postinsts()
> 
> And with that it will be as it used to be before the above commit, and
> the way it should be.

Sorry, but no. You are making an implicit assumption about how rpmrootfs 
child class behaves here, which is not a good thing to do in a parent class.

Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 17:49                               ` Alexander Kanavin
@ 2017-12-14 18:04                                 ` Alexander Kanavin
  2017-12-16 12:02                                   ` Stefan Agner
  0 siblings, 1 reply; 19+ messages in thread
From: Alexander Kanavin @ 2017-12-14 18:04 UTC (permalink / raw)
  To: Stefan Agner; +Cc: Stefan Agner, openembedded-core

On 12/14/2017 07:49 PM, Alexander Kanavin wrote:
> On 12/14/2017 07:40 PM, Stefan Agner wrote:
> 
>> Oh, I see, well that simplifies it, doesn't it? E.g.
>>
>>      # If package managers support postinsts and the package manager is
>> present on the
>>      # rootfs, then it will handle postinsts just fine, no need to deploy
>> scripts again.
>>      if delayed_postinsts and not runtime_pkgmanage:
>>          self._save_postinsts()
>>
>> And with that it will be as it used to be before the above commit, and
>> the way it should be.
> 
> Sorry, but no. You are making an implicit assumption about how rpmrootfs 
> child class behaves here, which is not a good thing to do in a parent 
> class.

What you *can* do however is move the "and not runtime_pkgmanage" check 
into the child classes for opkg and dpkg. I'm fine with that.

Alex


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC] opkg: avoid running postinst scripts twice when using systemd
  2017-12-14 18:04                                 ` Alexander Kanavin
@ 2017-12-16 12:02                                   ` Stefan Agner
  0 siblings, 0 replies; 19+ messages in thread
From: Stefan Agner @ 2017-12-16 12:02 UTC (permalink / raw)
  To: Alexander Kanavin; +Cc: Stefan Agner, openembedded-core

On 2017-12-14 19:04, Alexander Kanavin wrote:
> On 12/14/2017 07:49 PM, Alexander Kanavin wrote:
>> On 12/14/2017 07:40 PM, Stefan Agner wrote:
>>
>>> Oh, I see, well that simplifies it, doesn't it? E.g.
>>>
>>>      # If package managers support postinsts and the package manager is
>>> present on the
>>>      # rootfs, then it will handle postinsts just fine, no need to deploy
>>> scripts again.
>>>      if delayed_postinsts and not runtime_pkgmanage:
>>>          self._save_postinsts()
>>>
>>> And with that it will be as it used to be before the above commit, and
>>> the way it should be.
>>
>> Sorry, but no. You are making an implicit assumption about how rpmrootfs child class behaves here, which is not a good thing to do in a parent class.
> 
> What you *can* do however is move the "and not runtime_pkgmanage"
> check into the child classes for opkg and dpkg. I'm fine with that.
> 

That sounds sensible. Will send a patchset.

--
Stefan


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2017-12-16 12:07 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-13 18:06 [RFC] opkg: avoid running postinst scripts twice when using systemd Stefan Agner
2017-12-13 18:29 ` Stefan Agner
2017-12-14 13:10   ` Alexander Kanavin
2017-12-14 13:11     ` Stefan Agner
2017-12-14 13:52       ` Alexander Kanavin
2017-12-14 13:59         ` Stefan Agner
2017-12-14 14:24           ` Alexander Kanavin
2017-12-14 14:41             ` Stefan Agner
2017-12-14 14:55               ` Alexander Kanavin
2017-12-14 14:57                 ` Stefan Agner
2017-12-14 15:14                   ` Alexander Kanavin
2017-12-14 16:46                     ` Stefan Agner
2017-12-14 16:59                       ` Alexander Kanavin
2017-12-14 17:16                         ` Stefan Agner
2017-12-14 17:28                           ` Alexander Kanavin
2017-12-14 17:40                             ` Stefan Agner
2017-12-14 17:49                               ` Alexander Kanavin
2017-12-14 18:04                                 ` Alexander Kanavin
2017-12-16 12:02                                   ` Stefan Agner

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.