[PATCH 1/3] package.bbclass: run pre/post installation/removal scriptlets using sh -e

[PATCH 1/3] package.bbclass: run pre/post installation/removal scriptlets using sh -e

[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 without any visible warnings or errors.

This was previously done only for rpm packages; this patch replaces
the rpm-specific tweak with one that works for all package types.

Signed-off-by: Alexander Kanavin <alexander.kanavin@linux.intel.com>
---
 meta/classes/package.bbclass     | 12 ++++++++++++
 meta/classes/package_rpm.bbclass |  8 ++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 6a7f35a3e78..607b548d08d 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -1339,6 +1339,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
@@ -1435,6 +1446,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')
diff --git a/meta/classes/package_rpm.bbclass b/meta/classes/package_rpm.bbclass
index e26b2ad6625..af64ef62c58 100644
--- a/meta/classes/package_rpm.bbclass
+++ b/meta/classes/package_rpm.bbclass
@@ -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.15.1

[PATCH 2/3] meta/lib/oe/package_manager.py: warn about failing scriptlets for all package types

[Alexander Kanavin]

Previously this was done only for rpm packages; now also ipk/deb scriptlet
failures are reported.

In the future this will become a hard error, but it can't yet happen
due to the legacy 'exit 1' way of deferring scriptlet execution to first boot which
needs a deprecation period.

Signed-off-by: Alexander Kanavin <alexander.kanavin@linux.intel.com>
---
 meta/lib/oe/package_manager.py | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/meta/lib/oe/package_manager.py b/meta/lib/oe/package_manager.py
index f7e013437c9..1ae31f8e744 100644
--- a/meta/lib/oe/package_manager.py
+++ b/meta/lib/oe/package_manager.py
@@ -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):
@@ -605,8 +610,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)
 
@@ -1068,6 +1072,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.note)[attempt_only]("Unable to install packages. "
                                               "Command '%s' returned %d:\n%s" %
@@ -1331,9 +1342,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.15.1

[PATCH 3/3] oe-selftest: add a test for failing package post-installation scriptlets

[Alexander Kanavin]

The test runs a scriptlet that has an intentionally failing command in the middle
and checks for two things:
1) that bitbake does warn the user about the failure
2) that scriptlet execution stops at that point.

The test is run for all three package types: rpm, deb, ipk.

Signed-off-by: Alexander Kanavin <alexander.kanavin@linux.intel.com>
---
 .../recipes-test/postinst/postinst_1.0.bb          | 14 +++++++-
 meta/lib/oeqa/selftest/cases/runtime_test.py       | 37 ++++++++++++++++++++++
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/meta-selftest/recipes-test/postinst/postinst_1.0.bb b/meta-selftest/recipes-test/postinst/postinst_1.0.bb
index d4bab6dcc22..913bfabf89e 100644
--- a/meta-selftest/recipes-test/postinst/postinst_1.0.bb
+++ b/meta-selftest/recipes-test/postinst/postinst_1.0.bb
@@ -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
+}
diff --git a/meta/lib/oeqa/selftest/cases/runtime_test.py b/meta/lib/oeqa/selftest/cases/runtime_test.py
index 1c69255b568..9c9b4b34111 100644
--- a/meta/lib/oeqa/selftest/cases/runtime_test.py
+++ b/meta/lib/oeqa/selftest/cases/runtime_test.py
@@ -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 <alexander.kanavin@intel.com>
+        """
+
+        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.15.1

Re: [PATCH 1/3] package.bbclass: run pre/post installation/removal scriptlets using sh -e

[Alexander Kanavin]

On 02/08/2018 08:37 AM, Martin Hundebøll wrote:
> I guess there is no support for non-shell scriptlets? E.g. RPM supports 
> python:
> https://stackoverflow.com/questions/21590014/rpm-post-install-execute-with-python 

Nope, there is no support for it. Besides, that would mean the recipe 
would no longer work with deb or ipk as package types.

I hardly see an issue though: just write a shell one-liner that invokes 
the actual script.


Alex