Skip to contentSkip to conversation

Fix first boot deferral and postinst error reporting

RFC PATCH New Alexander Kanavin 1 participant 7 messages Jan 8, 2018

lore
Share & tools
lore
#0 lore raw

cover Fix first boot deferral and postinst error reporting

This patchset is aiming to fix long-standing problems with package post-installation scriptlets:

1) The way to defer them to first boot is to issue 'exit 1'. On the other hand, 'exit 1' is also the way to indicate an error in the script. The patches decouple these two things.

2) Scriptlets should be executed using '-e', otherwise failing commands in the middle of the scriptlet will not be reported as such.

3) The patchset also adds a convenient shortcut for deferring scriptlets to first boot execution:

pkg_postinst_ontarget_PACKAGENAME() {
# Actions to carry out on the device go here
....
}

Please review the following changes for suitability for inclusion. If you have any objections or suggestions for improvement, please respond to the patches. If you agree with the changes, please provide your Acked-by.

The following changes since commit 205cfd702190026e64eed9cae27c05ff62d1637e:

bitbake: fetch2/__init__: Disable pseudo in runfetchcmd() (2018-01-08 08:48:54 +0000)

are available in the Git repository at:

git://git.yoctoproject.org/poky-contrib akanavin/dnf-rpm4-postinst-fix
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=akanavin/dnf-rpm4-postinst-fix

Alexander Kanavin (6):
postinst_intercepts: add a delay_to_first_boot hook meta/lib/oe/rootfs.py: separate first boot deferral logic into a
separate function
meta/lib/oe/rootfs.py: do not execute defer_to_first_boot when
processing postinst_intercept hooks
package.bbclass: add support for pkg_postinst_ontarget() meta/lib/oe/package_manager.py: deprecate 'exit 1' as a way to defer
to first boot
package_rpm.bbclass: run pre/post installation scriptlets using sh -e

 .../recipes-test/postinst/postinst_1.0.bb          | 14 ++------
 meta/classes/package.bbclass                       | 20 +++++++++++
 meta/classes/package_rpm.bbclass                   |  8 ++---
 meta/lib/oe/package_manager.py                     |  3 ++
 meta/lib/oe/rootfs.py                              | 39 +++++++++++++---------
 scripts/postinst-intercepts/delay_to_first_boot    |  2 ++
 6 files changed, 54 insertions(+), 32 deletions(-)
 create mode 100644 scripts/postinst-intercepts/delay_to_first_boot
Signature · 1 line
2.15.1
#1 lore raw

RFC PATCH 1/6 postinst_intercepts: add a delay_to_first_boot hook

This allows postinsts scripts to explicitly request a deferral to first boot (by calling 'postinst_intercept delay_to_first_boot') instead of 'exit 1' which should be used only to indicate actual script failures.

---
 scripts/postinst-intercepts/delay_to_first_boot | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 scripts/postinst-intercepts/delay_to_first_boot
scripts/postinst-intercepts/delay_to_first_boot new file +2 -0
new file mode 100644
index 00000000000..ecdbef95dde
@@ -0,0 +1,2 @@
+#!/bin/sh
+exit 1
Signature · 1 line
2.15.1
#2 lore raw

RFC PATCH 2/6 meta/lib/oe/rootfs.py: separate first boot deferral logic into a separate function

In reply to Alexander Kanavin
---
 meta/lib/oe/rootfs.py | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)
meta/lib/oe/rootfs.py +19 -16
index 754ef563abe..d3ec8a56c48 100644
@@ -293,6 +293,24 @@ class Rootfs(object, metaclass=ABCMeta):
             # Remove the package manager data files
             self.pm.remove_packaging_data()
 
+    def _postpone_to_first_boot(self, postinst_intercept_hook):
+        with open(postinst_intercept_hook) as intercept:
+            registered_pkgs = None
+            for line in intercept.read().split("\n"):
+                m = re.match("^##PKGS:(.*)", line)
+                if m is not None:
+                    registered_pkgs = m.group(1).strip()
+                    break
+
+            if registered_pkgs is not None:
+                bb.warn("The postinstalls for the following packages "
+                        "will be postponed for first boot: %s" %
+                        registered_pkgs)
+
+                # call the backend dependent handler
+                self._handle_intercept_failure(registered_pkgs)
+
+
     def _run_intercepts(self):
         intercepts_dir = os.path.join(self.d.getVar('WORKDIR'),
                                       "intercept_scripts")
@@ -314,22 +332,7 @@ class Rootfs(object, metaclass=ABCMeta):
             except subprocess.CalledProcessError as e:
                 bb.warn("The postinstall intercept hook '%s' failed, details in log.do_rootfs" % script)
                 bb.note("Exit code %d. Output:\n%s" % (e.returncode, e.output.decode("utf-8")))
-
-                with open(script_full) as intercept:
-                    registered_pkgs = None
-                    for line in intercept.read().split("\n"):
-                        m = re.match("^##PKGS:(.*)", line)
-                        if m is not None:
-                            registered_pkgs = m.group(1).strip()
-                            break
-
-                    if registered_pkgs is not None:
-                        bb.warn("The postinstalls for the following packages "
-                                "will be postponed for first boot: %s" %
-                                registered_pkgs)
-
-                        # call the backend dependent handler
-                        self._handle_intercept_failure(registered_pkgs)
+                self._postpone_to_first_boot(script_full)
 
     def _run_ldconfig(self):
         if self.d.getVar('LDCONFIGDEPEND'):
Signature · 1 line
2.15.1
#3 lore raw

RFC PATCH 3/6 meta/lib/oe/rootfs.py: do not execute defer_to_first_boot when processing postinst_intercept hooks

In reply to Alexander Kanavin

That hook is empty, and doesn't need to be executed; it merely indicates that packages that have used it are requesting to defer their postinst scripts to first boot unconditionally.

---
 meta/lib/oe/rootfs.py | 4 ++++
 1 file changed, 4 insertions(+)
meta/lib/oe/rootfs.py +4 -0
index d3ec8a56c48..8bf77aea492 100644
@@ -324,6 +324,10 @@ class Rootfs(object, metaclass=ABCMeta):
             if script == "postinst_intercept" or not os.access(script_full, os.X_OK):
                 continue
 
+            if script == "delay_to_first_boot":
+                self._postpone_to_first_boot(script_full)
+                continue
+
             bb.note("> Executing %s intercept ..." % script)
 
             try:
Signature · 1 line
2.15.1
#4 lore raw

RFC PATCH 4/6 package.bbclass: add support for pkg_postinst_ontarget()

In reply to Alexander Kanavin

This function is a convenient and more readable shortcut for situations when the postinst code always needs to run on target. All commands that cannot be executed during cross-install and can only be run on target should go into this function. They will only be executed on first boot (if package was cross-installed) or immediately during package installation on target.

Plain pkg_postinst() works as before: it is run during cross-install time, it can contain a request to defer to first boot, and it is also run during package installation on target.

Also fix the oeqa test for this functionality to use the new function where appropriate.

---
 meta-selftest/recipes-test/postinst/postinst_1.0.bb | 14 ++------------
 meta/classes/package.bbclass                        | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 12 deletions(-)
meta-selftest/recipes-test/postinst/postinst_1.0.bb +2 -12
index 112aa08c80f..d4bab6dcc22 100644
@@ -35,14 +35,9 @@ pkg_postinst_${PN}-rootfs () {
 
 # Depends on rootfs, delays until first boot, verifies that the rootfs file was
 # written.
-pkg_postinst_${PN}-delayed-a () {
+pkg_postinst_ontarget_${PN}-delayed-a () {
     set -e
 
-    if [ -n "$D" ]; then
-        echo "Delaying ${PN}-delayed-a until first boot"
-        exit 1
-    fi
-
     if [ ! -e ${TESTDIR}/rootfs ]; then
         echo "${PN}-delayed-a: ${TESTDIR}/rootfs not found"
         exit 1
@@ -53,14 +48,9 @@ pkg_postinst_${PN}-delayed-a () {
 
 # Depends on delayed-a, delays until first boot, verifies that the delayed-a file was
 # written. This verifies the ordering between delayed postinsts.
-pkg_postinst_${PN}-delayed-b () {
+pkg_postinst_ontarget_${PN}-delayed-b () {
     set -e
 
-    if [ -n "$D" ]; then
-        echo "Delaying ${PN}-delayed-b until first boot"
-        exit 1
-    fi
-
     if [ ! -e ${TESTDIR}/delayed-a ]; then
         echo "${PN}-delayed-b: ${TESTDIR}/delayed-a not found"
         exit 1
meta/classes/package.bbclass +20 -0
index 7dc759699f4..6a7f35a3e78 100644
@@ -1320,6 +1320,25 @@ python emit_pkgdata() {
     from glob import glob
     import json
 
+    def process_postinst_on_target(pkg, mlprefix):
+        defer_fragment = """
+if [ -n "$D" ]; then
+    $INTERCEPT_DIR/postinst_intercept delay_to_first_boot %s mlprefix=%s
+    exit 0
+fi
+""" % (pkg, mlprefix)
+
+        postinst = d.getVar('pkg_postinst_%s' % pkg)
+        postinst_ontarget = d.getVar('pkg_postinst_ontarget_%s' % pkg)
+
+        if postinst_ontarget:
+            bb.debug(1, 'adding deferred pkg_postinst_ontarget() to pkg_postinst() for %s' % pkg)
+            if not postinst:
+                postinst = '#!/bin/sh\n'
+            postinst += defer_fragment
+            postinst += postinst_ontarget
+            d.setVar('pkg_postinst_%s' % pkg, postinst)
+
     def write_if_exists(f, pkg, var):
         def encode(str):
             import codecs
@@ -1415,6 +1434,7 @@ python emit_pkgdata() {
         write_if_exists(sf, pkg, 'ALLOW_EMPTY')
         write_if_exists(sf, pkg, 'FILES')
         write_if_exists(sf, pkg, 'CONFFILES')
+        process_postinst_on_target(pkg, d.getVar("MLPREFIX"))
         write_if_exists(sf, pkg, 'pkg_postinst')
         write_if_exists(sf, pkg, 'pkg_postrm')
         write_if_exists(sf, pkg, 'pkg_preinst')
Signature · 1 line
2.15.1
#5 lore raw

RFC PATCH 5/6 meta/lib/oe/package_manager.py: deprecate 'exit 1' as a way to defer to first boot

In reply to Alexander Kanavin

'exit 1' is not optimal for two reasons:

1) Code is hard to read; it is not obvious that it means 'defer what follows to first boot'.
2) Worse, this hides actual errors in the scriptlets; there is no difference between scriptlet failing because it's intended to be run on target and scriptlet failing because there's a bug or a regression somewhere.

The new, supported way is to place the code that has to run on target into pkg_postinst_ontarget(), or, if a more fine-tuned control is required, call 'postinst-intercepts defer_to_first_boot' from pkg_postinst() to explicitly request deferral to first boot.

---
 meta/lib/oe/package_manager.py | 3 +++
 1 file changed, 3 insertions(+)
meta/lib/oe/package_manager.py +3 -0
index ea99165d61b..0cc7f3cb3cc 100644
@@ -604,6 +604,9 @@ class RpmPM(PackageManager):
             if line.startswith("Non-fatal POSTIN scriptlet failure in rpm package"):
                 failed_scriptlets_pkgnames[line.split()[-1]] = True
 
+        if len(failed_scriptlets_pkgnames) > 0:
+            bb.warn("Intentionally failing postinstall scriptlets of %s to defer them to first boot is deprecated. Please place them into pkg_postinst_ontarget_${PN} ()." %(list(failed_scriptlets_pkgnames.keys())))
+            bb.warn("If deferring to first boot wasn't the intent, then scriptlet failure may mean an issue in the recipe, or a regression elsewhere.")
         for pkg in failed_scriptlets_pkgnames.keys():
             self.save_rpmpostinst(pkg)
 
Signature · 1 line
2.15.1
#6 lore raw

RFC PATCH 6/6 package_rpm.bbclass: run pre/post installation scriptlets using sh -e

In reply to Alexander Kanavin

This allows catching errors in the scriptlets which would otherwise go unnoticed, e.g. this sequence:

====
bogus_command
proper_command
====

would work just fine. Note that this patch needs all of the preceding patches, as otherwise running failing scriptlets with -e would defer them to first boot, instead of properly reporting failure and aborting the package installation.

---
 meta/classes/package_rpm.bbclass | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
meta/classes/package_rpm.bbclass +4 -4
index b08608c8466..f4b6d0b65f2 100644
@@ -465,12 +465,12 @@ python write_specfile () {
 
         # Now process scriptlets
         if splitrpreinst:
-            spec_scriptlets_bottom.append('%%pre -n %s' % splitname)
+            spec_scriptlets_bottom.append('%%pre -n %s -p "/bin/sh -e"' % splitname)
             spec_scriptlets_bottom.append('# %s - preinst' % splitname)
             spec_scriptlets_bottom.append(splitrpreinst)
             spec_scriptlets_bottom.append('')
         if splitrpostinst:
-            spec_scriptlets_bottom.append('%%post -n %s' % splitname)
+            spec_scriptlets_bottom.append('%%post -n %s -p "/bin/sh -e"' % splitname)
             spec_scriptlets_bottom.append('# %s - postinst' % splitname)
             spec_scriptlets_bottom.append(splitrpostinst)
             spec_scriptlets_bottom.append('')
@@ -559,12 +559,12 @@ python write_specfile () {
     spec_preamble_top.append('')
 
     if srcrpreinst:
-        spec_scriptlets_top.append('%pre')
+        spec_scriptlets_top.append('%pre -p "/bin/sh -e"')
         spec_scriptlets_top.append('# %s - preinst' % srcname)
         spec_scriptlets_top.append(srcrpreinst)
         spec_scriptlets_top.append('')
     if srcrpostinst:
-        spec_scriptlets_top.append('%post')
+        spec_scriptlets_top.append('%post -p "/bin/sh -e"')
         spec_scriptlets_top.append('# %s - postinst' % srcname)
         spec_scriptlets_top.append(srcrpostinst)
         spec_scriptlets_top.append('')
Signature · 1 line
2.15.1