[PATCH V3 0/3] Add a method for image level user/group configuration

[PATCH V3 0/3] Add a method for image level user/group configuration

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

This patchset mainly does two things:
1. code refactor to avoid code duplication
2. add a method for image level user/group configuration

The following changes since commit a63229917a5708de2d161aba0d67168ce0da6365:

  meta-yocto-bsp: update reference board SRCREVs (2013-07-10 09:45:51 +0100)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib ChenQi/user_group_settings
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=ChenQi/user_group_settings

Chen Qi (3):
  userbase.bbclass: add a new bbclass
  useradd.bbclass: code refactor
  usersettings.bbclass: add a new bbclass

 meta/classes/useradd.bbclass      |  103 ++---------------
 meta/classes/userbase.bbclass     |  230 +++++++++++++++++++++++++++++++++++++
 meta/classes/usersettings.bbclass |   48 ++++++++
 3 files changed, 290 insertions(+), 91 deletions(-)
 create mode 100644 meta/classes/userbase.bbclass
 create mode 100644 meta/classes/usersettings.bbclass

-- 
1.7.9.5

[PATCH V3 1/3] userbase.bbclass: add a new bbclass

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

This class is mainly a collection of basic functions for user/group
settings.

This class is intended to be inherited by useradd.bbclass and the
usersettings.bbclass.

[YOCTO #4074]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/classes/userbase.bbclass |  230 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 230 insertions(+)
 create mode 100644 meta/classes/userbase.bbclass

diff --git a/meta/classes/userbase.bbclass b/meta/classes/userbase.bbclass
new file mode 100644
index 0000000..3de309c
--- /dev/null
+++ b/meta/classes/userbase.bbclass
@@ -0,0 +1,230 @@
+# This bbclass provides basic functionality for user/group settings.
+# This bbclass is intended to be inherited by useradd.bbclass and
+# usersettings.bbclass.
+
+# The following functions basically have similar logic.
+# *) Perform necessary checks before invoking the actual command
+# *) Invoke the actual command, make retries if necessary
+# *) Error out if an error occurs.
+
+# Note that before invoking these functions, make sure the global variable
+# PSEUDO is set up correctly.
+
+perform_groupadd () {
+	local rootdir="$1"
+	local opts="$2"
+	local retries="$3"
+	bbnote "Performing groupadd with [$opts] and $retries times of retry"
+	local groupname=`echo "$opts" | awk '{ print $NF }'`
+	local group_exists=`grep "^$groupname:" $rootdir/etc/group || true`
+	if test "x$group_exists" = "x"; then
+		local count=0
+		while true; do
+			eval $PSEUDO groupadd $opts || true
+			group_exists=`grep "^$groupname:" $rootdir/etc/group || true`
+			if test "x$group_exists" = "x"; then
+				bbwarn "groupadd command did not succeed. Retrying..."
+				sleep 1
+			else
+				break
+			fi
+			count=`expr $count + 1`
+			if test $count = $retries; then
+				bbfatal "Tried running groupadd command $retries times without scucess, giving up"
+			fi
+		done
+	else
+		bbwarn "group $groupname already exists, not re-creating it"
+	fi
+}
+
+perform_useradd () {
+	local rootdir="$1"
+	local opts="$2"
+	local retries="$3"
+	bbnote "Performing useradd with [$opts] and $retries times of retry"
+	local username=`echo "$opts" | awk '{ print $NF }'`
+	local user_exists=`grep "^$username:" $rootdir/etc/passwd || true`
+	if test "x$user_exists" = "x"; then
+	       local count=0
+	       while true; do
+		       eval $PSEUDO useradd $opts || true
+		       user_exists=`grep "^$username:" $rootdir/etc/passwd || true`
+		       if test "x$user_exists" = "x"; then
+			       bbwarn "useradd command did not succeed. Retrying..."
+			       sleep 1
+		       else
+			       break
+		       fi
+		       count=`expr $count + 1`
+		       if test $count = $retries; then
+				bbfatal "Tried running useradd command $retries times without scucess, giving up"
+		       fi
+	       done
+	else
+		bbwarn "user $username already exists, not re-creating it"
+	fi
+}
+
+perform_groupmems () {
+	local rootdir="$1"
+	local opts="$2"
+	local retries="$3"
+	bbnote "Performing groupmems with [$opts] and $retries times of retry"
+	local groupname=`echo "$opts" | awk '{ for (i = 1; i < NF; i++) if ($i == "-g" || $i == "--group") print $(i+1) }'`
+	local username=`echo "$opts" | awk '{ for (i = 1; i < NF; i++) if ($i == "-a" || $i == "--add") print $(i+1) }'`
+	bbnote "Running groupmems command with group $groupname and user $username"
+	# groupmems fails if /etc/gshadow does not exist
+	local gshadow=""
+	if [ -f $rootdir${sysconfdir}/gshadow ]; then
+		gshadow="yes"
+	else
+		gshadow="no"
+		touch $rootdir${sysconfdir}/gshadow
+	fi
+	local mem_exists=`grep "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*" $rootdir/etc/group || true`
+	if test "x$mem_exists" = "x"; then
+		local count=0
+		while true; do
+			eval $PSEUDO groupmems $opts || true
+			mem_exists=`grep "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*" $rootdir/etc/group || true`
+			if test "x$mem_exists" = "x"; then
+				bbwarn "groupmems command did not succeed. Retrying..."
+				sleep 1
+			else
+				break
+			fi
+			count=`expr $count + 1`
+			if test $count = $retries; then
+				if test "x$gshadow" = "xno"; then
+					rm -f $rootdir${sysconfdir}/gshadow
+					rm -f $rootdir${sysconfdir}/gshadow-
+				fi
+				bbfatal "Tried running groupmems command $retries times without scucess, giving up"
+			fi
+		done
+	else
+		bbwarn "group $groupname already contains $username, not re-adding it"
+	fi
+	if test "x$gshadow" = "xno"; then
+		rm -f $rootdir${sysconfdir}/gshadow
+		rm -f $rootdir${sysconfdir}/gshadow-
+	fi
+}
+
+perform_groupdel () {
+	local rootdir="$1"
+	local opts="$2"
+	local retries="$3"
+	bbnote "Performing groupdel with [$opts] and $retries times of retry"
+	local groupname=`echo "$opts" | awk '{ print $NF }'`
+	local group_exists=`grep "^$groupname:" $rootdir/etc/group || true`
+	if test "x$group_exists" != "x"; then
+		local count=0
+		while true; do
+			eval $PSEUDO groupdel $opts || true
+			group_exists=`grep "^$groupname:" $rootdir/etc/group || true`
+			if test "x$group_exists" != "x"; then
+				bbwarn "groupdel command did not succeed. Retrying..."
+				sleep 1
+			else
+				break
+			fi
+			count=`expr $count + 1`
+			if test $count = $retries; then
+				bbfatal "Tried running groupdel command $retries times without scucess, giving up"
+			fi
+		done
+	else
+		bbwarn "group $groupname doesn't exist, not removing it"
+	fi
+}
+
+perform_userdel () {
+	local rootdir="$1"
+	local opts="$2"
+	local retries="$3"
+	bbnote "Performing userdel with [$opts] and $retries times of retry"
+	local username=`echo "$opts" | awk '{ print $NF }'`
+	local user_exists=`grep "^$username:" $rootdir/etc/passwd || true`
+	if test "x$user_exists" != "x"; then
+	       local count=0
+	       while true; do
+		       eval $PSEUDO userdel $opts || true
+		       user_exists=`grep "^$username:" $rootdir/etc/passwd || true`
+		       if test "x$user_exists" != "x"; then
+			       bbwarn "userdel command did not succeed. Retrying..."
+			       sleep 1
+		       else
+			       break
+		       fi
+		       count=`expr $count + 1`
+		       if test $count = $retries; then
+				bbfatal "Tried running userdel command $retries times without scucess, giving up"
+		       fi
+	       done
+	else
+		bbwarn "user $username doesn't exist, not removing it"
+	fi
+}
+
+perform_groupmod () {
+	# Other than the return value of groupmod, there's no simple way to judge whether the command
+	# succeeds, so we disable -e option temporarily
+	set +e
+	local rootdir="$1"
+	local opts="$2"
+	local retries="$3"
+	bbnote "Performing groupmod with [$opts] and $retries times of retry"
+	local groupname=`echo "$opts" | awk '{ print $NF }'`
+	local group_exists=`grep "^$groupname:" $rootdir/etc/group || true`
+	if test "x$group_exists" != "x"; then
+		local count=0
+		while true; do
+			eval $PSEUDO groupmod $opts
+			if test $? != 0; then
+				bbwarn "groupmod command did not succeed. Retrying..."
+				sleep 1
+			else
+				break
+			fi
+			count=`expr $count + 1`
+			if test $count = $retries; then
+				bbfatal "Tried running groupmod command $retries times without scucess, giving up"
+			fi
+		done
+	else
+		bbfatal "group $groupname doesn't exist, unable to modify it"
+	fi
+	set -e
+}
+
+perform_usermod () {
+	# Same reason with groupmod, temporarily disable -e option
+	set +e
+	local rootdir="$1"
+	local opts="$2"
+	local retries="$3"
+	bbnote "Performing usermod with [$opts] and $retries times of retry"
+	local username=`echo "$opts" | awk '{ print $NF }'`
+	local user_exists=`grep "^$username:" $rootdir/etc/passwd || true`
+	if test "x$user_exists" != "x"; then
+	       local count=0
+	       while true; do
+		       eval $PSEUDO usermod $opts
+		       if test $? != 0; then
+			       bbwarn "usermod command did not succeed. Retrying..."
+			       sleep 1
+		       else
+			       break
+		       fi
+		       count=`expr $count + 1`
+		       if test $count = $retries; then
+				bbfatal "Tried running usermod command $retries times without scucess, giving up"
+		       fi
+	       done
+	else
+		bbfatal "user $username doesn't exist, unable to modify it"
+	fi
+	set -e
+}
-- 
1.7.9.5

[PATCH V3 2/3] useradd.bbclass: code refactor

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

The basic functions have moved to userbase.bbclass. So this class
only needs to inherit userbase.bbclass and use the functions defined
there. The reason is to avoid code duplication with usersettings.bbclass.

[YOCTO #4074]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/classes/useradd.bbclass |  103 +++++-------------------------------------
 1 file changed, 12 insertions(+), 91 deletions(-)

diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass
index 3fe011d..2b6388a 100644
--- a/meta/classes/useradd.bbclass
+++ b/meta/classes/useradd.bbclass
@@ -1,3 +1,5 @@
+inherit userbase
+
 # base-passwd-cross provides the default passwd and group files in the
 # target sysroot, and shadow -native and -sysroot provide the utilities
 # and support files needed to add and modify user and group accounts
@@ -44,30 +46,7 @@ if test "x$GROUPADD_PARAM" != "x"; then
 	opts=`echo "$GROUPADD_PARAM" | cut -d ';' -f 1`
 	remaining=`echo "$GROUPADD_PARAM" | cut -d ';' -f 2-`
 	while test "x$opts" != "x"; do
-		groupname=`echo "$opts" | awk '{ print $NF }'`
-		group_exists=`grep "^$groupname:" $SYSROOT/etc/group || true`
-		if test "x$group_exists" = "x"; then
-			count=1
-			while true; do
-				eval $PSEUDO groupadd $OPT $opts || true
-				group_exists=`grep "^$groupname:" $SYSROOT/etc/group || true`
-				if test "x$group_exists" = "x"; then
-					# File locking issues can require us to retry the command
-					echo "WARNING: groupadd command did not succeed. Retrying..."
-					sleep 1
-				else
-					break
-				fi
-				count=`expr $count + 1`
-				if test $count = 11; then
-					echo "ERROR: tried running groupadd command 10 times without success, giving up"
-					exit 1
-				fi
-			done		
-		else
-			echo "Note: group $groupname already exists, not re-creating it"
-		fi
-
+		perform_groupadd "$SYSROOT" "$OPT $opts" 10
 		if test "x$opts" = "x$remaining"; then
 			break
 		fi
@@ -83,32 +62,7 @@ if test "x$USERADD_PARAM" != "x"; then
 	opts=`echo "$USERADD_PARAM" | cut -d ';' -f 1`
 	remaining=`echo "$USERADD_PARAM" | cut -d ';' -f 2-`
 	while test "x$opts" != "x"; do
-		# useradd does not have a -f option, so we have to check if the
-		# username already exists manually
-		username=`echo "$opts" | awk '{ print $NF }'`
-		user_exists=`grep "^$username:" $SYSROOT/etc/passwd || true`
-		if test "x$user_exists" = "x"; then
-			count=1
-			while true; do
-				eval $PSEUDO useradd $OPT $opts || true
-				user_exists=`grep "^$username:" $SYSROOT/etc/passwd || true`
-				if test "x$user_exists" = "x"; then
-					# File locking issues can require us to retry the command
-					echo "WARNING: useradd command did not succeed. Retrying..."
-					sleep 1
-				else
-					break
-				fi
-				count=`expr $count + 1`
-				if test $count = 11; then
-					echo "ERROR: tried running useradd command 10 times without success, giving up"
-					exit 1
-				fi
-			done
-		else
-			echo "Note: username $username already exists, not re-creating it"
-		fi
-
+		perform_useradd "$SYSROOT" "$OPT $opts" 10
 		if test "x$opts" = "x$remaining"; then
 			break
 		fi
@@ -119,58 +73,18 @@ fi
 
 if test "x$GROUPMEMS_PARAM" != "x"; then
 	echo "Running groupmems commands..."
-	# groupmems fails if /etc/gshadow does not exist
-	if [ -f $SYSROOT${sysconfdir}/gshadow ]; then
-		gshadow="yes"
-	else
-		gshadow="no"
-		touch $SYSROOT${sysconfdir}/gshadow
-	fi
 	# Invoke multiple instances of groupmems for parameter lists
 	# separated by ';'
 	opts=`echo "$GROUPMEMS_PARAM" | cut -d ';' -f 1`
 	remaining=`echo "$GROUPMEMS_PARAM" | cut -d ';' -f 2-`
 	while test "x$opts" != "x"; do
-		groupname=`echo "$opts" | awk '{ for (i = 1; i < NF; i++) if ($i == "-g" || $i == "--group") print $(i+1) }'`
-		username=`echo "$opts" | awk '{ for (i = 1; i < NF; i++) if ($i == "-a" || $i == "--add") print $(i+1) }'`
-		echo "$groupname $username"
-		mem_exists=`grep "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*" $SYSROOT/etc/group || true`
-		if test "x$mem_exists" = "x"; then
-			count=1
-			while true; do
-				eval $PSEUDO groupmems $OPT $opts || true
-				mem_exists=`grep "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*" $SYSROOT/etc/group || true`
-				if test "x$mem_exists" = "x"; then
-					# File locking issues can require us to retry the command
-					echo "WARNING: groupmems command did not succeed. Retrying..."
-					sleep 1
-				else
-					break
-				fi
-				count=`expr $count + 1`
-				if test $count = 11; then
-					echo "ERROR: tried running groupmems command 10 times without success, giving up"
-					if test "x$gshadow" = "xno"; then
-						rm -f $SYSROOT${sysconfdir}/gshadow
-						rm -f $SYSROOT${sysconfdir}/gshadow-
-					fi
-					exit 1
-				fi
-			done
-		else
-			echo "Note: group $groupname already contains $username, not re-adding it"
-		fi
-
+		perform_groupmems "$SYSROOT" "$OPT $opts" 10
 		if test "x$opts" = "x$remaining"; then
 			break
 		fi
 		opts=`echo "$remaining" | cut -d ';' -f 1`
 		remaining=`echo "$remaining" | cut -d ';' -f 2-`
 	done
-	if test "x$gshadow" = "xno"; then
-		rm -f $SYSROOT${sysconfdir}/gshadow
-		rm -f $SYSROOT${sysconfdir}/gshadow-
-	fi
 fi
 }
 
@@ -254,6 +168,13 @@ fakeroot python populate_packages_prepend () {
         preinst = d.getVar('pkg_preinst_%s' % pkg, True) or d.getVar('pkg_preinst', True)
         if not preinst:
             preinst = '#!/bin/sh\n'
+        preinst += 'perform_groupadd () {\n%s}\n' % d.getVar('perform_groupadd', True)
+        preinst += 'perform_useradd () {\n%s}\n' % d.getVar('perform_useradd', True)
+        preinst += 'perform_groupmems () {\n%s}\n' % d.getVar('perform_groupmems', True)
+        preinst += 'perform_groupdel () {\n%s}\n' % d.getVar('perform_groupdel', True)
+        preinst += 'perform_userdel () {\n%s}\n' % d.getVar('perform_userdel', True)
+        preinst += 'perform_groupmod () {\n%s}\n' % d.getVar('perform_groupmod', True)
+        preinst += 'perform_usermod () {\n%s}\n' % d.getVar('perform_usermod', True)
         preinst += d.getVar('useradd_preinst', True)
         d.setVar('pkg_preinst_%s' % pkg, preinst)
 
-- 
1.7.9.5

[PATCH V3 3/3] usersettings.bbclass: add a new bbclass

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

This class is dedicated to image level user/group configuration.
It inherits userbase.bbclass.

Users need to inherit this class in their layers or local.conf to
make the setting of USER_GROUP_SETTINGS effective.

For detailed configuration format of USER_GROUP_SETTINGS, please
refer to local.conf.sample.extended.

[YOCTO #4074]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/classes/usersettings.bbclass |   48 +++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 meta/classes/usersettings.bbclass

diff --git a/meta/classes/usersettings.bbclass b/meta/classes/usersettings.bbclass
new file mode 100644
index 0000000..e5a5156
--- /dev/null
+++ b/meta/classes/usersettings.bbclass
@@ -0,0 +1,48 @@
+# This bbclass is mainly used for image level user/group configuration.
+# Inherit this class if you want to make USER_GROUP_SETTINGS effective.
+inherit userbase
+
+IMAGE_INSTALL_append += "${@['', 'base-passwd shadow'][bool(d.getVar('USER_GROUP_SETTINGS', True))]}"
+
+# Image level user / group settings
+ROOTFS_POSTPROCESS_COMMAND_append = " set_user_group;"
+
+# Image level user / group settings
+set_user_group () {
+	user_group_settings="${USER_GROUP_SETTINGS}"
+	export PSEUDO="${FAKEROOTENV} ${STAGING_DIR_NATIVE}${bindir}/pseudo"
+	setting=`echo $user_group_settings | cut -d ';' -f1`
+	remaining=`echo $user_group_settings | cut -d ';' -f2-`
+	while test "x$setting" != "x"; do
+		action=`echo $setting | cut -d ',' -f1`
+		opts=`echo $setting | cut -d ',' -f2`
+		# Different from useradd.bbclass, there's no file locking issue here, as
+		# this setting is actually a serial process. So we only retry once.
+		case $action in
+			useradd)
+				perform_useradd "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
+				;;
+			groupadd)
+				perform_groupadd "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
+				;;
+			userdel)
+				perform_userdel "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
+				;;
+			groupdel)
+				perform_groupdel "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
+				;;
+			usermod)
+				perform_usermod "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
+				;;
+			groupmod)
+				perform_groupmod "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
+				;;
+			*)
+				bbfatal "Incorrect setting for USER_GROUP_SETTINGS"
+				;;
+		esac
+		# iterate to the next setting
+		setting=`echo $remaining | cut -d ';' -f1`
+		remaining=`echo $remaining | cut -d ';' -f2-`
+	done
+}
-- 
1.7.9.5

Re: [PATCH V3 0/3] Add a method for image level user/group configuration

[Paul Eggleton]

Hi Qi,

On Thursday 11 July 2013 19:11:36 Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
> 
> This patchset mainly does two things:
> 1. code refactor to avoid code duplication
> 2. add a method for image level user/group configuration
> 
> The following changes since commit a63229917a5708de2d161aba0d67168ce0da6365:
> 
>   meta-yocto-bsp: update reference board SRCREVs (2013-07-10 09:45:51 +0100)
> 
> are available in the git repository at:
> 
>   git://git.pokylinux.org/poky-contrib ChenQi/user_group_settings
>  
> http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=ChenQi/user_group_set
> tings
> 
> Chen Qi (3):
>   userbase.bbclass: add a new bbclass
>   useradd.bbclass: code refactor
>   usersettings.bbclass: add a new bbclass
> 
>  meta/classes/useradd.bbclass      |  103 ++---------------
>  meta/classes/userbase.bbclass     |  230
> +++++++++++++++++++++++++++++++++++++ meta/classes/usersettings.bbclass |  
> 48 ++++++++
>  3 files changed, 290 insertions(+), 91 deletions(-)
>  create mode 100644 meta/classes/userbase.bbclass
>  create mode 100644 meta/classes/usersettings.bbclass

In terms of naming I'd rather see the base class be called useradd_base to
match the naming of similar existing classes. For the new class, I'm not sure
usersettings is really an appropriate name, perhaps something like
"useradd_real" or "useraccount" or something else that more accurately reflects 
what it is for?

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH V3 3/3] usersettings.bbclass: add a new bbclass

[Paul Eggleton]

On Thursday 11 July 2013 19:11:39 Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
> 
> This class is dedicated to image level user/group configuration.
> It inherits userbase.bbclass.
> 
> Users need to inherit this class in their layers or local.conf to
> make the setting of USER_GROUP_SETTINGS effective.
> 
> For detailed configuration format of USER_GROUP_SETTINGS, please
> refer to local.conf.sample.extended.
> 
> [YOCTO #4074]
> 
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  meta/classes/usersettings.bbclass |   48
> +++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
>  create mode 100644 meta/classes/usersettings.bbclass
> 
> diff --git a/meta/classes/usersettings.bbclass
> b/meta/classes/usersettings.bbclass new file mode 100644
> index 0000000..e5a5156
> --- /dev/null
> +++ b/meta/classes/usersettings.bbclass
> @@ -0,0 +1,48 @@
> +# This bbclass is mainly used for image level user/group configuration.
> +# Inherit this class if you want to make USER_GROUP_SETTINGS effective.
> +inherit userbase
> +
> +IMAGE_INSTALL_append += "${@['', 'base-passwd shadow'][bool(d.getVar('USER_GROUP_SETTINGS', True))]}"

This is a bit ugly. We should avoid using _append += since it is confusing,
and we have base_conditional for things like this. I'd suggest:

IMAGE_INSTALL_append = "${@base_conditional('USER_GROUP_SETTINGS', '1', ' base-passwd shadow', '', d)}"

> +# Image level user / group settings
> +ROOTFS_POSTPROCESS_COMMAND_append = " set_user_group;"
> +
> +# Image level user / group settings
> +set_user_group () {
> +	user_group_settings="${USER_GROUP_SETTINGS}"
> +	export PSEUDO="${FAKEROOTENV} ${STAGING_DIR_NATIVE}${bindir}/pseudo"
> +	setting=`echo $user_group_settings | cut -d ';' -f1`
> +	remaining=`echo $user_group_settings | cut -d ';' -f2-`
> +	while test "x$setting" != "x"; do
> +		action=`echo $setting | cut -d ',' -f1`
> +		opts=`echo $setting | cut -d ',' -f2`
> +		# Different from useradd.bbclass, there's no file locking issue here, as
> +		# this setting is actually a serial process. So we only retry once.
> +		case $action in
> +			useradd)
> +				perform_useradd "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			groupadd)
> +				perform_groupadd "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			userdel)
> +				perform_userdel "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			groupdel)
> +				perform_groupdel "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			usermod)
> +				perform_usermod "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			groupmod)
> +				perform_groupmod "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			*)
> +				bbfatal "Incorrect setting for USER_GROUP_SETTINGS"

Could you please make it report $action in the error so it's clear which
value is invalid?

> +				;;
> +		esac
> +		# iterate to the next setting
> +		setting=`echo $remaining | cut -d ';' -f1`
> +		remaining=`echo $remaining | cut -d ';' -f2-`
> +	done
> +}

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH V3 3/3] usersettings.bbclass: add a new bbclass

[Paul Eggleton]

On Thursday 11 July 2013 13:58:17 Paul Eggleton wrote:
> On Thursday 11 July 2013 19:11:39 Qi.Chen@windriver.com wrote:
> > +IMAGE_INSTALL_append += "${@['', 'base-passwd
> > shadow'][bool(d.getVar('USER_GROUP_SETTINGS', True))]}"
>
> This is a bit ugly. We should avoid using _append += since it is confusing,
> and we have base_conditional for things like this. I'd suggest:
> 
> IMAGE_INSTALL_append = "${@base_conditional('USER_GROUP_SETTINGS', '1', '
> base-passwd shadow', '', d)}"

Actually reading this again I've misinterpreted what it's supposed to be 
doing. Unfortunately I don't think we have a neat base_ifset() or similar so 
perhaps this will need to be left as-is. I still think we should avoid _append 
+= however.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH V3 3/3] usersettings.bbclass: add a new bbclass

[Mark Hatle]

On 7/11/13 6:11 AM, Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
>
> This class is dedicated to image level user/group configuration.
> It inherits userbase.bbclass.
>
> Users need to inherit this class in their layers or local.conf to
> make the setting of USER_GROUP_SETTINGS effective.
>
> For detailed configuration format of USER_GROUP_SETTINGS, please
> refer to local.conf.sample.extended.
>
> [YOCTO #4074]
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>   meta/classes/usersettings.bbclass |   48 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 48 insertions(+)
>   create mode 100644 meta/classes/usersettings.bbclass
>
> diff --git a/meta/classes/usersettings.bbclass b/meta/classes/usersettings.bbclass
> new file mode 100644
> index 0000000..e5a5156
> --- /dev/null
> +++ b/meta/classes/usersettings.bbclass
> @@ -0,0 +1,48 @@
> +# This bbclass is mainly used for image level user/group configuration.
> +# Inherit this class if you want to make USER_GROUP_SETTINGS effective.
> +inherit userbase

One request, in the comment above, please put some examples of the using the new 
functionality.  It's not completely clear to me looking at this what may be 
expected of the user.

--Mark

> +
> +IMAGE_INSTALL_append += "${@['', 'base-passwd shadow'][bool(d.getVar('USER_GROUP_SETTINGS', True))]}"
> +
> +# Image level user / group settings
> +ROOTFS_POSTPROCESS_COMMAND_append = " set_user_group;"
> +
> +# Image level user / group settings
> +set_user_group () {
> +	user_group_settings="${USER_GROUP_SETTINGS}"
> +	export PSEUDO="${FAKEROOTENV} ${STAGING_DIR_NATIVE}${bindir}/pseudo"
> +	setting=`echo $user_group_settings | cut -d ';' -f1`
> +	remaining=`echo $user_group_settings | cut -d ';' -f2-`
> +	while test "x$setting" != "x"; do
> +		action=`echo $setting | cut -d ',' -f1`
> +		opts=`echo $setting | cut -d ',' -f2`
> +		# Different from useradd.bbclass, there's no file locking issue here, as
> +		# this setting is actually a serial process. So we only retry once.
> +		case $action in
> +			useradd)
> +				perform_useradd "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			groupadd)
> +				perform_groupadd "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			userdel)
> +				perform_userdel "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			groupdel)
> +				perform_groupdel "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			usermod)
> +				perform_usermod "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			groupmod)
> +				perform_groupmod "${IMAGE_ROOTFS}" "-R ${IMAGE_ROOTFS} $opts" 1
> +				;;
> +			*)
> +				bbfatal "Incorrect setting for USER_GROUP_SETTINGS"
> +				;;
> +		esac
> +		# iterate to the next setting
> +		setting=`echo $remaining | cut -d ';' -f1`
> +		remaining=`echo $remaining | cut -d ';' -f2-`
> +	done
> +}
>

Re: [PATCH V3 0/3] Add a method for image level user/group configuration

[Saul Wold]

On 07/11/2013 05:49 AM, Paul Eggleton wrote:
> Hi Qi,
>
> On Thursday 11 July 2013 19:11:36 Qi.Chen@windriver.com wrote:
>> From: Chen Qi <Qi.Chen@windriver.com>
>>
>> This patchset mainly does two things:
>> 1. code refactor to avoid code duplication
>> 2. add a method for image level user/group configuration
>>
>> The following changes since commit a63229917a5708de2d161aba0d67168ce0da6365:
>>
>>    meta-yocto-bsp: update reference board SRCREVs (2013-07-10 09:45:51 +0100)
>>
>> are available in the git repository at:
>>
>>    git://git.pokylinux.org/poky-contrib ChenQi/user_group_settings
>>
>> http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=ChenQi/user_group_set
>> tings
>>
>> Chen Qi (3):
>>    userbase.bbclass: add a new bbclass
>>    useradd.bbclass: code refactor
>>    usersettings.bbclass: add a new bbclass
>>
>>   meta/classes/useradd.bbclass      |  103 ++---------------
>>   meta/classes/userbase.bbclass     |  230
>> +++++++++++++++++++++++++++++++++++++ meta/classes/usersettings.bbclass |
>> 48 ++++++++
>>   3 files changed, 290 insertions(+), 91 deletions(-)
>>   create mode 100644 meta/classes/userbase.bbclass
>>   create mode 100644 meta/classes/usersettings.bbclass
>
> In terms of naming I'd rather see the base class be called useradd_base to
> match the naming of similar existing classes. For the new class, I'm not sure
> usersettings is really an appropriate name, perhaps something like
> "useradd_real" or "useraccount" or something else that more accurately reflects
> what it is for?
>

I think this is a better approach than what you had orignally.

I agree the naming is not correct currently, useraccount or maybe 
extrausers, also when you rename the class the USER_GROUP_SETTING should 
also be renamed to reflect that, something like EXTRA_USERS_PARAMS or 
USER_ACCOUNT_PARAMS to match the class name.

You also need to provide full documentation on this new variable, please 
send details to Scott Rifenbark.


> Cheers,
> Paul
>