diff mbox series

[v3,1/3] useradd.bbclass: Fix order of postinst-useradd-*

Message ID 07eb596dd4e8cfc319add777a3069d6e50d33e56.1708694024.git.pidge@baylibre.com
State Accepted, archived
Commit 322ef726132a47d977d2c6ee41de5358f1e85994
Headers show
Series Useradd postinstall fixes and tests | expand

Commit Message

Eilís 'pidge' Ní Fhlannagáin Feb. 23, 2024, 1:25 p.m. UTC
From: Piotr Łobacz <p.lobacz@welotec.com>

postinst-useradd-* haven't been running in order of dependency.

This patch is reworked from Piotr Łobacz's patch and fixes:

https://bugzilla.yoctoproject.org/show_bug.cgi?id=15084
https://bugzilla.yoctoproject.org/show_bug.cgi?id=13904

basepasswd_sysroot_postinst in base-passwd can install postinst-useradd-*
scripts with any order. Sometimes this means, for example a useradd postinst
will attempt to run without the corresponding group postinst causing errors.
This patch ensures that we first run groupadd, then useradd and then
group membership.

Signed-off-by: Eilís 'pidge' Ní Fhlannagáin <pidge@baylibre.com>
Signed-off-by: Piotr Łobacz <p.lobacz@welotec.com>
Signed-off-by: Jan Górski <j.gorski@welotec.com>
---
 meta/classes-global/staging.bbclass |  4 +-
 meta/classes/useradd.bbclass        | 67 ++++++++++++++++++-----------
 2 files changed, 43 insertions(+), 28 deletions(-)

Comments

Richard Purdie Feb. 24, 2024, 12:11 p.m. UTC | #1
On Fri, 2024-02-23 at 13:25 +0000, Eilís 'pidge' Ní Fhlannagáin wrote:
>  python useradd_sysroot_sstate () {
> -    scriptfile = None
> -    task = d.getVar("BB_CURRENTTASK")
> -    if task == "package_setscene":
> -        bb.build.exec_func("useradd_sysroot", d)
> -    elif task == "prepare_recipe_sysroot":
> -        # Used to update this recipe's own sysroot so the user/groups are available to do_install
> -
> -        # If do_populate_sysroot is triggered and we write the file here, there would be an overlapping
> -        # files. See usergrouptests.UserGroupTests.test_add_task_between_p_sysroot_and_package
> -        scriptfile = d.expand("${RECIPE_SYSROOT}${bindir}/postinst-useradd-${PN}-recipedebug")
> -
> -        bb.build.exec_func("useradd_sysroot", d)
> -    elif task == "populate_sysroot":
> -        # Used when installed in dependent task sysroots
> -        scriptfile = d.expand("${SYSROOT_DESTDIR}${bindir}/postinst-useradd-${PN}")
> -
> -    if scriptfile:
> -        bb.utils.mkdirhier(os.path.dirname(scriptfile))
> -        with open(scriptfile, 'w') as script:
> -            script.write("#!/bin/sh -e\n")
> -            bb.data.emit_func("useradd_sysroot", script, d)
> -            script.write("useradd_sysroot\n")
> -        os.chmod(scriptfile, 0o755)
> +    for type, sort_prefix in [("group", "01"), ("user", "02"), ("groupmems", "03")]:
> +        scriptfile = None
> +        task = d.getVar("BB_CURRENTTASK")
> +        if task == "package_setscene":
> +            bb.build.exec_func(f"{type}add_sysroot", d)
> +        elif task == "prepare_recipe_sysroot":
> +            # Used to update this recipe's own sysroot so the user/groups are available to do_install
> +            scriptfile = d.expand("${RECIPE_SYSROOT}${bindir}/" f"postinst-useradd-{sort_prefix}{type}" "-${PN}")
> +            bb.build.exec_func(f"{type}add_sysroot", d)
> +        elif task == "populate_sysroot":
> +            # Used when installed in dependent task sysroots
> +            scriptfile = d.expand("${SYSROOT_DESTDIR}${bindir}/" f"postinst-useradd-{sort_prefix}{type}" "-${PN}")
> +    
> +        if scriptfile:
> +            bb.utils.mkdirhier(os.path.dirname(scriptfile))
> +            with open(scriptfile, 'w') as script:
> +                script.write("#!/bin/sh\n")
> +                bb.data.emit_func(f"{type}add_sysroot", script, d)
> +                script.write(f"{type}add_sysroot\n")
> +            os.chmod(scriptfile, 0o755)
>  }

This change has all kinds of hidden issues and changes. The -e change
isn't mentioned but probably a good idea. The addition of trailing
whitespace is not great but harmless. This is on top of the previous
issues the series had.

What really upsets me though is that this reverted:

https://git.yoctoproject.org/poky/commit/?id=4bb222e0d71a4cb159b8a4f1a90b65b1af32ac10

which thankfully we have a test for, which then failed:

https://autobuilder.yoctoproject.org/typhoon/#/builders/80/builds/6395/steps/14/logs/stdio

This amounts to a lack of attention to detail and really frustrates me.

I'll queue something in -next to fix this up as I do want to fix the
ordering issue and we are now close with that but it will make me worry
a lot about future patches.

Richard
diff mbox series

Patch

diff --git a/meta/classes-global/staging.bbclass b/meta/classes-global/staging.bbclass
index ab3e1d71b52..3678a1b4415 100644
--- a/meta/classes-global/staging.bbclass
+++ b/meta/classes-global/staging.bbclass
@@ -245,7 +245,7 @@  def staging_populate_sysroot_dir(targetsysroot, nativesysroot, native, d):
                         continue
 
     staging_processfixme(fixme, targetdir, targetsysroot, nativesysroot, d)
-    for p in postinsts:
+    for p in sorted(postinsts):
         bb.note("Running postinst {}, output:\n{}".format(p, subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)))
 
 #
@@ -629,7 +629,7 @@  python extend_recipe_sysroot() {
     for f in fixme:
         staging_processfixme(fixme[f], f, recipesysroot, recipesysrootnative, d)
 
-    for p in postinsts:
+    for p in sorted(postinsts):
         bb.note("Running postinst {}, output:\n{}".format(p, subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)))
 
     for dep in manifests:
diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass
index f0ec0809203..198be82228b 100644
--- a/meta/classes/useradd.bbclass
+++ b/meta/classes/useradd.bbclass
@@ -103,6 +103,18 @@  fi
 }
 
 useradd_sysroot () {
+	user_group_groupmems_add_sysroot user
+}
+
+groupadd_sysroot () {
+	user_group_groupmems_add_sysroot group
+}
+
+groupmemsadd_sysroot () {
+	user_group_groupmems_add_sysroot groupmems
+}
+
+user_group_groupmems_add_sysroot () {
 	# Pseudo may (do_prepare_recipe_sysroot) or may not (do_populate_sysroot_setscene) be running 
 	# at this point so we're explicit about the environment so pseudo can load if 
 	# not already present.
@@ -131,9 +143,15 @@  useradd_sysroot () {
 	fi
 
 	# Add groups and users defined for all recipe packages
-	GROUPADD_PARAM="${@get_all_cmd_params(d, 'groupadd')}"
-	USERADD_PARAM="${@get_all_cmd_params(d, 'useradd')}"
-	GROUPMEMS_PARAM="${@get_all_cmd_params(d, 'groupmems')}"
+	if test "$1" = "group"; then
+		GROUPADD_PARAM="${@get_all_cmd_params(d, 'groupadd')}"
+	elif test "$1" = "user"; then
+		USERADD_PARAM="${@get_all_cmd_params(d, 'useradd')}"
+	elif test "$1" = "groupmems"; then
+		GROUPMEMS_PARAM="${@get_all_cmd_params(d, 'groupmems')}"
+	elif test "x$1" = "x"; then
+		bbwarn "missing type of passwd db action"
+	fi
 
 	# Tell the system to use the environment vars
 	UA_SYSROOT=1
@@ -148,29 +166,26 @@  useradd_sysroot () {
 EXTRA_STAGING_FIXMES += "PSEUDO_SYSROOT PSEUDO_LOCALSTATEDIR LOGFIFO"
 
 python useradd_sysroot_sstate () {
-    scriptfile = None
-    task = d.getVar("BB_CURRENTTASK")
-    if task == "package_setscene":
-        bb.build.exec_func("useradd_sysroot", d)
-    elif task == "prepare_recipe_sysroot":
-        # Used to update this recipe's own sysroot so the user/groups are available to do_install
-
-        # If do_populate_sysroot is triggered and we write the file here, there would be an overlapping
-        # files. See usergrouptests.UserGroupTests.test_add_task_between_p_sysroot_and_package
-        scriptfile = d.expand("${RECIPE_SYSROOT}${bindir}/postinst-useradd-${PN}-recipedebug")
-
-        bb.build.exec_func("useradd_sysroot", d)
-    elif task == "populate_sysroot":
-        # Used when installed in dependent task sysroots
-        scriptfile = d.expand("${SYSROOT_DESTDIR}${bindir}/postinst-useradd-${PN}")
-
-    if scriptfile:
-        bb.utils.mkdirhier(os.path.dirname(scriptfile))
-        with open(scriptfile, 'w') as script:
-            script.write("#!/bin/sh -e\n")
-            bb.data.emit_func("useradd_sysroot", script, d)
-            script.write("useradd_sysroot\n")
-        os.chmod(scriptfile, 0o755)
+    for type, sort_prefix in [("group", "01"), ("user", "02"), ("groupmems", "03")]:
+        scriptfile = None
+        task = d.getVar("BB_CURRENTTASK")
+        if task == "package_setscene":
+            bb.build.exec_func(f"{type}add_sysroot", d)
+        elif task == "prepare_recipe_sysroot":
+            # Used to update this recipe's own sysroot so the user/groups are available to do_install
+            scriptfile = d.expand("${RECIPE_SYSROOT}${bindir}/" f"postinst-useradd-{sort_prefix}{type}" "-${PN}")
+            bb.build.exec_func(f"{type}add_sysroot", d)
+        elif task == "populate_sysroot":
+            # Used when installed in dependent task sysroots
+            scriptfile = d.expand("${SYSROOT_DESTDIR}${bindir}/" f"postinst-useradd-{sort_prefix}{type}" "-${PN}")
+    
+        if scriptfile:
+            bb.utils.mkdirhier(os.path.dirname(scriptfile))
+            with open(scriptfile, 'w') as script:
+                script.write("#!/bin/sh\n")
+                bb.data.emit_func(f"{type}add_sysroot", script, d)
+                script.write(f"{type}add_sysroot\n")
+            os.chmod(scriptfile, 0o755)
 }
 
 do_prepare_recipe_sysroot[postfuncs] += "${SYSROOTFUNC}"