[PATCH 0/3] Useradd postinstall fixes and tests

[PATCH 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           |  7 +-
 meta/classes/useradd.bbclass                  | 72 ++++++++++++-------
 .../lib/oeqa/selftest/cases/usergrouptests.py |  4 ++
 9 files changed, 189 insertions(+), 33 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 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

It ensures that groups, users and groupmems are all run in the correct order.

Signed-off-by: Eilís 'pidge' Ní Fhlannagáin <pidge@baylibre.com>
---
 meta/classes-global/staging.bbclass |  7 +--
 meta/classes/useradd.bbclass        | 72 ++++++++++++++++++-----------
 2 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/meta/classes-global/staging.bbclass b/meta/classes-global/staging.bbclass
index ab3e1d71b5..6c21510377 100644
--- a/meta/classes-global/staging.bbclass
+++ b/meta/classes-global/staging.bbclass
@@ -245,8 +245,9 @@ def staging_populate_sysroot_dir(targetsysroot, nativesysroot, native, d):
                         continue
 
     staging_processfixme(fixme, targetdir, targetsysroot, nativesysroot, d)
-    for p in postinsts:
-        bb.note("Running postinst {}, output:\n{}".format(p, subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)))
+    bb.warn(sorted(postinsts))
+    for p in sorted(postinsts):
+        bb.warn("Running postinst {}, output:\n{}".format(p, subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)))
 
 #
 # Manifests here are complicated. The main sysroot area has the unpacked sstate
@@ -629,7 +630,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 a58f8287c9..924eeadf9c 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.
@@ -130,10 +142,19 @@ useradd_sysroot () {
 		exit 0
 	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')}"
+ 	# Add groups and users defined for all recipe packages
+	if test "x$1" = "xgroup"; then
+		GROUPADD_PARAM="${@get_all_cmd_params(d, 'groupadd')}"
+	fi
+	if test "x$1" = "xuser"; then
+		USERADD_PARAM="${@get_all_cmd_params(d, 'useradd')}"
+	fi
+	if test "x$1" = "xgroupmems"; then
+		GROUPMEMS_PARAM="${@get_all_cmd_params(d, 'groupmems')}"
+	fi
+	if 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 +169,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 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 0000000000..66ed5695a2
--- /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 0000000000..c4844dd0da
--- /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 0000000000..021b1ebbf7
--- /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 0000000000..b96ca92a16
--- /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 a331ca9f2c..3c59b0f290 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 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 ebbbfaa83d..afd23ed1ee 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 ef697f09b4..f776f43aed 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 1/3] useradd.bbclass: Fix order of postinst-useradd-*

[Richard Purdie]

On Thu, 2024-02-22 at 14:53 +0000, Eilís 'pidge' Ní Fhlannagáin wrote:
> 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
> 
> It ensures that groups, users and groupmems are all run in the correct order.

I think this needs a little bit more explanation in the commit message.
What is the issue, why do we need to solve it like this etc.

> 
> Signed-off-by: Eilís 'pidge' Ní Fhlannagáin <pidge@baylibre.com>
> ---
>  meta/classes-global/staging.bbclass |  7 +--
>  meta/classes/useradd.bbclass        | 72 ++++++++++++++++++-----------
>  2 files changed, 49 insertions(+), 30 deletions(-)
> 
> diff --git a/meta/classes-global/staging.bbclass b/meta/classes-global/staging.bbclass
> index ab3e1d71b5..6c21510377 100644
> --- a/meta/classes-global/staging.bbclass
> +++ b/meta/classes-global/staging.bbclass
> @@ -245,8 +245,9 @@ def staging_populate_sysroot_dir(targetsysroot, nativesysroot, native, d):
>                          continue
>  
>      staging_processfixme(fixme, targetdir, targetsysroot, nativesysroot, d)
> -    for p in postinsts:
> -        bb.note("Running postinst {}, output:\n{}".format(p, subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)))


> +    bb.warn(sorted(postinsts))

Come on, please! :)


> +    for p in sorted(postinsts):
> +        bb.warn("Running postinst {}, output:\n{}".format(p, subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)))
>  
>  #
>  # Manifests here are complicated. The main sysroot area has the unpacked sstate
> @@ -629,7 +630,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 a58f8287c9..924eeadf9c 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.
> @@ -130,10 +142,19 @@ useradd_sysroot () {
>  		exit 0
>  	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')}"
> + 	# Add groups and users defined for all recipe packages
> +	if test "x$1" = "xgroup"; then
> +		GROUPADD_PARAM="${@get_all_cmd_params(d, 'groupadd')}"
> +	fi
> +	if test "x$1" = "xuser"; then
> +		USERADD_PARAM="${@get_all_cmd_params(d, 'useradd')}"
> +	fi
> +	if test "x$1" = "xgroupmems"; then
> +		GROUPMEMS_PARAM="${@get_all_cmd_params(d, 'groupmems')}"
> +	fi
> +	if test "x$1" = "x"; then
> +		bbwarn "missing type of passwd db action"
> +    fi		

whitespace...



Cheers,

Richard

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

[Piotr Łobacz]

Dnia czwartek, 22 lutego 2024 16:02:47 CET Richard Purdie pisze:
> On Thu, 2024-02-22 at 14:53 +0000, Eilís 'pidge' Ní Fhlannagáin wrote:
> > 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://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla
> > .yoctoproject.org%2Fshow_bug.cgi%3Fid%3D15084&data=05%7C02%7Cp.lobacz%40we
> > lotec.com%7C71e58402c2f2409f56bb08dc33b755b3%7C25111a7f1d5a4c51a4ca7f8e440
> > 11b39%7C0%7C0%7C638442109731703010%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> > wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=%2B
> > U2cg6Esme3QhxIwjKW417DMncM%2FwWjEChZwrBNullA%3D&reserved=0
> > https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzill
> > a.yoctoproject.org%2Fshow_bug.cgi%3Fid%3D13904&data=05%7C02%7Cp.lobacz%40w
> > elotec.com%7C71e58402c2f2409f56bb08dc33b755b3%7C25111a7f1d5a4c51a4ca7f8e44
> > 011b39%7C0%7C0%7C638442109731710513%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
> > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=s3
> > %2B9l0f1cr7JKjcg9R9arzZHAnBPOzOtecWUGj1vBF4%3D&reserved=0
> > 
> > It ensures that groups, users and groupmems are all run in the correct
> > order.
> I think this needs a little bit more explanation in the commit message.
> What is the issue, why do we need to solve it like this etc.

Acctualy, the issue was discovered by me and the patch given by me was created 
by Jan so I think he can put his comments on that. That would be the best I 
think.

> > Signed-off-by: Eilís 'pidge' Ní Fhlannagáin <pidge@baylibre.com>
> > ---
> >  meta/classes-global/staging.bbclass |  7 +--
> >  meta/classes/useradd.bbclass        | 72 ++++++++++++++++++-----------
> >  2 files changed, 49 insertions(+), 30 deletions(-)
> > 
> > diff --git a/meta/classes-global/staging.bbclass
> > b/meta/classes-global/staging.bbclass index ab3e1d71b5..6c21510377 100644
> > --- a/meta/classes-global/staging.bbclass
> > +++ b/meta/classes-global/staging.bbclass
> > @@ -245,8 +245,9 @@ def staging_populate_sysroot_dir(targetsysroot,
> > nativesysroot, native, d): continue
> >  
> >      staging_processfixme(fixme, targetdir, targetsysroot, nativesysroot,
> > d) -    for p in postinsts:
> > -        bb.note("Running postinst {}, output:\n{}".format(p,
> > subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)))
> > 
> > 
> > +    bb.warn(sorted(postinsts))
> 
> Come on, please! :)
> 
> > +    for p in sorted(postinsts):
> > +        bb.warn("Running postinst {}, output:\n{}".format(p,
> > subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT))) 
> >  #
> >  # Manifests here are complicated. The main sysroot area has the unpacked
> > sstate @@ -629,7 +630,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 a58f8287c9..924eeadf9c 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.
> > @@ -130,10 +142,19 @@ useradd_sysroot () {
> >  		exit 0
> >  	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')}"
> > + 	# Add groups and users defined for all recipe packages
> > +	if test "x$1" = "xgroup"; then
> > +		GROUPADD_PARAM="${@get_all_cmd_params(d, 'groupadd')}"
> > +	fi
> > +	if test "x$1" = "xuser"; then
> > +		USERADD_PARAM="${@get_all_cmd_params(d, 'useradd')}"
> > +	fi
> > +	if test "x$1" = "xgroupmems"; then
> > +		GROUPMEMS_PARAM="${@get_all_cmd_params(d, 
'groupmems')}"
> > +	fi
> > +	if test "x$1" = "x"; then
> > +		bbwarn "missing type of passwd db action"
> > +    fi
> 
> whitespace...
> 
> 
> 
> Cheers,
> 
> Richard

BR
Piotr

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

[Jermain Horsman]

Although I did notice the rest of the file uses this syntax,
and I do not believe there are any guidelines specific to this (that I am aware of),
I did want to comment on this.

> +	if test "x$1" = "xgroup"; then

> +	if test "x$1" = "xuser"; then

> +	if test "x$1" = "xgroupmems"; then

As these are quotes variables the added 'x' doesn't really add anything here.

> +	if test "x$1" = "x"; then

Same here, but this could just test for a zero length variable instead using 'test -z "$1"'.

Sincerely,

Jermain Horsman

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

[ChenQi]

1. I can see what this patch mainly does is ensuring groups first, then 
user, then groupmems. But the current useradd_preinst, which is called 
by useradd_sysroot has already ensured this. Is that not working?

2. You're using bb.warn as a method of outputting debugging information, 
which I don't think is appropriate.

Regards,
Qi

On 2/22/24 22:53, Eilís 'pidge' Ní Fhlannagáin wrote:
> 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
>
> It ensures that groups, users and groupmems are all run in the correct order.
>
> Signed-off-by: Eilís 'pidge' Ní Fhlannagáin <pidge@baylibre.com>
> ---
>   meta/classes-global/staging.bbclass |  7 +--
>   meta/classes/useradd.bbclass        | 72 ++++++++++++++++++-----------
>   2 files changed, 49 insertions(+), 30 deletions(-)
>
> diff --git a/meta/classes-global/staging.bbclass b/meta/classes-global/staging.bbclass
> index ab3e1d71b5..6c21510377 100644
> --- a/meta/classes-global/staging.bbclass
> +++ b/meta/classes-global/staging.bbclass
> @@ -245,8 +245,9 @@ def staging_populate_sysroot_dir(targetsysroot, nativesysroot, native, d):
>                           continue
>   
>       staging_processfixme(fixme, targetdir, targetsysroot, nativesysroot, d)
> -    for p in postinsts:
> -        bb.note("Running postinst {}, output:\n{}".format(p, subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)))
> +    bb.warn(sorted(postinsts))
> +    for p in sorted(postinsts):
> +        bb.warn("Running postinst {}, output:\n{}".format(p, subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)))
>   
>   #
>   # Manifests here are complicated. The main sysroot area has the unpacked sstate
> @@ -629,7 +630,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 a58f8287c9..924eeadf9c 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.
> @@ -130,10 +142,19 @@ useradd_sysroot () {
>   		exit 0
>   	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')}"
> + 	# Add groups and users defined for all recipe packages
> +	if test "x$1" = "xgroup"; then
> +		GROUPADD_PARAM="${@get_all_cmd_params(d, 'groupadd')}"
> +	fi
> +	if test "x$1" = "xuser"; then
> +		USERADD_PARAM="${@get_all_cmd_params(d, 'useradd')}"
> +	fi
> +	if test "x$1" = "xgroupmems"; then
> +		GROUPMEMS_PARAM="${@get_all_cmd_params(d, 'groupmems')}"
> +	fi
> +	if 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 +169,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}"
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#196031): https://lists.openembedded.org/g/openembedded-core/message/196031
> Mute This Topic: https://lists.openembedded.org/mt/104509398/7304865
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [Qi.Chen@eng.windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

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

[Piotr Łobacz]

Dnia piątek, 23 lutego 2024 10:36:27 CET Chen Qi via lists.openembedded.org 
pisze:
> 1. I can see what this patch mainly does is ensuring groups first, then
> user, then groupmems. But the current useradd_preinst, which is called
> by useradd_sysroot has already ensured this. Is that not working?

It is working but all postinst scripts are being called alphabetically which 
in some corner cases is causing the failures...

> 2. You're using bb.warn as a method of outputting debugging information,
> which I don't think is appropriate.

> Regards,
> Qi
> 
> On 2/22/24 22:53, Eilís 'pidge' Ní Fhlannagáin wrote:
> > 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://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla
> > .yoctoproject.org%2Fshow_bug.cgi%3Fid%3D15084&data=05%7C02%7Cp.lobacz%40we
> > lotec.com%7C4b378de2bbf8442903fe08dc3452f110%7C25111a7f1d5a4c51a4ca7f8e440
> > 11b39%7C0%7C0%7C638442778043567095%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> > wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=NtJ
> > L1zoUYxbKRErJ9GBH6cx%2F2UMoSFAT53LLQ8XBzYk%3D&reserved=0
> > https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzill
> > a.yoctoproject.org%2Fshow_bug.cgi%3Fid%3D13904&data=05%7C02%7Cp.lobacz%40w
> > elotec.com%7C4b378de2bbf8442903fe08dc3452f110%7C25111a7f1d5a4c51a4ca7f8e44
> > 011b39%7C0%7C0%7C638442778043575031%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
> > AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=wY
> > jvmpaXqsu8GzyGrYJflJA8oNxniQXkDKZ8GxKRnXY%3D&reserved=0
> > 
> > It ensures that groups, users and groupmems are all run in the correct
> > order.
> > 
> > Signed-off-by: Eilís 'pidge' Ní Fhlannagáin <pidge@baylibre.com>
> > ---
> > 
> >   meta/classes-global/staging.bbclass |  7 +--
> >   meta/classes/useradd.bbclass        | 72 ++++++++++++++++++-----------
> >   2 files changed, 49 insertions(+), 30 deletions(-)
> > 
> > diff --git a/meta/classes-global/staging.bbclass
> > b/meta/classes-global/staging.bbclass index ab3e1d71b5..6c21510377 100644
> > --- a/meta/classes-global/staging.bbclass
> > +++ b/meta/classes-global/staging.bbclass
> > 
> > @@ -245,8 +245,9 @@ def staging_populate_sysroot_dir(targetsysroot, 
nativesysroot, native, d):
> >                           continue
> >       
> >       staging_processfixme(fixme, targetdir, targetsysroot, nativesysroot,
> >       d)
> > 
> > -    for p in postinsts:
> > -        bb.note("Running postinst {}, output:\n{}".format(p,
> > subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT))) +   
> > bb.warn(sorted(postinsts))
> > +    for p in sorted(postinsts):
> > +        bb.warn("Running postinst {}, output:\n{}".format(p,
> > subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)))> 
> >   #
> >   # Manifests here are complicated. The main sysroot area has the unpacked
> >   sstate> 
> > @@ -629,7 +630,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 a58f8287c9..924eeadf9c 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.
> > 
> > @@ -130,10 +142,19 @@ useradd_sysroot () {
> > 
> >   		exit 0
> >   	
> >   	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')}"
> > + 	# Add groups and users defined for all recipe packages
> > +	if test "x$1" = "xgroup"; then
> > +		GROUPADD_PARAM="${@get_all_cmd_params(d, 'groupadd')}"
> > +	fi
> > +	if test "x$1" = "xuser"; then
> > +		USERADD_PARAM="${@get_all_cmd_params(d, 'useradd')}"
> > +	fi
> > +	if test "x$1" = "xgroupmems"; then
> > +		GROUPMEMS_PARAM="${@get_all_cmd_params(d, 
'groupmems')}"
> > +	fi
> > +	if 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 +169,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}"

BR
Piotr