[2/2] staging: ensure postinst-useradd is run in order

This patch is based off the work of Piotr Łobacz found here:
https://bugzilla.yoctoproject.org/attachment.cgi?id=4972&action=diff

The one issue with this is I'm not entirely certain that errors are
filtering up from postinsts, as the test for base-passwd's postinst
declares that if it's not found, we'll just call it later, but if so,
I'm not seeing where exactly that's happening.

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

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Eilís 'pidge' Ní Fhlannagáin
> Sent: den 12 december 2023 18:17
> To: openembedded-core@lists.openembedded.org
> Cc: Eilís 'pidge' Ní Fhlannagáin <pidge@baylibre.com>
> Subject: [OE-core] [PATCH 2/2] staging: ensure postinst-useradd is run in order
> 
> This patch is based off the work of Piotr Łobacz found here:
> https://bugzilla.yoctoproject.org/attachment.cgi?id=4972&action=diff
> 
> The one issue with this is I'm not entirely certain that errors are
> filtering up from postinsts, as the test for base-passwd's postinst
> declares that if it's not found, we'll just call it later, but if so,
> I'm not seeing where exactly that's happening.
> 
> Signed-off-by: Eilís 'pidge' Ní Fhlannagáin <pidge@baylibre.com>
> ---
>  meta/classes-global/staging.bbclass |  6 +--
>  meta/classes/useradd.bbclass        | 74 ++++++++++++++++++-----------
>  2 files changed, 49 insertions(+), 31 deletions(-)
> 
> diff --git a/meta/classes-global/staging.bbclass b/meta/classes-global/staging.bbclass
> index cf1e4600fd6..e879333aeb9 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):
>          subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)
> 
>  #
> @@ -629,9 +629,9 @@ 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):
>          subprocess.check_output(p, shell=True, stderr=subprocess.STDOUT)
> -
> +

This introduces trailing whitespace.

>      for dep in manifests:
>          c = setscenedeps[dep][0]
>          os.symlink(manifests[dep], depdir + "/" + c + ".complete")
> diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass
> index 0997b3da7a5..b044b926d99 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')}"
> +	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"
> +		exit 0
> +	fi

Use a case statement instead:

	case $1 in
		group)     GROUPADD_PARAM="${@get_all_cmd_params(d, 'groupadd')}";;
		user)      USERADD_PARAM="${@get_all_cmd_params(d, 'useradd')}";;
		groupmems) GROUPMEMS_PARAM="${@get_all_cmd_params(d, 'groupmems')}";;
		*)         bbfatal "Unknown/missing type of passwd db action: $1";;
	esac

> 
>  	# 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\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")]:

Change to:

    for type, sort_prefix in [("groupadd", "01"), ("useradd", "02"), ("groupmems", "03")]:

and adapt accordingly. Using "groupmemsadd" in other places as a result of 
the original names here does not make any sense.

> +        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}")

I suggest using f"postinst-{sort_prefix}-{type}" as name instead.

> +            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}"
> @@ -235,7 +253,7 @@ fakeroot python populate_packages:prepend () {
>          preinst = d.getVar('pkg_preinst:%s' % pkg) or d.getVar('pkg_preinst')
>          if not preinst:
>              preinst = '#!/bin/sh\n'
> -        preinst += 'bbnote () {\n\techo "NOTE: $*"\n}\n'
> +        preinst += 'set -e \n bbnote () {\n\techo "NOTE: $*"\n}\n'

Was this intentional? In that case I suggest changing the above to:

        if not preinst:
            preinst = '#!/bin/sh\n'
        preinst += 'set -e\n'
        preinst += 'bbnote () {\n\techo "NOTE: $*"\n}\n'

>          preinst += 'bbwarn () {\n\techo "WARNING: $*"\n}\n'
>          preinst += 'bbfatal () {\n\techo "ERROR: $*"\n\texit 1\n}\n'
>          preinst += 'perform_groupadd () {\n%s}\n' % d.getVar('perform_groupadd')
> --
> 2.34.1

//Peter