Re: [RFC] opkg: avoid running postinst scripts twice when using systemd

[Stefan Agner <stefan@agner.ch>]

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