[OE-core] [PATCH] useradd-staticids.bbclass: Support recipes specifying static IDs

[Mark Hatle]

On 3/13/17 7:47 PM, Peter Kjellerstedt wrote:
>> -----Original Message-----
>> From: Mark Hatle [mailto:mark.hatle at windriver.com]
>> Sent: den 13 mars 2017 17:50
>> To: Peter Kjellerstedt; openembedded-core at lists.openembedded.org
>> Subject: Re: [OE-core] [PATCH] useradd-staticids.bbclass: Support
>> recipes specifying static IDs
>>
>> On 3/13/17 11:11 AM, Peter Kjellerstedt wrote:
>>>> -----Original Message-----
>>>> From: Mark Hatle [mailto:mark.hatle at windriver.com]
>>>> Sent: den 13 mars 2017 14:56
>>>> To: Peter Kjellerstedt; openembedded-core at lists.openembedded.org
>>>> Subject: Re: [OE-core] [PATCH] useradd-staticids.bbclass: Support
>>>> recipes specifying static IDs
>>>>
>>>> On 3/10/17 11:27 PM, Peter Kjellerstedt wrote:
>>>>> If this bblcass is used and a recipe specifies a static ID for a
>>>>> user/group as part of the USERADD_PARAM_${PN} or
>> GROUPADD_PARAM_${PN},
>>>>> the build would fail with and error like this if there was no
>>>>> corresponding ID in the passwd/group files specified via
>>>>> USERADD_UID_TABLES/USERADD_GID_TABLES:
>>>>
>>>> If the system is set in warning mode, it should warn if this is done
>>>> (but allow it).
>>>>
>>>> If the system is set to error mode, it should error.
>>>>
>>>> The useradd-staticids class is expecting the files (tables) to
>> contain
>>>> all of the entries.  This is specifically to avoid the situation
>> where
>>>> a new recipe is introduced that adds a new user or group (dynamic or
>>>> static -- static because it could conflict with one of your existing
>>>> static entries.)
>>>
>>> AFAICT, even before my rewrite of much of this code in 3149319a and
>>> subsequent correction in adc0f830, if a static ID was specified as
>> part
>>> of USERADD_PARAM_${PN} then no error would be issued. However, we can
>>> certainly change the code to always fail/warn if no ID is specified
>>> in any of the passwd/groups files. It should just be a matter of
>>> removing the if statements preceding the calls to handle_missing_id()
>>> below. (I will do some verification and send an updated patch
>>> tomorrow if this is the way the class should work.)
>>
>> The intention was always that the uname/gname had to be specified in
>> the useradd tables in some way, or a warning/error would occur.
>>
>> If the fields were blank in the tables, the system would use the
>> versions located in the recipe -- otherwise it would use the version in 
>> the table.  (At one point, but I don't think this was ever pushed in -- 
>> if a static ID was set in the recipe, there was a verification that the 
>> static version in both the recipe and the table files was the same -- 
>> otherwise it could indicate the package may not be built properly.)
>>
>>>> I agree the error message below looks wrong, do the existing
>>>> warning/error messages about it not being defined int he useradd 
>>>> tables still occur with this patch?
>>>
>>> The error message below occurred because before my fix it forgot all
>>> the parameters that were specified in USERADD_PARAM_${PN} when it
>>> took the continue.
>>
>> Ok, we need to fix that bug first then.
>>
>> We should then verify the second issue and fix it in a subsequent
>> commit.  The key being that of the user/group was not in the table 
>> we warn them (or error) -- preventing unexpected users from showing up.
> 
> I have verified now that removing the if statements will get us the wanted 
> behavior that all users/group must have static IDs in the passwd/groups 
> files even if they have static IDs defined in the recipe. I had intended 
> to just squash that change with the first one, but I can do it as two 
> separate commits.
> 
>> The check both static IDs are the same could very well be a TODO item
>> (that is never done) -- but something that should be considered.
> 
> There is a warning if a static ID is specified in the recipe and it is 
> changed due to having a different value in the passwd file. I think that 
> is good enough.

Perfect!  Thank you for checking all of this.

--Mark

>> Thanks!
>> --Mark
> 
> //Peter
>