Re: [PATCH 2/3] useradd.bbclass: new class for managing user/group permissions

[Scott Garman <scott.a.garman@intel.com>]

On 06/30/2011 04:10 PM, Richard Purdie wrote:
> Hi Scott,
>
> This is looking good, thanks for the changes! I just have some cosmetic
> things to tweak before it goes in.

Ok, respinning now based on your requests.

BTW, some parts of useradd.bbclass were derived from 
update-rc.d.bbclass. So it sounds like that class could use a refresh in 
light of the preferred style you've been showing me. I'd be happy to do 
this as a background task over the next week if it would be useful.

Scott

>
> On Thu, 2011-06-30 at 15:39 -0700, Scott Garman wrote:
> [...]
>> +# Recipe parse-time sanity checks
>> +def update_useradd_after_parse(d):
>> +	if bb.data.getVar('USERADD_PACKAGES', d) == None:
>
> if not d.getVar('USERADD_PACKAGES', False):
>
>> +		if bb.data.getVar('USERADD_PARAM', d) == None and bb.data.getVar('GROUPADD_PARAM', d) == None:
>
> if not d.getVar('USERADD_PARAM', False) and not d.getVar('GROUPADD_PARAM', False):
>
>> +			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)
>
> No need to use createCopy here since you don't change OVERRIDES or write
> to the datastore at all. Just use d directly.
>
>> +	param_type = cmd_type.upper() + "ADD_PARAM_%s"
>> +	params = []
>> +
>> +	pkgs = localdata.getVar('USERADD_PACKAGES', True)
>> +	if not pkgs:
>> +		pkgs = localdata.getVar('USERADDPN', True)
>> +		packages = (localdata.getVar('PACKAGES', True) or "").split()
>> +		if packages and pkgs not in packages:
>> +			pkgs = packages[0]
>> +
>> +	for pkg in pkgs.split():
>> +		param = localdata.getVar(param_type % pkg, True)
>> +		if param:
>> +			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 = localdata.getVar("OVERRIDES", True)
>> +		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.
>> +		"""
>
> You can just do:
>
> preinst = d.getVar('pkg_preinst_%s' % pkg, True) or d.getVar('pkg_preinst', True)
>
> and ditch all the createCopy/OVERRIDES stuff.
>
>> +		preinst = localdata.getVar('pkg_preinst', True)
>> +		if not preinst:
>> +			preinst = '#!/bin/sh\n'
>> +		preinst += localdata.getVar('useradd_preinst', True)
>> +		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 = d.getVar('USERADD_PACKAGES', True)
>> +	if pkgs == None:
>
> if not pkgs:
>
>> +		pkgs = d.getVar('USERADDPN', True)
>> +		packages = (d.getVar('PACKAGES', True) or "").split()
>> +		if not pkgs in packages and packages != []:
>
> if packages and pkgs not in packages:
>
>> +			pkgs = packages[0]
>> +	for pkg in pkgs.split():
>> +		update_useradd_package(pkg)
>> +}
>
> Cheers,
>
> Richard
>


-- 
Scott Garman
Embedded Linux Engineer - Yocto Project
Intel Open Source Technology Center