Patchwork [4/7] useradd.bbclass: new class for managing user/group permissions

login
register
mail settings
Submitter Scott Garman
Date June 2, 2011, 11:50 p.m.
Message ID <9591788655a4de97cc5f6a0fd98e742e3ebcab23.1307058256.git.scott.a.garman@intel.com>
Download mbox | patch
Permalink /patch/5353/
State New, archived
Headers show

Comments

Scott Garman - June 2, 2011, 11:50 p.m.
This class is to be used by recipes that need to set up specific
user/group accounts and set custom file/directory permissions.

Signed-off-by: Scott Garman <scott.a.garman@intel.com>
---
 meta/classes/useradd.bbclass |  163 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 163 insertions(+), 0 deletions(-)
 create mode 100644 meta/classes/useradd.bbclass
Richard Purdie - June 28, 2011, 1:04 p.m.
Hi Scott,

Sorry its taken me a while to get to this. Some comments below.

On Thu, 2011-06-02 at 16:50 -0700, Scott Garman wrote:
> This class is to be used by recipes that need to set up specific
> user/group accounts and set custom file/directory permissions.
> 
> Signed-off-by: Scott Garman <scott.a.garman@intel.com>
> ---
>  meta/classes/useradd.bbclass |  163 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 163 insertions(+), 0 deletions(-)
>  create mode 100644 meta/classes/useradd.bbclass
> 
> diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass
> new file mode 100644
> index 0000000..3f07e5e
> --- /dev/null
> +++ b/meta/classes/useradd.bbclass
> @@ -0,0 +1,163 @@
> +USERADDPN ?= "${PN}"
> +
> +# base-passwd-cross provides the default passwd and group files in the
> +# target sysroot, and shadow-native provides the utilities needed to
> +# add and modify user and group accounts
> +DEPENDS_append = " base-passwd shadow-native"
> +RDEPENDS_${USERADDPN}_append = " base-passwd shadow"
> +
> +PSEUDO="${STAGING_DIR_NATIVE}/usr/bin/pseudo"
> +export PSEUDO

For reference this can be done with:

export PSEUDO = "${STAGING_DIR_NATIVE}/usr/bin/pseudo"

> +PSEUDO_LOCALSTATEDIR="${STAGING_DIR_TARGET}/var/pseudo"
> +export PSEUDO_LOCALSTATEDIR

I'm a little bit puzzled at this point. This is changing the default
PSEUDO state directory to be one shared by many other users rather than
the default of the one in workdir. Is that really what you intend here?

I guess the question is whether we need to preserve these users in the
sysroot or whether preserving them for the install/package/package_write
cycle is enough.

Since tasks don't share the same pseudo context by default, I suspect
we're not preserving users for the sysroot anyhow?

> +PSEUDO_PASSWD = "${STAGING_DIR_TARGET}"
> +export PSEUDO_PASSWD

Should we set this by default as part of the pseudo options in
bitbake.conf?

> +useradd_preinst () {
> +OPT=""
> +SYSROOT=""
> +
> +if test "x$D" != "x"; then
> +	# Installing into a sysroot
> +	SYSROOT="${STAGING_DIR_TARGET}"
> +	OPT="--root ${STAGING_DIR_TARGET}"
> +
> +	# Add groups and users defined for all recipe packages
> +	GROUPADD_PARAM="${@get_all_cmd_params(d, 'group')}"
> +	USERADD_PARAM="${@get_all_cmd_params(d, 'user')}"
> +else
> +	# Installing onto a target
> +	PSEUDO=""
> +
> +	# Add groups and users defined only for this package
> +	GROUPADD_PARAM="${GROUPADD_PARAM}"
> +	USERADD_PARAM="${USERADD_PARAM}"
> +fi
> +
> +# Perform group additions first, since user additions may depend
> +# on these groups existing
> +if test "x$GROUPADD_PARAM" != "x"; then
> +	echo "Running groupadd commands..."
> +	# Invoke multiple instances of groupadd for parameter lists
> +	# separated by ';'
> +	opts=`echo "$GROUPADD_PARAM" | cut -d ';' -f 1`
> +	remaining=`echo "$GROUPADD_PARAM" | cut -d ';' -f 2-`
> +	while test "x$opts" != "x"; do
> +		eval $PSEUDO groupadd -f $OPT $opts

If this task is already running under pseudo, do we specifically need to
run under pseudo here? Is it not already running under pseudo when
needed?

> +		if test "x$opts" = "x$remaining"; then
> +			break
> +		fi
> +		opts=`echo "$remaining" | cut -d ';' -f 1`
> +		remaining=`echo "$remaining" | cut -d ';' -f 2-`
> +	done
> +fi 
> +
> +if test "x$USERADD_PARAM" != "x"; then
> +	echo "Running useradd commands..."
> +	# Invoke multiple instances of useradd for parameter lists
> +	# separated by ';'
> +	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
> +			eval $PSEUDO useradd $OPT $opts
> +		else
> +			echo "Note: username $username already exists, not re-creating it"
> +		fi
> +
> +		if test "x$opts" = "x$remaining"; then
> +			break
> +		fi
> +		opts=`echo "$remaining" | cut -d ';' -f 1`
> +		remaining=`echo "$remaining" | cut -d ';' -f 2-`
> +	done
> +fi
> +}
> +
> +useradd_sysroot () {
> +	# Explicitly set $D since it isn't set to anything
> +	# before do_install
> +	D=${D}
> +	useradd_preinst
> +}
> +
> +useradd_sysroot_sstate () {
> +	if [ "${BB_CURRENTTASK}" = "populate_sysroot_setscene" ]
> +	then
> +		useradd_sysroot
> +	fi
> +}
> +
> +do_install[prefuncs] += "useradd_sysroot"
> +SSTATEPOSTINSTFUNCS += "useradd_sysroot_sstate"
> +
> +# Recipe parse-time sanity checks
> +def update_useradd_after_parse(d):
> +	if bb.data.getVar('USERADD_PACKAGES', d) == None:
> +		if bb.data.getVar('USERADD_PARAM', d) == None and bb.data.getVar('GROUPADD_PARAM', d) == None:
> +			raise bb.build.FuncFailed, "%s inherits useradd but doesn't set USERADD_PARAM or GROUPADD_PARAM" % bb.data.getVar('FILE', d)
> +
> +python __anonymous() {
> +	update_useradd_after_parse(d)
> +}
> +
> +# Return a single [GROUP|USER]ADD_PARAM formatted string which includes the
> +# [group|user]add parameters for all packages in this recipe
> +def get_all_cmd_params(d, cmd_type):
> +	import string
> +	
> +	localdata = bb.data.createCopy(d)
> +	param_type = cmd_type.upper() + "ADD_PARAM_%s"
> +	params = []
> +
> +	pkgs = bb.data.getVar('USERADD_PACKAGES', d, 1)

Please use True, not 1. Its also simpler to use:

d.getVar('USERADD_PACKAGES', True)

> +	if pkgs == None:

if not pkgs:

> +		pkgs = bb.data.getVar('USERADDPN', d, 1)
> +		packages = (bb.data.getVar('PACKAGES', d, 1) or "").split()
> +		if not pkgs in packages and packages != []:

if packages and pkgs not in packages:

> +			pkgs = packages[0]
> +
> +	for pkg in pkgs.split():
> +		param = bb.data.getVar(param_type % pkg, localdata, 1)
> +		params.append(param)
> +
> +	return string.join(params, "; ")
> +
> +# Adds the preinst script into generated packages
> +fakeroot python populate_packages_prepend () {
> +	def update_useradd_package(pkg):
> +		bb.debug(1, 'adding user/group calls to preinst for %s' % pkg)
> +		localdata = bb.data.createCopy(d)
> +		overrides = bb.data.getVar("OVERRIDES", localdata, 1)
> +		bb.data.setVar("OVERRIDES", "%s:%s" % (pkg, overrides), localdata)
> +		bb.data.update_data(localdata)
> +
> +		"""
> +		useradd preinst is appended here because pkg_preinst may be
> +		required to execute on the target. Not doing so may cause
> +		useradd preinst to be invoked twice, causing unwanted warnings.
> +		"""
> +		preinst = bb.data.getVar('pkg_preinst', localdata, 1)
> +		if not preinst:
> +			preinst = '#!/bin/sh\n'
> +		preinst += bb.data.getVar('useradd_preinst', localdata, 1)
> +		bb.data.setVar('pkg_preinst_%s' % pkg, preinst, d)
> +
> +	# We add the user/group calls to all packages to allow any package
> +	# to contain files owned by the users/groups defined in the recipe.
> +	# The user/group addition code is careful not to create duplicate
> +	# entries, so this is safe.
> +	pkgs = bb.data.getVar('USERADD_PACKAGES', d, 1)
> +	if pkgs == None:
> +		pkgs = bb.data.getVar('USERADDPN', d, 1)
> +		packages = (bb.data.getVar('PACKAGES', d, 1) or "").split()
> +		if not pkgs in packages and packages != []:
> +			pkgs = packages[0]
> +	for pkg in pkgs.split():
> +		update_useradd_package(pkg)
> +}

Cheers,

Richard
Mark Hatle - June 28, 2011, 2:42 p.m.
On 6/28/11 8:04 AM, Richard Purdie wrote:
> Hi Scott,
> 
> Sorry its taken me a while to get to this. Some comments below.

I think I know the answer to a few of the issues mentioned below.  Scott can
correct me if I'm wrong.

> On Thu, 2011-06-02 at 16:50 -0700, Scott Garman wrote:
>> This class is to be used by recipes that need to set up specific
>> user/group accounts and set custom file/directory permissions.
>>
>> Signed-off-by: Scott Garman <scott.a.garman@intel.com>
>> ---
>>  meta/classes/useradd.bbclass |  163 ++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 163 insertions(+), 0 deletions(-)
>>  create mode 100644 meta/classes/useradd.bbclass
>>
>> diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass
>> new file mode 100644
>> index 0000000..3f07e5e
>> --- /dev/null
>> +++ b/meta/classes/useradd.bbclass
>> @@ -0,0 +1,163 @@
>> +USERADDPN ?= "${PN}"
>> +
>> +# base-passwd-cross provides the default passwd and group files in the
>> +# target sysroot, and shadow-native provides the utilities needed to
>> +# add and modify user and group accounts
>> +DEPENDS_append = " base-passwd shadow-native"
>> +RDEPENDS_${USERADDPN}_append = " base-passwd shadow"
>> +
>> +PSEUDO="${STAGING_DIR_NATIVE}/usr/bin/pseudo"
>> +export PSEUDO
> 
> For reference this can be done with:
> 
> export PSEUDO = "${STAGING_DIR_NATIVE}/usr/bin/pseudo"
> 
>> +PSEUDO_LOCALSTATEDIR="${STAGING_DIR_TARGET}/var/pseudo"
>> +export PSEUDO_LOCALSTATEDIR
> 
> I'm a little bit puzzled at this point. This is changing the default
> PSEUDO state directory to be one shared by many other users rather than
> the default of the one in workdir. Is that really what you intend here?
> 
> I guess the question is whether we need to preserve these users in the
> sysroot or whether preserving them for the install/package/package_write
> cycle is enough.

If we're populating the sysroot, we need to have a pseudo directory setup for it
so that we can run the adduser/group items.  The new adduser/group require the
ability to "chroot" into a (pseudo) root.  The above should only kick in while
working with sysroots.

> Since tasks don't share the same pseudo context by default, I suspect
> we're not preserving users for the sysroot anyhow?

Users are added/preserved within the sysroot /etc/passwd,group in the code.
There is a separate section that runs at sysroot population time that adds the
necessary users/groups... but as mentioned we need the chroot functionality to
do the settings.  (The uid/gid of the files themselves doesn't matter in the
sysroot.)

>> +PSEUDO_PASSWD = "${STAGING_DIR_TARGET}"
>> +export PSEUDO_PASSWD
> 
> Should we set this by default as part of the pseudo options in
> bitbake.conf?

Yes, this likely should go into the bitbake.conf now.  I believe it is in here
though for the same reason that the above was.  It's needed during the sysroot
configuration and the way the utilities work when chrooted.

>> +useradd_preinst () {
>> +OPT=""
>> +SYSROOT=""
>> +
>> +if test "x$D" != "x"; then
>> +	# Installing into a sysroot
>> +	SYSROOT="${STAGING_DIR_TARGET}"
>> +	OPT="--root ${STAGING_DIR_TARGET}"
>> +
>> +	# Add groups and users defined for all recipe packages
>> +	GROUPADD_PARAM="${@get_all_cmd_params(d, 'group')}"
>> +	USERADD_PARAM="${@get_all_cmd_params(d, 'user')}"
>> +else
>> +	# Installing onto a target
>> +	PSEUDO=""
>> +
>> +	# Add groups and users defined only for this package
>> +	GROUPADD_PARAM="${GROUPADD_PARAM}"
>> +	USERADD_PARAM="${USERADD_PARAM}"
>> +fi
>> +
>> +# Perform group additions first, since user additions may depend
>> +# on these groups existing
>> +if test "x$GROUPADD_PARAM" != "x"; then
>> +	echo "Running groupadd commands..."
>> +	# Invoke multiple instances of groupadd for parameter lists
>> +	# separated by ';'
>> +	opts=`echo "$GROUPADD_PARAM" | cut -d ';' -f 1`
>> +	remaining=`echo "$GROUPADD_PARAM" | cut -d ';' -f 2-`
>> +	while test "x$opts" != "x"; do
>> +		eval $PSEUDO groupadd -f $OPT $opts
> 
> If this task is already running under pseudo, do we specifically need to
> run under pseudo here? Is it not already running under pseudo when
> needed?

See answer below...

>> +		if test "x$opts" = "x$remaining"; then
>> +			break
>> +		fi
>> +		opts=`echo "$remaining" | cut -d ';' -f 1`
>> +		remaining=`echo "$remaining" | cut -d ';' -f 2-`
>> +	done
>> +fi 
>> +
>> +if test "x$USERADD_PARAM" != "x"; then
>> +	echo "Running useradd commands..."
>> +	# Invoke multiple instances of useradd for parameter lists
>> +	# separated by ';'
>> +	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
>> +			eval $PSEUDO useradd $OPT $opts
>> +		else
>> +			echo "Note: username $username already exists, not re-creating it"
>> +		fi
>> +
>> +		if test "x$opts" = "x$remaining"; then
>> +			break
>> +		fi
>> +		opts=`echo "$remaining" | cut -d ';' -f 1`
>> +		remaining=`echo "$remaining" | cut -d ';' -f 2-`
>> +	done
>> +fi
>> +}
>> +
>> +useradd_sysroot () {
>> +	# Explicitly set $D since it isn't set to anything
>> +	# before do_install
>> +	D=${D}
>> +	useradd_preinst
>> +}
>> +
>> +useradd_sysroot_sstate () {
>> +	if [ "${BB_CURRENTTASK}" = "populate_sysroot_setscene" ]
>> +	then
>> +		useradd_sysroot
>> +	fi
>> +}

The way this is implemented the new sysroot tasks call the regular tasks so the
code only has to be implemented once.  Assuming context doesn't change, it
should be possible to move the "PSEUDO" calls down into the _sysroot functions
and remove them from the items that get embedded into the target packages.

>> +
>> +do_install[prefuncs] += "useradd_sysroot"
>> +SSTATEPOSTINSTFUNCS += "useradd_sysroot_sstate"
>> +
>> +# Recipe parse-time sanity checks
>> +def update_useradd_after_parse(d):
>> +	if bb.data.getVar('USERADD_PACKAGES', d) == None:
>> +		if bb.data.getVar('USERADD_PARAM', d) == None and bb.data.getVar('GROUPADD_PARAM', d) == None:
>> +			raise bb.build.FuncFailed, "%s inherits useradd but doesn't set USERADD_PARAM or GROUPADD_PARAM" % bb.data.getVar('FILE', d)
>> +
>> +python __anonymous() {
>> +	update_useradd_after_parse(d)
>> +}
>> +
>> +# Return a single [GROUP|USER]ADD_PARAM formatted string which includes the
>> +# [group|user]add parameters for all packages in this recipe
>> +def get_all_cmd_params(d, cmd_type):
>> +	import string
>> +	
>> +	localdata = bb.data.createCopy(d)
>> +	param_type = cmd_type.upper() + "ADD_PARAM_%s"
>> +	params = []
>> +
>> +	pkgs = bb.data.getVar('USERADD_PACKAGES', d, 1)
> 
> Please use True, not 1. Its also simpler to use:
> 
> d.getVar('USERADD_PACKAGES', True)

I hadn't noticed this in my earlier review -- but "d" should probably be using
"localdata" instead.  I realize at this point in the code "d" and "localdata"
are the same.... or ...

>> +	if pkgs == None:
> 
> if not pkgs:
> 
>> +		pkgs = bb.data.getVar('USERADDPN', d, 1)
>> +		packages = (bb.data.getVar('PACKAGES', d, 1) or "").split()
>> +		if not pkgs in packages and packages != []:
> 
> if packages and pkgs not in packages:
> 
>> +			pkgs = packages[0]
>> +
>> +	for pkg in pkgs.split():
>> +		param = bb.data.getVar(param_type % pkg, localdata, 1)
>> +		params.append(param)
>> +
>> +	return string.join(params, "; ")

... it's not clear to me that any of the 'd' elements are changing.  So
localdata may not actually be needed in any way.  Either we should be consistent
and use localdata everywhere or just 'd'.  (Normally I use localdata when I'm
transforming the data "in-place" and using setVar... but I don't see that in here.

>> +# Adds the preinst script into generated packages
>> +fakeroot python populate_packages_prepend () {
>> +	def update_useradd_package(pkg):
>> +		bb.debug(1, 'adding user/group calls to preinst for %s' % pkg)
>> +		localdata = bb.data.createCopy(d)
>> +		overrides = bb.data.getVar("OVERRIDES", localdata, 1)
>> +		bb.data.setVar("OVERRIDES", "%s:%s" % (pkg, overrides), localdata)
>> +		bb.data.update_data(localdata)

However, above we are using setVar, so localdata is correct.

>> +
>> +		"""
>> +		useradd preinst is appended here because pkg_preinst may be
>> +		required to execute on the target. Not doing so may cause
>> +		useradd preinst to be invoked twice, causing unwanted warnings.
>> +		"""
>> +		preinst = bb.data.getVar('pkg_preinst', localdata, 1)
>> +		if not preinst:
>> +			preinst = '#!/bin/sh\n'
>> +		preinst += bb.data.getVar('useradd_preinst', localdata, 1)
>> +		bb.data.setVar('pkg_preinst_%s' % pkg, preinst, d)
>> +
>> +	# We add the user/group calls to all packages to allow any package
>> +	# to contain files owned by the users/groups defined in the recipe.
>> +	# The user/group addition code is careful not to create duplicate
>> +	# entries, so this is safe.
>> +	pkgs = bb.data.getVar('USERADD_PACKAGES', d, 1)
>> +	if pkgs == None:
>> +		pkgs = bb.data.getVar('USERADDPN', d, 1)
>> +		packages = (bb.data.getVar('PACKAGES', d, 1) or "").split()
>> +		if not pkgs in packages and packages != []:
>> +			pkgs = packages[0]
>> +	for pkg in pkgs.split():
>> +		update_useradd_package(pkg)
>> +}
> 
> Cheers,
> 
> Richard
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Scott Garman - June 29, 2011, 1:20 a.m.
On 06/28/2011 06:04 AM, Richard Purdie wrote:
> Hi Scott,
>
> Sorry its taken me a while to get to this. Some comments below.

Hi Richard,

Thanks for the feedback. I'm hoping to get you a v2 pull request 
sometime tomorrow which incorporates some of your suggested changes. 
Mark gave good explanations on the points I will not be changing.

Scott
Richard Purdie - June 29, 2011, 2:22 p.m.
On Tue, 2011-06-28 at 09:42 -0500, Mark Hatle wrote:
> On 6/28/11 8:04 AM, Richard Purdie wrote:
> > Hi Scott,
> > 
> > Sorry its taken me a while to get to this. Some comments below.
> 
> I think I know the answer to a few of the issues mentioned below.  Scott can
> correct me if I'm wrong.
> 
> > On Thu, 2011-06-02 at 16:50 -0700, Scott Garman wrote:
> >> This class is to be used by recipes that need to set up specific
> >> user/group accounts and set custom file/directory permissions.
> >>
> >> Signed-off-by: Scott Garman <scott.a.garman@intel.com>
> >> ---
> >>  meta/classes/useradd.bbclass |  163 ++++++++++++++++++++++++++++++++++++++++++
> >>  1 files changed, 163 insertions(+), 0 deletions(-)
> >>  create mode 100644 meta/classes/useradd.bbclass
> >>
> >> diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass
> >> new file mode 100644
> >> index 0000000..3f07e5e
> >> --- /dev/null
> >> +++ b/meta/classes/useradd.bbclass
> >> @@ -0,0 +1,163 @@
> >> +USERADDPN ?= "${PN}"
> >> +
> >> +# base-passwd-cross provides the default passwd and group files in the
> >> +# target sysroot, and shadow-native provides the utilities needed to
> >> +# add and modify user and group accounts
> >> +DEPENDS_append = " base-passwd shadow-native"
> >> +RDEPENDS_${USERADDPN}_append = " base-passwd shadow"
> >> +
> >> +PSEUDO="${STAGING_DIR_NATIVE}/usr/bin/pseudo"
> >> +export PSEUDO
> > 
> > For reference this can be done with:
> > 
> > export PSEUDO = "${STAGING_DIR_NATIVE}/usr/bin/pseudo"
> > 
> >> +PSEUDO_LOCALSTATEDIR="${STAGING_DIR_TARGET}/var/pseudo"
> >> +export PSEUDO_LOCALSTATEDIR
> > 
> > I'm a little bit puzzled at this point. This is changing the default
> > PSEUDO state directory to be one shared by many other users rather than
> > the default of the one in workdir. Is that really what you intend here?
> > 
> > I guess the question is whether we need to preserve these users in the
> > sysroot or whether preserving them for the install/package/package_write
> > cycle is enough.
> 
> If we're populating the sysroot, we need to have a pseudo directory setup for it
> so that we can run the adduser/group items.  The new adduser/group require the
> ability to "chroot" into a (pseudo) root.  The above should only kick in while
> working with sysroots.

I understand this now. I still do have a concern about namespacing
though. I think alongside the existing code which does:

SYSROOT="${STAGING_DIR_TARGET}"
OPT="--root ${STAGING_DIR_TARGET}"

we should have these PSEUDO* exports, then they can't easily get
confused with the general pseudo environment we're using. Some comments
about what this is doing would also be useful as if I can't understand
this now, its likely in 3 months time its just going to be worse...

> The way this is implemented the new sysroot tasks call the regular tasks so the
> code only has to be implemented once.  Assuming context doesn't change, it
> should be possible to move the "PSEUDO" calls down into the _sysroot functions
> and remove them from the items that get embedded into the target packages.

I like this idea and I think it will be clearer whats going on,
particularly if we can do something like I mention above :)

> >> +			pkgs = packages[0]
> >> +
> >> +	for pkg in pkgs.split():
> >> +		param = bb.data.getVar(param_type % pkg, localdata, 1)
> >> +		params.append(param)
> >> +
> >> +	return string.join(params, "; ")
> 
> ... it's not clear to me that any of the 'd' elements are changing.  So
> localdata may not actually be needed in any way.  Either we should be consistent
> and use localdata everywhere or just 'd'.  (Normally I use localdata when I'm
> transforming the data "in-place" and using setVar... but I don't see that in here.

Good catch, I think there should be more use of localdata here too.

Cheers,

Richard

Patch

diff --git a/meta/classes/useradd.bbclass b/meta/classes/useradd.bbclass
new file mode 100644
index 0000000..3f07e5e
--- /dev/null
+++ b/meta/classes/useradd.bbclass
@@ -0,0 +1,163 @@ 
+USERADDPN ?= "${PN}"
+
+# base-passwd-cross provides the default passwd and group files in the
+# target sysroot, and shadow-native provides the utilities needed to
+# add and modify user and group accounts
+DEPENDS_append = " base-passwd shadow-native"
+RDEPENDS_${USERADDPN}_append = " base-passwd shadow"
+
+PSEUDO="${STAGING_DIR_NATIVE}/usr/bin/pseudo"
+export PSEUDO
+PSEUDO_LOCALSTATEDIR="${STAGING_DIR_TARGET}/var/pseudo"
+export PSEUDO_LOCALSTATEDIR
+PSEUDO_PASSWD = "${STAGING_DIR_TARGET}"
+export PSEUDO_PASSWD
+
+useradd_preinst () {
+OPT=""
+SYSROOT=""
+
+if test "x$D" != "x"; then
+	# Installing into a sysroot
+	SYSROOT="${STAGING_DIR_TARGET}"
+	OPT="--root ${STAGING_DIR_TARGET}"
+
+	# Add groups and users defined for all recipe packages
+	GROUPADD_PARAM="${@get_all_cmd_params(d, 'group')}"
+	USERADD_PARAM="${@get_all_cmd_params(d, 'user')}"
+else
+	# Installing onto a target
+	PSEUDO=""
+
+	# Add groups and users defined only for this package
+	GROUPADD_PARAM="${GROUPADD_PARAM}"
+	USERADD_PARAM="${USERADD_PARAM}"
+fi
+
+# Perform group additions first, since user additions may depend
+# on these groups existing
+if test "x$GROUPADD_PARAM" != "x"; then
+	echo "Running groupadd commands..."
+	# Invoke multiple instances of groupadd for parameter lists
+	# separated by ';'
+	opts=`echo "$GROUPADD_PARAM" | cut -d ';' -f 1`
+	remaining=`echo "$GROUPADD_PARAM" | cut -d ';' -f 2-`
+	while test "x$opts" != "x"; do
+		eval $PSEUDO groupadd -f $OPT $opts
+
+		if test "x$opts" = "x$remaining"; then
+			break
+		fi
+		opts=`echo "$remaining" | cut -d ';' -f 1`
+		remaining=`echo "$remaining" | cut -d ';' -f 2-`
+	done
+fi 
+
+if test "x$USERADD_PARAM" != "x"; then
+	echo "Running useradd commands..."
+	# Invoke multiple instances of useradd for parameter lists
+	# separated by ';'
+	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
+			eval $PSEUDO useradd $OPT $opts
+		else
+			echo "Note: username $username already exists, not re-creating it"
+		fi
+
+		if test "x$opts" = "x$remaining"; then
+			break
+		fi
+		opts=`echo "$remaining" | cut -d ';' -f 1`
+		remaining=`echo "$remaining" | cut -d ';' -f 2-`
+	done
+fi
+}
+
+useradd_sysroot () {
+	# Explicitly set $D since it isn't set to anything
+	# before do_install
+	D=${D}
+	useradd_preinst
+}
+
+useradd_sysroot_sstate () {
+	if [ "${BB_CURRENTTASK}" = "populate_sysroot_setscene" ]
+	then
+		useradd_sysroot
+	fi
+}
+
+do_install[prefuncs] += "useradd_sysroot"
+SSTATEPOSTINSTFUNCS += "useradd_sysroot_sstate"
+
+# Recipe parse-time sanity checks
+def update_useradd_after_parse(d):
+	if bb.data.getVar('USERADD_PACKAGES', d) == None:
+		if bb.data.getVar('USERADD_PARAM', d) == None and bb.data.getVar('GROUPADD_PARAM', d) == None:
+			raise bb.build.FuncFailed, "%s inherits useradd but doesn't set USERADD_PARAM or GROUPADD_PARAM" % bb.data.getVar('FILE', d)
+
+python __anonymous() {
+	update_useradd_after_parse(d)
+}
+
+# Return a single [GROUP|USER]ADD_PARAM formatted string which includes the
+# [group|user]add parameters for all packages in this recipe
+def get_all_cmd_params(d, cmd_type):
+	import string
+	
+	localdata = bb.data.createCopy(d)
+	param_type = cmd_type.upper() + "ADD_PARAM_%s"
+	params = []
+
+	pkgs = bb.data.getVar('USERADD_PACKAGES', d, 1)
+	if pkgs == None:
+		pkgs = bb.data.getVar('USERADDPN', d, 1)
+		packages = (bb.data.getVar('PACKAGES', d, 1) or "").split()
+		if not pkgs in packages and packages != []:
+			pkgs = packages[0]
+
+	for pkg in pkgs.split():
+		param = bb.data.getVar(param_type % pkg, localdata, 1)
+		params.append(param)
+
+	return string.join(params, "; ")
+
+# Adds the preinst script into generated packages
+fakeroot python populate_packages_prepend () {
+	def update_useradd_package(pkg):
+		bb.debug(1, 'adding user/group calls to preinst for %s' % pkg)
+		localdata = bb.data.createCopy(d)
+		overrides = bb.data.getVar("OVERRIDES", localdata, 1)
+		bb.data.setVar("OVERRIDES", "%s:%s" % (pkg, overrides), localdata)
+		bb.data.update_data(localdata)
+
+		"""
+		useradd preinst is appended here because pkg_preinst may be
+		required to execute on the target. Not doing so may cause
+		useradd preinst to be invoked twice, causing unwanted warnings.
+		"""
+		preinst = bb.data.getVar('pkg_preinst', localdata, 1)
+		if not preinst:
+			preinst = '#!/bin/sh\n'
+		preinst += bb.data.getVar('useradd_preinst', localdata, 1)
+		bb.data.setVar('pkg_preinst_%s' % pkg, preinst, d)
+
+	# We add the user/group calls to all packages to allow any package
+	# to contain files owned by the users/groups defined in the recipe.
+	# The user/group addition code is careful not to create duplicate
+	# entries, so this is safe.
+	pkgs = bb.data.getVar('USERADD_PACKAGES', d, 1)
+	if pkgs == None:
+		pkgs = bb.data.getVar('USERADDPN', d, 1)
+		packages = (bb.data.getVar('PACKAGES', d, 1) or "").split()
+		if not pkgs in packages and packages != []:
+			pkgs = packages[0]
+	for pkg in pkgs.split():
+		update_useradd_package(pkg)
+}