[PATCH v3 0/3] Useradd postinstall fixes and tests

[PATCH v3 0/3] Useradd postinstall fixes and tests

[Eilís 'pidge' Ní Fhlannagáin]

These commits fix #13904 and #15084, adds a test for it and adds a minor fix for
the creategroup tests

Eilís 'pidge' Ní Fhlannagáin (2):
  selftest-users: Convoluted selftest for USERADD_DEPENDS
  creategroup*: Remove coreutils-native as a DEPENDS

Piotr Łobacz (1):
  useradd.bbclass: Fix order of postinst-useradd-*

 .../selftest-users/acreategroup.bb            | 32 +++++++++
 .../selftest-users/bcreategroup.bb            | 37 ++++++++++
 .../selftest-users/ccreategroup.bb            | 34 ++++++++++
 .../selftest-users/creategroup1.bb            |  2 -
 .../selftest-users/creategroup2.bb            |  1 -
 .../selftest-users/dcreategroup.bb            | 33 +++++++++
 meta/classes-global/staging.bbclass           |  4 +-
 meta/classes/useradd.bbclass                  | 67 ++++++++++++-------
 .../lib/oeqa/selftest/cases/usergrouptests.py |  4 ++
 9 files changed, 183 insertions(+), 31 deletions(-)
 create mode 100644 meta-selftest/recipes-test/selftest-users/acreategroup.bb
 create mode 100644 meta-selftest/recipes-test/selftest-users/bcreategroup.bb
 create mode 100644 meta-selftest/recipes-test/selftest-users/ccreategroup.bb
 create mode 100644 meta-selftest/recipes-test/selftest-users/dcreategroup.bb

-- 
2.34.1

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

[Eilís 'pidge' Ní Fhlannagáin]

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(-)

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}"
-- 
2.34.1

[PATCH v3 2/3] selftest-users: Convoluted selftest for USERADD_DEPENDS

[Eilís 'pidge' Ní Fhlannagáin]

This adds a test for 13904's fix by creating a convoluted set of recipes
with USERADD_DEPENDS in non-alpha order.

Signed-off-by: Eilís 'pidge' Ní Fhlannagáin <pidge@baylibre.com>
---
 .../selftest-users/acreategroup.bb            | 32 ++++++++++++++++
 .../selftest-users/bcreategroup.bb            | 37 +++++++++++++++++++
 .../selftest-users/ccreategroup.bb            | 34 +++++++++++++++++
 .../selftest-users/dcreategroup.bb            | 33 +++++++++++++++++
 .../lib/oeqa/selftest/cases/usergrouptests.py |  4 ++
 5 files changed, 140 insertions(+)
 create mode 100644 meta-selftest/recipes-test/selftest-users/acreategroup.bb
 create mode 100644 meta-selftest/recipes-test/selftest-users/bcreategroup.bb
 create mode 100644 meta-selftest/recipes-test/selftest-users/ccreategroup.bb
 create mode 100644 meta-selftest/recipes-test/selftest-users/dcreategroup.bb

diff --git a/meta-selftest/recipes-test/selftest-users/acreategroup.bb b/meta-selftest/recipes-test/selftest-users/acreategroup.bb
new file mode 100644
index 00000000000..66ed5695a26
--- /dev/null
+++ b/meta-selftest/recipes-test/selftest-users/acreategroup.bb
@@ -0,0 +1,32 @@
+SUMMARY = "creategroup_a"
+LIC_FILES_CHKSUM = "file://${COMMON_LICENSE_DIR}/MIT;md5=0835ade698e0bcf8506ecda2f7b4f302"
+
+LICENSE = "MIT"
+
+S = "${WORKDIR}"
+
+EXCLUDE_FROM_WORLD="1"
+
+inherit useradd allarch
+
+USERADD_PACKAGES = "${PN}"
+USERADD_PARAM:${PN} = "-u 5560 --gid a_group a_user"
+GROUPADD_PARAM:${PN} = "-r a_group"
+
+TESTDIR = "${D}${sysconfdir}/creategroup"
+
+do_install() {
+	install -d   ${TESTDIR}
+	install -d   ${TESTDIR}/dir
+	touch        ${TESTDIR}/file
+	ln -s ./file ${TESTDIR}/symlink
+	install -d   ${TESTDIR}/fifotest
+	mkfifo       ${TESTDIR}/fifotest/fifo
+
+	chown    a_user:a_group ${TESTDIR}/file
+	chown -R a_user:a_group ${TESTDIR}/dir
+	chown -h a_user:a_group ${TESTDIR}/symlink
+	chown -R a_user:a_group ${TESTDIR}/fifotest
+}
+
+FILES:${PN} = "${sysconfdir}/creategroup/*"
diff --git a/meta-selftest/recipes-test/selftest-users/bcreategroup.bb b/meta-selftest/recipes-test/selftest-users/bcreategroup.bb
new file mode 100644
index 00000000000..c4844dd0da0
--- /dev/null
+++ b/meta-selftest/recipes-test/selftest-users/bcreategroup.bb
@@ -0,0 +1,37 @@
+SUMMARY = "creategroup_b"
+LIC_FILES_CHKSUM = "file://${COMMON_LICENSE_DIR}/MIT;md5=0835ade698e0bcf8506ecda2f7b4f302"
+
+LICENSE = "MIT"
+
+# This recipe requires a and c. C requires A. Reverse alpha.
+
+USERADD_DEPENDS = "acreategroup ccreategroup"
+
+S = "${WORKDIR}"
+
+EXCLUDE_FROM_WORLD="1"
+
+inherit useradd allarch
+
+USERADD_PACKAGES = "${PN}"
+USERADD_PARAM:${PN} = "-u 5561 -g b_group -G a_group,c_group b_user "
+GROUPADD_PARAM:${PN} = "-r b_group"
+
+TESTDIR = "${D}${sysconfdir}/creategroup"
+
+do_install() {
+	install -d   ${TESTDIR}
+	install -d   ${TESTDIR}/dir
+	touch        ${TESTDIR}/file
+	ln -s ./file ${TESTDIR}/symlink
+	install -d   ${TESTDIR}/fifotest
+	mkfifo       ${TESTDIR}/fifotest/fifo
+
+	chown    a_user:a_group ${TESTDIR}/file
+	chown -R c_user:c_group ${TESTDIR}/dir
+	chown -h a_user:a_group ${TESTDIR}/symlink
+	chown -R b_user:b_group ${TESTDIR}/fifotest
+}
+
+FILES:${PN} = "${sysconfdir}/creategroup/*"
+
diff --git a/meta-selftest/recipes-test/selftest-users/ccreategroup.bb b/meta-selftest/recipes-test/selftest-users/ccreategroup.bb
new file mode 100644
index 00000000000..021b1ebbf77
--- /dev/null
+++ b/meta-selftest/recipes-test/selftest-users/ccreategroup.bb
@@ -0,0 +1,34 @@
+SUMMARY = "creategroup_c"
+LIC_FILES_CHKSUM = "file://${COMMON_LICENSE_DIR}/MIT;md5=0835ade698e0bcf8506ecda2f7b4f302"
+
+LICENSE = "MIT"
+
+USERADD_DEPENDS = "acreategroup"
+
+S = "${WORKDIR}"
+
+EXCLUDE_FROM_WORLD="1"
+
+inherit useradd allarch
+
+USERADD_PACKAGES = "${PN}"
+USERADD_PARAM:${PN} = "-u 5563 --gid c_group -G a_group c_user"
+GROUPADD_PARAM:${PN} = "-r c_group"
+
+TESTDIR = "${D}${sysconfdir}/creategroup"
+
+do_install() {
+	install -d   ${TESTDIR}
+	install -d   ${TESTDIR}/dir
+	touch        ${TESTDIR}/file
+	ln -s ./file ${TESTDIR}/symlink
+	install -d   ${TESTDIR}/fifotest
+	mkfifo       ${TESTDIR}/fifotest/fifo
+
+	chown    c_user:c_group ${TESTDIR}/file
+	chown -R c_user:c_group ${TESTDIR}/dir
+	chown -h c_user:c_group ${TESTDIR}/symlink
+	chown -R c_user:c_group ${TESTDIR}/fifotest
+}
+
+FILES:${PN} = "${sysconfdir}/creategroup/*"
diff --git a/meta-selftest/recipes-test/selftest-users/dcreategroup.bb b/meta-selftest/recipes-test/selftest-users/dcreategroup.bb
new file mode 100644
index 00000000000..b96ca92a160
--- /dev/null
+++ b/meta-selftest/recipes-test/selftest-users/dcreategroup.bb
@@ -0,0 +1,33 @@
+SUMMARY = "creategroup_d"
+LIC_FILES_CHKSUM = "file://${COMMON_LICENSE_DIR}/MIT;md5=0835ade698e0bcf8506ecda2f7b4f302"
+
+LICENSE = "MIT"
+
+USERADD_DEPENDS = "bcreategroup"
+
+S = "${WORKDIR}"
+
+EXCLUDE_FROM_WORLD="1"
+
+inherit useradd allarch
+
+USERADD_PACKAGES = "${PN}"
+USERADD_PARAM:${PN} = "-u 5564 -g d_group -G b_group d_user "
+GROUPADD_PARAM:${PN} = "-r d_group"
+
+TESTDIR = "${D}${sysconfdir}/creategroup"
+
+do_install() {
+	install -d   ${TESTDIR}
+	install -d   ${TESTDIR}/dir
+	touch        ${TESTDIR}/file
+	ln -s ./file ${TESTDIR}/symlink
+	install -d   ${TESTDIR}/fifotest
+	mkfifo       ${TESTDIR}/fifotest/fifo
+
+	chown    d_user:d_group ${TESTDIR}/file
+	chown -R d_user:b_group ${TESTDIR}/dir
+	chown -h d_user:d_group ${TESTDIR}/symlink
+	chown -R d_user:b_group ${TESTDIR}/fifotest
+}
+
diff --git a/meta/lib/oeqa/selftest/cases/usergrouptests.py b/meta/lib/oeqa/selftest/cases/usergrouptests.py
index a331ca9f2c4..3c59b0f2908 100644
--- a/meta/lib/oeqa/selftest/cases/usergrouptests.py
+++ b/meta/lib/oeqa/selftest/cases/usergrouptests.py
@@ -22,6 +22,10 @@ class UserGroupTests(OESelftestTestCase):
         # Test for YOCTO #14961
         self.assertTrue(bitbake('useraddbadtask -C fetch'))
 
+    def test_postinst_order(self):
+        self.logger.info("Building dcreategroup")
+        self.assertTrue(bitbake(' dcreategroup'))
+
     def test_static_useradd_from_dynamic(self):
         metaselftestpath = get_test_layer()
         self.logger.info("Building core-image-minimal to generate passwd/group file")
-- 
2.34.1

[PATCH v3 3/3] creategroup*: Remove coreutils-native as a DEPENDS

[Eilís 'pidge' Ní Fhlannagáin]

There isn't any reason for coreutils-native as a DEPENDS, so
remove it to speed up tests.

Signed-off-by: Eilís 'pidge' Ní Fhlannagáin <pidge@baylibre.com>
---
 meta-selftest/recipes-test/selftest-users/creategroup1.bb | 2 --
 meta-selftest/recipes-test/selftest-users/creategroup2.bb | 1 -
 2 files changed, 3 deletions(-)

diff --git a/meta-selftest/recipes-test/selftest-users/creategroup1.bb b/meta-selftest/recipes-test/selftest-users/creategroup1.bb
index ebbbfaa83d3..afd23ed1eea 100644
--- a/meta-selftest/recipes-test/selftest-users/creategroup1.bb
+++ b/meta-selftest/recipes-test/selftest-users/creategroup1.bb
@@ -3,8 +3,6 @@ LIC_FILES_CHKSUM = "file://${COMMON_LICENSE_DIR}/MIT;md5=0835ade698e0bcf8506ecda
 
 LICENSE = "MIT"
 
-DEPENDS = "coreutils-native"
-
 S = "${WORKDIR}"
 
 inherit useradd allarch
diff --git a/meta-selftest/recipes-test/selftest-users/creategroup2.bb b/meta-selftest/recipes-test/selftest-users/creategroup2.bb
index ef697f09b4d..f776f43aedf 100644
--- a/meta-selftest/recipes-test/selftest-users/creategroup2.bb
+++ b/meta-selftest/recipes-test/selftest-users/creategroup2.bb
@@ -3,7 +3,6 @@ LIC_FILES_CHKSUM = "file://${COMMON_LICENSE_DIR}/MIT;md5=0835ade698e0bcf8506ecda
 
 LICENSE = "MIT"
 
-DEPENDS = "coreutils-native"
 USERADD_DEPENDS = "creategroup1"
 
 S = "${WORKDIR}"
-- 
2.34.1

Re: [OE-core] [PATCH v3 1/3] useradd.bbclass: Fix order of postinst-useradd-*

[Richard Purdie]

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