package.bbclass: run pre/post installation/removal scriptlets using sh -e

1/4

---
 meta/classes/package.bbclass     | 12 ++++++++++++
 meta/classes/package_rpm.bbclass |  8 ++++----
 2 files changed, 16 insertions(+), 4 deletions(-)
index 317c77585fc..83f53a49ef6 100644
@@ -1343,6 +1343,17 @@ fi
             postinst += postinst_ontarget
             d.setVar('pkg_postinst_%s' % pkg, postinst)
 
+    def add_set_e_to_scriptlets(pkg):
+        for scriptlet_name in ('pkg_preinst', 'pkg_postinst', 'pkg_prerm', 'pkg_postrm'):
+            scriptlet = d.getVar('%s_%s' % (scriptlet_name, pkg))
+            if scriptlet:
+                scriptlet_split = scriptlet.split('\n')
+                if scriptlet_split[0].startswith("#!"):
+                    scriptlet = scriptlet_split[0] + "\nset -e\n" + "\n".join(scriptlet_split[1:])
+                else:
+                    scriptlet = "set -e\n" + "\n".join(scriptlet_split[0:])
+            d.setVar('%s_%s' % (scriptlet_name, pkg), scriptlet)
+
     def write_if_exists(f, pkg, var):
         def encode(str):
             import codecs

@@ -1439,6 +1450,7 @@ fi
         write_if_exists(sf, pkg, 'FILES')
         write_if_exists(sf, pkg, 'CONFFILES')
         process_postinst_on_target(pkg, d.getVar("MLPREFIX"))
+        add_set_e_to_scriptlets(pkg)
         write_if_exists(sf, pkg, 'pkg_postinst')
         write_if_exists(sf, pkg, 'pkg_postrm')
         write_if_exists(sf, pkg, 'pkg_preinst')

index e26b2ad6625..af64ef62c58 100644
@@ -470,12 +470,12 @@ python write_specfile () {
 
         # Now process scriptlets
         if splitrpreinst:
-            spec_scriptlets_bottom.append('%%pre -n %s -p "/bin/sh -e"' % splitname)
+            spec_scriptlets_bottom.append('%%pre -n %s' % 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 -p "/bin/sh -e"' % splitname)
+            spec_scriptlets_bottom.append('%%post -n %s' % splitname)
             spec_scriptlets_bottom.append('# %s - postinst' % splitname)
             spec_scriptlets_bottom.append(splitrpostinst)
             spec_scriptlets_bottom.append('')

@@ -564,12 +564,12 @@ python write_specfile () {
     spec_preamble_top.append('')
 
     if srcrpreinst:
-        spec_scriptlets_top.append('%pre -p "/bin/sh -e"')
+        spec_scriptlets_top.append('%pre')
         spec_scriptlets_top.append('# %s - preinst' % srcname)
         spec_scriptlets_top.append(srcrpreinst)
         spec_scriptlets_top.append('')
     if srcrpostinst:
-        spec_scriptlets_top.append('%post -p "/bin/sh -e"')
+        spec_scriptlets_top.append('%post')
         spec_scriptlets_top.append('# %s - postinst' % srcname)
         spec_scriptlets_top.append(srcrpostinst)
         spec_scriptlets_top.append('')

2.16.1
---
 meta/lib/oe/package_manager.py | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)
index c10efb8df4c..0c5cf3cf7f3 100644
@@ -83,6 +83,11 @@ def opkg_query(cmd_output):
 
     return output
 
+# Note: this should be bb.fatal in the future.
+def failed_postinsts_warn(pkgs, log_path):
+    bb.warn("""Intentionally failing postinstall scriptlets of %s to defer them to first boot is deprecated. Please place them into pkg_postinst_ontarget_${PN} ().
+If deferring to first boot wasn't the intent, then scriptlet failure may mean an issue in the recipe, or a regression elsewhere.
+Details of the failure are in %s.""" %(pkgs, log_path))
 
 class Indexer(object, metaclass=ABCMeta):
     def __init__(self, d, deploy_dir):

@@ -709,8 +714,7 @@ class RpmPM(PackageManager):
                 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.")
+            failed_postinsts_warn(list(failed_scriptlets_pkgnames.keys()), self.d.expand("${T}/log.do_rootfs"))
         for pkg in failed_scriptlets_pkgnames.keys():
             self.save_rpmpostinst(pkg)
 

@@ -1172,6 +1176,13 @@ class OpkgPM(OpkgDpkgPM):
             bb.note(cmd)
             output = subprocess.check_output(cmd.split(), stderr=subprocess.STDOUT).decode("utf-8")
             bb.note(output)
+            failed_pkgs = []
+            for line in output.split('\n'):
+                if line.endswith("configuration required on target."):
+                    bb.warn(line)
+                    failed_pkgs.append(line.split(".")[0])
+            if failed_pkgs:
+                failed_postinsts_warn(failed_pkgs, self.d.expand("${T}/log.do_rootfs"))
         except subprocess.CalledProcessError as e:
             (bb.fatal, bb.warn)[attempt_only]("Unable to install packages. "
                                               "Command '%s' returned %d:\n%s" %

@@ -1435,9 +1446,10 @@ class DpkgPM(OpkgDpkgPM):
                                 stderr=subprocess.STDOUT).decode("utf-8")
                         bb.note(output)
                     except subprocess.CalledProcessError as e:
-                        bb.note("%s for package %s failed with %d:\n%s" %
+                        bb.warn("%s for package %s failed with %d:\n%s" %
                                 (control_script.name, pkg_name, e.returncode,
                                     e.output.decode("utf-8")))
+                        failed_postinsts_warn([pkg_name], self.d.expand("${T}/log.do_rootfs"))
                         failed_pkgs.append(pkg_name)
                         break
 

2.16.1
---
 .../recipes-test/postinst/postinst_1.0.bb          | 14 +++++++-
 meta/lib/oeqa/selftest/cases/runtime_test.py       | 37 ++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)
index d4bab6dcc22..913bfabf89e 100644
@@ -3,11 +3,12 @@ LICENSE = "MIT"
 
 inherit allarch
 
-PACKAGES = "${PN}-rootfs ${PN}-delayed-a ${PN}-delayed-b"
+PACKAGES = "${PN}-rootfs ${PN}-delayed-a ${PN}-delayed-b ${PN}-rootfs-failing"
 
 ALLOW_EMPTY_${PN}-rootfs = "1"
 ALLOW_EMPTY_${PN}-delayed-a = "1"
 ALLOW_EMPTY_${PN}-delayed-b = "1"
+ALLOW_EMPTY_${PN}-rootfs-failing = "1"
 
 RDEPENDS_${PN}-delayed-a = "${PN}-rootfs"
 RDEPENDS_${PN}-delayed-b = "${PN}-delayed-a"

@@ -58,3 +59,14 @@ pkg_postinst_ontarget_${PN}-delayed-b () {
 
     touch ${TESTDIR}/delayed-b
 }
+
+# This scriptlet intentionally includes a bogus command in the middle to test 
+# that we catch and report such errors properly.
+pkg_postinst_${PN}-rootfs-failing () {
+    mkdir -p $D${TESTDIR}
+    touch $D${TESTDIR}/rootfs-before-failure
+    run_a_really_broken_command
+    # Scriptlet execution should stop here; the following commands are NOT supposed to run.
+    # (oe-selftest checks for it).
+    touch $D${TESTDIR}/rootfs-after-failure
+}

index 1c69255b568..9c9b4b34111 100644
@@ -221,3 +221,40 @@ class Postinst(OESelftestTestCase):
                         for filename in ("rootfs", "delayed-a", "delayed-b"):
                             status, output = qemu.run_serial("test -f %s && echo found" % os.path.join(targettestdir, filename))
                             self.assertEqual(output, "found", "%s was not present on boot" % filename)
+
+
+
+    def test_failing_postinst(self):
+        """
+        Summary:        The purpose of this test case is to verify that post-installation
+                        scripts that contain errors are properly reported.
+        Expected:       The scriptlet failure is properly reported.
+                        The file that is created after the error in the scriptlet is not present.
+        Product: oe-core
+        Author: Alexander Kanavin <[email protected]>
+        """
+
+        import oe.path
+
+        vars = get_bb_vars(("IMAGE_ROOTFS", "sysconfdir"), "core-image-minimal")
+        rootfs = vars["IMAGE_ROOTFS"]
+        self.assertIsNotNone(rootfs)
+        sysconfdir = vars["sysconfdir"]
+        self.assertIsNotNone(sysconfdir)
+        # Need to use oe.path here as sysconfdir starts with /
+        hosttestdir = oe.path.join(rootfs, sysconfdir, "postinst-test")
+
+        for classes in ("package_rpm", "package_deb", "package_ipk"):
+            with self.subTest(package_class=classes):
+                features = 'CORE_IMAGE_EXTRA_INSTALL = "postinst-rootfs-failing"\n'
+                features += 'PACKAGE_CLASSES = "%s"\n' % classes
+                self.write_config(features)
+                bb_result = bitbake('core-image-minimal')
+                self.assertGreaterEqual(bb_result.output.find("Intentionally failing postinstall scriptlets of ['postinst-rootfs-failing'] to defer them to first boot is deprecated."), 0,
+                    "Warning about a failed scriptlet not found in bitbake output: %s" %(bb_result.output))
+
+                self.assertTrue(os.path.isfile(os.path.join(hosttestdir, "rootfs-before-failure")),
+                                    "rootfs-before-failure file was not created")
+                self.assertFalse(os.path.isfile(os.path.join(hosttestdir, "rootfs-after-failure")),
+                                    "rootfs-after-failure file was created")
+

2.16.1
---
 meta/lib/oe/patch.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
index f02dee8d27d..bfa7d218792 100644
@@ -36,6 +36,22 @@ def runcmd(args, dir = None):
         (exitstatus, output) = oe.utils.getstatusoutput(cmd)
         if exitstatus != 0:
             raise CmdError(cmd, exitstatus >> 8, output)
+        if " fuzz " in output:
+            bb.warn("""
+Some of the context lines in patches were ignored. This can lead to incorrectly applied patches.
+The context lines in the patches can be updated with devtool:
+
+    devtool modify <recipe>
+    devtool finish --force-patch-refresh <recipe> <layer_path>
+
+Then the updated patches and the source tree (in devtool's workspace)
+should be reviewed to make sure the patches apply in the correct place
+and don't introduce duplicate lines (which can, and does happen
+when some of the context is ignored). Further information:
+http://lists.openembedded.org/pipermail/openembedded-core/2018-March/148675.html
+https://bugzilla.yoctoproject.org/show_bug.cgi?id=10450
+Details:
+{}""".format(output))
         return output
 
     finally:

2.16.1
---
 meta/lib/oe/patch.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
index f02dee8d27d..bfa7d218792 100644
@@ -36,6 +36,22 @@ def runcmd(args, dir = None):
         (exitstatus, output) = oe.utils.getstatusoutput(cmd)
         if exitstatus != 0:
             raise CmdError(cmd, exitstatus >> 8, output)
+        if " fuzz " in output:
+            bb.warn("""
+Some of the context lines in patches were ignored. This can lead to

+The context lines in the patches can be updated with devtool:
+
+    devtool modify <recipe>
+    devtool finish --force-patch-refresh <recipe> <layer_path>
+
+Then the updated patches and the source tree (in devtool's workspace)
+should be reviewed to make sure the patches apply in the correct place
+and don't introduce duplicate lines (which can, and does happen
+when some of the context is ignored). Further information:
+http://lists.openembedded.org/pipermail/openembedded-

+https://bugzilla.yoctoproject.org/show_bug.cgi?id=10450
+Details:
+{}""".format(output))
         return output
 
     finally:
--


package.bbclass: run pre/post installation/removal scriptlets using sh -e

Alexander Kanavin

Alexander Kanavin

Alexander Kanavin

Alexander Kanavin

Martin Jansa

Alexander Kanavin

Martin Jansa

Alexander Kanavin

Khem Raj