[PATCH 0/7] Fix first boot deferral and postinst error reporting

[PATCH 0/7] Fix first boot deferral and postinst error reporting

[Alexander Kanavin]

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
      ....
}

The following changes since commit 0c537554fc2cf58a9cbfeaf39ed918c607483dce:

  testimage: enable gi test suite (2018-01-29 10:31:58 +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 (7):
  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
  rootfs.py: change the notice about deferring scripts to first boot
    from warning to a note

 .../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

-- 
2.15.1

[PATCH 1/7] postinst_intercepts: add a delay_to_first_boot hook

[Alexander Kanavin]

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.

Signed-off-by: Alexander Kanavin <alexander.kanavin@linux.intel.com>
---
 scripts/postinst-intercepts/delay_to_first_boot | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 scripts/postinst-intercepts/delay_to_first_boot

diff --git a/scripts/postinst-intercepts/delay_to_first_boot b/scripts/postinst-intercepts/delay_to_first_boot
new file mode 100644
index 00000000000..ecdbef95dde
--- /dev/null
+++ b/scripts/postinst-intercepts/delay_to_first_boot
@@ -0,0 +1,2 @@
+#!/bin/sh
+exit 1
-- 
2.15.1

[PATCH 2/7] meta/lib/oe/rootfs.py: separate first boot deferral logic into a separate function

[Alexander Kanavin]

Signed-off-by: Alexander Kanavin <alexander.kanavin@linux.intel.com>
---
 meta/lib/oe/rootfs.py | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index 754ef563abe..d3ec8a56c48 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -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'):
-- 
2.15.1

[PATCH 3/7] meta/lib/oe/rootfs.py: do not execute defer_to_first_boot when processing postinst_intercept hooks

[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.

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

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index d3ec8a56c48..8bf77aea492 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -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:
-- 
2.15.1

[PATCH 4/7] package.bbclass: add support for pkg_postinst_ontarget()

[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.

Signed-off-by: Alexander Kanavin <alexander.kanavin@linux.intel.com>
---
 meta-selftest/recipes-test/postinst/postinst_1.0.bb | 14 ++------------
 meta/classes/package.bbclass                        | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/meta-selftest/recipes-test/postinst/postinst_1.0.bb b/meta-selftest/recipes-test/postinst/postinst_1.0.bb
index 112aa08c80f..d4bab6dcc22 100644
--- a/meta-selftest/recipes-test/postinst/postinst_1.0.bb
+++ b/meta-selftest/recipes-test/postinst/postinst_1.0.bb
@@ -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
diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 7dc759699f4..6a7f35a3e78 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -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')
-- 
2.15.1

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

[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.

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

diff --git a/meta/lib/oe/package_manager.py b/meta/lib/oe/package_manager.py
index 2a07f0e39ad..f7e013437c9 100644
--- a/meta/lib/oe/package_manager.py
+++ b/meta/lib/oe/package_manager.py
@@ -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)
 
-- 
2.15.1

[PATCH 6/7] package_rpm.bbclass: run pre/post installation 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. 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.

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

diff --git a/meta/classes/package_rpm.bbclass b/meta/classes/package_rpm.bbclass
index af64ef62c58..e26b2ad6625 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' % 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('')
@@ -564,12 +564,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('')
-- 
2.15.1

[PATCH 7/7] rootfs.py: change the notice about deferring scripts to first boot from warning to a note

[Alexander Kanavin]

Now that the first boot deferral needs to be requested explicitly,
it's not really something to be concerned about.

Signed-off-by: Alexander Kanavin <alexander.kanavin@linux.intel.com>
---
 meta/lib/oe/rootfs.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index 8bf77aea492..bf2aea2b25a 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -303,7 +303,7 @@ class Rootfs(object, metaclass=ABCMeta):
                     break
 
             if registered_pkgs is not None:
-                bb.warn("The postinstalls for the following packages "
+                bb.note("The postinstalls for the following packages "
                         "will be postponed for first boot: %s" %
                         registered_pkgs)
 
-- 
2.15.1

Re: [PATCH 0/7] Fix first boot deferral and postinst error reporting

[Diaz de Grenu, Jose]

I fully agree with this patchset. The old way of defining seems a bit hacky. Also even if you follow the documented way to create postinstallation scripts [1] you still get noisy warnings which seem like a problem.

Thanks for the effort!

[1] http://www.yoctoproject.org/docs/2.4/mega-manual/mega-manual.html#new-recipe-post-installation-scripts 

-----Original Message-----
From: openembedded-core-bounces@lists.openembedded.org [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of Alexander Kanavin
Sent: Monday, January 29, 2018 1:01 PM
To: openembedded-core@lists.openembedded.org
Subject: [OE-core] [PATCH 0/7] 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
      ....
}

The following changes since commit 0c537554fc2cf58a9cbfeaf39ed918c607483dce:

  testimage: enable gi test suite (2018-01-29 10:31:58 +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 (7):
  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
  rootfs.py: change the notice about deferring scripts to first boot
    from warning to a note

 .../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

--
2.15.1

--
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

✗ patchtest: failure for Fix first boot deferral and postinst error reporting

[Patchwork]

== Series Details ==

Series: Fix first boot deferral and postinst error reporting
Revision: 1
URL   : https://patchwork.openembedded.org/series/10741/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Patch            [3/7] meta/lib/oe/rootfs.py: do not execute defer_to_first_boot when processing postinst_intercept hooks
 Issue             Commit shortlog is too long [test_shortlog_length] 
  Suggested fix    Edit shortlog so that it is 90 characters or less (currently 98 characters)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe