Improvements for useradd-staticids.bbclass

Read and merge the passwd/group files before parsing the user and group definitions. This means they will only be read once per recipe. This solves a problem where if a user was definied in multiple files, it could generate group definitions for groups that should not be created. E.g., if the first passwd file read defines a user as:

foobar::1234::::

and the second passwd file defines it as:

foobar:::nogroup:The foobar user:/:/bin/sh

then a foobar group would be created even if the user will use the nogroup as its primary group.

-----Original Message-----
From: Mark Hatle [mailto:[email protected]]
Sent: den 4 november 2015 01:33
To: Peter Kjellerstedt; [email protected]
Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 6 lines · Peter Kjellerstedt

Read and merge the passwd/group files before parsing the user and group definitions. This means they will only be read once per recipe. This solves a problem where if a user was definied in multiple files, it could generate group definitions for groups that should not be created. E.g., if the first passwd file read defines a user as:

Quoted 8 lines

foobar::1234::::

and the second passwd file defines it as:

foobar:::nogroup:The foobar user:/:/bin/sh

then a foobar group would be created even if the user will use the nogroup as its primary group.

One minor thing

meta/classes/useradd-staticids.bbclass def update_useradd_static_config(d): · line 275

Quoted 10 lines

meta/classes/useradd-staticids.bbclass +1 -1

@@ -251,7 +269,7 @@ def update_useradd_static_config(d):
 
             newparams.append(newparam)
 
-        return " ;".join(newparams).strip()
+        return ";".join(newparams).strip()
 
     # Load and process the users and groups, rewriting the adduser/addgroup params
     useradd_packages = d.getVar('USERADD_PACKAGES', True)

The space was required because you could generate a user/group add line that ended with a string. Without the space, you could end up merging two sets of arguments causing a failure condition.

So I think that it should be retained unless there is a specific reason you believe it should be removed.

Quoted 40 lines

-----Original Message-----
From: Mark Hatle [mailto:[email protected]]
Sent: den 4 november 2015 01:33
To: Peter Kjellerstedt; [email protected]
Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 6 lines · Peter Kjellerstedt

Read and merge the passwd/group files before parsing the user and group definitions. This means they will only be read once per recipe. This solves a problem where if a user was definied in multiple files, it could generate group definitions for groups that should not be created. E.g., if the first passwd file read defines a user as:

Quoted 8 lines

foobar::1234::::

and the second passwd file defines it as:

foobar:::nogroup:The foobar user:/:/bin/sh

then a foobar group would be created even if the user will use the nogroup as its primary group.

One minor thing

meta/classes/useradd-staticids.bbclass def update_useradd_static_config(d): · line 275

Quoted 10 lines

meta/classes/useradd-staticids.bbclass +1 -1

@@ -251,7 +269,7 @@ def update_useradd_static_config(d):
 
             newparams.append(newparam)
 
-        return " ;".join(newparams).strip()
+        return ";".join(newparams).strip()
 
     # Load and process the users and groups, rewriting the adduser/addgroup params
     useradd_packages = d.getVar('USERADD_PACKAGES', True)

The space was required because you could generate a user/group add line that ended with a string. Without the space, you could end up merging two sets of arguments causing a failure condition.

Quoted 2 lines

So I think that it should be retained unless there is a specific reason you believe it should be removed.

I cannot see how that space can make any difference. Each set of useradd/grouppadd options added to newparams has the user/group name at the end of the string. And if that somehow interferes with the semicolon, then the code in useradd.bbclass which simply does "cut -d ';'" to split the useradd/groupadd line would break already.

Actually, now that I think about it, I do wonder why useradd-staticids.bbclass use this advanced variant to split the useradd/groupadd lines:

        for param in re.split('''[ \t]*;[ \t]*(?=(?:[^'"]|'[^']*'|"[^"]*")*$)''', params):

when this would do the job just as well:

for param in params.split(';'):

given that that is what useradd.bbclass does. It looks as if tries to support something like --comment "something with a ; in it", but using that would break in useradd.bbclass anyway...

//Peter

-----Original Message-----
From: Mark Hatle [mailto:[email protected]]
Sent: den 6 november 2015 21:14
To: Peter Kjellerstedt
Cc: [email protected]
Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 41 lines · Peter Kjellerstedt

Quoted 41 lines

-----Original Message-----
From: Mark Hatle [mailto:[email protected]]
Sent: den 4 november 2015 01:33
To: Peter Kjellerstedt; [email protected]
Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 7 lines · Peter Kjellerstedt

Read and merge the passwd/group files before parsing the user and group definitions. This means they will only be read once per recipe. This solves a problem where if a user was definied in multiple files, it could generate group definitions for groups that should not be created. E.g., if the first passwd file read defines a user as:

Quoted 8 lines

foobar::1234::::

and the second passwd file defines it as:

foobar:::nogroup:The foobar user:/:/bin/sh

then a foobar group would be created even if the user will use the nogroup as its primary group.

One minor thing

meta/classes/useradd-staticids.bbclass def update_useradd_static_config(d): · line 275

Quoted 10 lines

meta/classes/useradd-staticids.bbclass +1 -1

@@ -251,7 +269,7 @@ def update_useradd_static_config(d):
 
             newparams.append(newparam)
 
-        return " ;".join(newparams).strip()
+        return ";".join(newparams).strip()
 
     # Load and process the users and groups, rewriting the adduser/addgroup params
     useradd_packages = d.getVar('USERADD_PACKAGES', True)

The space was required because you could generate a user/group add line that ended with a string. Without the space, you could end up merging two sets of arguments causing a failure condition.

Quoted 8 lines

Quoted 2 lines

So I think that it should be retained unless there is a specific reason you believe it should be removed.

I cannot see how that space can make any difference. Each set of useradd/grouppadd options added to newparams has the user/group name at the end of the string. And if that somehow interferes with the semicolon, then the code in useradd.bbclass which simply does "cut -d ';'" to split the useradd/groupadd line would break already.

The contents when originally parsed my be run as arguments to a shell script or as parameters to these functions.

In the shell script world not have a space can confuse the argument parsing into thinking the ; is part of the argument.

You don't have that in the python world with the split behavior.

Quoted 5 lines

Actually, now that I think about it, I do wonder why useradd-staticids.bbclass use this advanced variant to split the useradd/groupadd lines:

        for param in re.split('''[ \t]*;[ \t]*(?=(?:[^'"]|'[^']*'|"[^"]*")*$)''', params):

It is perfectly legal to allow a ';' in the middle of a parameter (that allows it), a parameter that is quoted.

Something like:

adduser -c "This user;that user;all users" -d /home/allusers alluser

it's odd, but I've certainly seen people put ';' in the comment before.. and it is legal in other palces, like the home dir and such -- just not advised.

Quoted 4 lines

when this would do the job just as well:

for param in params.split(';'):

Quoted 3 lines

given that that is what useradd.bbclass does. It looks as if tries to support something like --comment "something with a ; in it", but using that would break in useradd.bbclass anyway...

Then the useradd class is broken in this case. The --comment processing needs to work, it's just rarely used in the normal case, but very much used in the "lets take a previously generated passwd file and reuse it" case of the adduser-static.

Quoted 1 line

//Peter

Quoted 59 lines

-----Original Message-----
From: Mark Hatle [mailto:[email protected]]
Sent: den 6 november 2015 21:14
To: Peter Kjellerstedt
Cc: [email protected]
Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 41 lines · Peter Kjellerstedt

Quoted 41 lines

-----Original Message-----
From: Mark Hatle [mailto:[email protected]]
Sent: den 4 november 2015 01:33
To: Peter Kjellerstedt; [email protected]
Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 7 lines · Peter Kjellerstedt

Read and merge the passwd/group files before parsing the user and group definitions. This means they will only be read once per recipe. This solves a problem where if a user was definied in multiple files, it could generate group definitions for groups that should not be created. E.g., if the first passwd file read defines a user as:

Quoted 8 lines

foobar::1234::::

and the second passwd file defines it as:

foobar:::nogroup:The foobar user:/:/bin/sh

then a foobar group would be created even if the user will use the nogroup as its primary group.

One minor thing

meta/classes/useradd-staticids.bbclass def update_useradd_static_config(d): · line 275

Quoted 10 lines

meta/classes/useradd-staticids.bbclass +1 -1

@@ -251,7 +269,7 @@ def update_useradd_static_config(d):
 
             newparams.append(newparam)
 
-        return " ;".join(newparams).strip()
+        return ";".join(newparams).strip()
 
     # Load and process the users and groups, rewriting the adduser/addgroup params
     useradd_packages = d.getVar('USERADD_PACKAGES', True)

The space was required because you could generate a user/group add line that ended with a string. Without the space, you could end up merging two sets of arguments causing a failure condition.

Quoted 8 lines

Quoted 2 lines

So I think that it should be retained unless there is a specific reason you believe it should be removed.

I cannot see how that space can make any difference. Each set of useradd/grouppadd options added to newparams has the user/group name at the end of the string. And if that somehow interferes with the semicolon, then the code in useradd.bbclass which simply does "cut -d ';'" to split the useradd/groupadd line would break already.

Quoted 5 lines

The contents when originally parsed my be run as arguments to a shell script or as parameters to these functions.

In the shell script world not have a space can confuse the argument parsing into thinking the ; is part of the argument.

No shell I have heard of (bash, zsh and dash comes to mind) would be affected by the lack of a space before the semicolon. Moreover, this is never actually parsed by a shell (except as part of a variable value). The semicolon is used by useradd.bbclass to split the variables, after which it lets the shell evaluate the part before the semicolon (which will ignore any trailing whitespace).

Quoted 10 lines

You don't have that in the python world with the split behavior.

Quoted 5 lines

Actually, now that I think about it, I do wonder why useradd-staticids.bbclass use this advanced variant to split the useradd/groupadd lines:

        for param in re.split('''[ \t]*;[ \t]*(?=(?:[^'"]|'[^']*'|"[^"]*")*$)''', params):

It is perfectly legal to allow a ';' in the middle of a parameter (that allows it), a parameter that is quoted.

Sure, and the code above handles some cases, but definitely not all. E.g., this would not be parsed as intended by useradd-staticids.bbclass:

USERADD_PARAMS_${PN} += "-c Comment\ with\ an\ \'\ in\ it oddcomment; \
-c Other\ odd \'\ comment otheruser"

but it would be handled correctly by useradd.bbclass...

Quoted 4 lines

Something like:

adduser -c "This user;that user;all users" -d /home/allusers alluser

Quoted 3 lines

it's odd, but I've certainly seen people put ';' in the comment before.. and it is legal in other palces, like the home dir and such -- just not advised.

It may be legal, but it has never been supported by the adduser.bbclass. And thus it has never worked in practice to take an existing passwd file that contains a semicolon in the comment field and expect it to work as input to adduser-staticids.bbclass.

Moreover, neither adduser.bbclass nor adduser-staticids.bbclass will handle a semicolon correctly in any other field. And let's not get started on other special characters like ", \, $ and > which could wreak havoc on both classes in a correctly crafted passwd file.

Quoted 13 lines

Quoted 7 lines

when this would do the job just as well:

for param in params.split(';'):

given that that is what useradd.bbclass does. It looks as if tries to support something like --comment "something with a ; in it", but using that would break in useradd.bbclass anyway...

Then the useradd class is broken in this case. The --comment processing needs to work, it's just rarely used in the normal case, but very much used in the "lets take a previously generated passwd file and reuse it" case of the adduser-static.

Quoted 1 line

Quoted 1 line

//Peter

So, from reading the code in both adduser.bbclass and adduser-staticids.bbclass, having spaces or no spaces before the semicolon in the USERADD_PARAM_${PN} variable cannot make any difference to how the users are created in the end. Thus it should be safe to remove them.

//Peter

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Mark Hatle
Sent: den 10 november 2015 17:07
To: Peter Kjellerstedt
Cc: [email protected]
Subject: Re: [OE-core] [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 66 lines · Peter Kjellerstedt

Quoted 60 lines

-----Original Message-----
From: Mark Hatle [mailto:[email protected]]
Sent: den 6 november 2015 21:14
To: Peter Kjellerstedt
Cc: [email protected]
Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 44 lines · Peter Kjellerstedt

Quoted 41 lines

-----Original Message-----
From: Mark Hatle [mailto:[email protected]]
Sent: den 4 november 2015 01:33
To: Peter Kjellerstedt; [email protected]
Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 7 lines · Peter Kjellerstedt

Read and merge the passwd/group files before parsing the user and group definitions. This means they will only be read once per recipe. This solves a problem where if a user was definied in multiple files, it could generate group definitions for groups that should not be created. E.g., if the first passwd file read defines a user as:

Quoted 8 lines

foobar::1234::::

and the second passwd file defines it as:

foobar:::nogroup:The foobar user:/:/bin/sh

then a foobar group would be created even if the user will use the nogroup as its primary group.

One minor thing

meta/classes/useradd-staticids.bbclass def update_useradd_static_config(d): · line 275

Quoted 10 lines

meta/classes/useradd-staticids.bbclass +1 -1

@@ -251,7 +269,7 @@ def update_useradd_static_config(d):
 
             newparams.append(newparam)
 
-        return " ;".join(newparams).strip()
+        return ";".join(newparams).strip()
 
     # Load and process the users and groups, rewriting the adduser/addgroup params
     useradd_packages = d.getVar('USERADD_PACKAGES', True)

The space was required because you could generate a user/group add line that ended with a string. Without the space, you could end up merging two sets of arguments causing a failure condition.

Quoted 2 lines

So I think that it should be retained unless there is a specific reason you believe it should be removed.

Quoted 6 lines

I cannot see how that space can make any difference. Each set of useradd/grouppadd options added to newparams has the user/group name at the end of the string. And if that somehow interferes with the semicolon, then the code in useradd.bbclass which simply does "cut -d ';'" to split the useradd/groupadd line would break already.

Quoted 5 lines

The contents when originally parsed my be run as arguments to a shell script or as parameters to these functions.

In the shell script world not have a space can confuse the argument parsing into thinking the ; is part of the argument.

Quoted 6 lines

No shell I have heard of (bash, zsh and dash comes to mind) would be affected by the lack of a space before the semicolon. Moreover, this is never actually parsed by a shell (except as part of a variable value). The semicolon is used by useradd.bbclass to split the variables, after which it lets the shell evaluate the part before the semicolon (which will ignore any trailing whitespace).

I've seen broken shells in the past where you would do something like:

/bin/echo foo;/bin/echo bar

and get: "/bin echo foo;/bin/echo bar" since it treated the middle item as a single command. I'm not saying it wasn't a bug in the shell or system -- just that I've been burned by it in the past and because of this, I try not to rely on it.. (when adding a space solves the issue.)

Quoted 18 lines

Quoted 10 lines

You don't have that in the python world with the split behavior.

Quoted 5 lines

Actually, now that I think about it, I do wonder why useradd-staticids.bbclass use this advanced variant to split the useradd/groupadd lines:

        for param in re.split('''[ \t]*;[ \t]*(?=(?:[^'"]|'[^']*'|"[^"]*")*$)''', params):

It is perfectly legal to allow a ';' in the middle of a parameter (that allows it), a parameter that is quoted.

Sure, and the code above handles some cases, but definitely not all. E.g., this would not be parsed as intended by useradd- staticids.bbclass:

USERADD_PARAMS_${PN} += "-c Comment\ with\ an\ \'\ in\ it oddcomment; \
-c Other\ odd \'\ comment otheruser"

Quoted 1 line

but it would be handled correctly by useradd.bbclass...

In this case, we need to emulate a reasonable set of argument processing. If there is something built into bitbake/oe then we can use it. The re.split though was a good approximation of the common configurations I was seeing at the time. (Specifically quotes parameters for spaces and ;)

Quoted 4 lines

Quoted 4 lines

Something like:

adduser -c "This user;that user;all users" -d /home/allusers alluser

Quoted 8 lines

Quoted 3 lines

it's odd, but I've certainly seen people put ';' in the comment before.. and it is legal in other palces, like the home dir and such -- just not advised.

It may be legal, but it has never been supported by the adduser.bbclass. And thus it has never worked in practice to take an existing passwd file that contains a semicolon in the comment field and expect it to work as input to adduser-staticids.bbclass.

Then that is a bug we should fix. At one time this was working
(perhaps not in OE, but locally?) Since I did have a customer who
had both spaces and ';' in their comment field. (This is how I originally found the problem and needed to figure out a regex that worked in more cases.)

Quoted 4 lines

Moreover, neither adduser.bbclass nor adduser-staticids.bbclass will handle a semicolon correctly in any other field. And let's not get started on other special characters like ", \, $ and > which could wreak havoc on both classes in a correctly crafted passwd file.

I realize there are may corner cases here. We need to try to support the reasonable ones and prevent the others. If that list should be avoided in certain cases -- then we should enhance the system catch things "not to do". But using ';' in the comment is fairly common in some environments.

Quoted 13 lines

Quoted 13 lines

Quoted 7 lines

when this would do the job just as well:

for param in params.split(';'):

given that that is what useradd.bbclass does. It looks as if tries to support something like --comment "something with a ; in it", but using that would break in useradd.bbclass anyway...

Then the useradd class is broken in this case. The --comment processing needs to work, it's just rarely used in the normal case, but very much used in the "lets take a previously generated passwd file and reuse it" case of the adduser-static.

Quoted 7 lines

Quoted 1 line

Quoted 1 line

//Peter

So, from reading the code in both adduser.bbclass and adduser-staticids.bbclass, having spaces or no spaces before the semicolon in the USERADD_PARAM_${PN} variable cannot make any difference to how the users are created in the end. Thus it should be safe to remove them.

The (initial) generated adduser/addgroup/etc command is dumped into the pre/post install script section of the packages. So when the package manager runs it needs to execute the shell script. This is where I've seen the problem of the ';' in the past...

Adduser and adduser-static should be synced to use the same delimitation mechanism. And for things like a ; in the comment, both should equally support it. We've got a mismatch and that is definitely a bug.

--Mark

Quoted 1 line

//Peter

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Peter Kjellerstedt
Sent: den 13 november 2015 13:52
To: Mark Hatle
Cc: [email protected]
Subject: Re: [OE-core] [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 3 lines

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf

Of

Quoted 7 lines

Mark Hatle
Sent: den 10 november 2015 17:07
To: Peter Kjellerstedt
Cc: [email protected]
Subject: Re: [OE-core] [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 19 lines

Quoted 18 lines · Peter Kjellerstedt

Quoted 18 lines

-----Original Message-----
From: Mark Hatle [mailto:[email protected]]
Sent: den 6 november 2015 21:14
To: Peter Kjellerstedt
Cc: [email protected]
Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 9 lines · Peter Kjellerstedt

Quoted 9 lines

-----Original Message-----
From: Mark Hatle [mailto:[email protected]]
Sent: den 4 november 2015 01:33
To: Peter Kjellerstedt; [email protected]
Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read passwd/group files before parsing

Quoted 1 line · Peter Kjellerstedt

Read and merge the passwd/group files before parsing the user

and

Quoted 17 lines

Quoted 17 lines

Quoted 17 lines

Quoted 17 lines

Quoted 17 lines

Quoted 6 lines

group definitions. This means they will only be read once per recipe. This solves a problem where if a user was definied in multiple files, it could generate group definitions for groups that should not be created. E.g., if the first passwd file read defines a user as:

Quoted 8 lines

foobar::1234::::

and the second passwd file defines it as:

foobar:::nogroup:The foobar user:/:/bin/sh

then a foobar group would be created even if the user will use the nogroup as its primary group.

One minor thing

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Review def update_useradd_static_config(d): · line 274

Quoted 8 lines

(quoted diff) +1 -1

@@ -251,7 +269,7 @@ def update_useradd_static_config(d):
 
             newparams.append(newparam)
 
-        return " ;".join(newparams).strip()
+        return ";".join(newparams).strip()
 
     # Load and process the users and groups, rewriting the

adduser/addgroup params

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Quoted 6 lines

Review def update_useradd_static_config(d): · line 275

Quoted 2 lines

(quoted diff) +0 -0

     useradd_packages = d.getVar('USERADD_PACKAGES', True)

The space was required because you could generate a user/group add line that ended with a string. Without the space, you could end up merging two sets of arguments causing a failure

condition.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

So I think that it should be retained unless there is a specific reason you believe it should be removed.

I cannot see how that space can make any difference. Each set of useradd/grouppadd options added to newparams has the user/group name at the end of the string. And if that somehow interferes

with

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

Quoted 3 lines

the semicolon, then the code in useradd.bbclass which simply does "cut -d ';'" to split the useradd/groupadd line would break already.

The contents when originally parsed my be run as arguments to a shell script or as parameters to these functions.

In the shell script world not have a space can confuse the

argument

Quoted 3 lines

Quoted 3 lines

Quoted 1 line

parsing into thinking the ; is part of the argument.

No shell I have heard of (bash, zsh and dash comes to mind) would

be

Quoted 1 line

Quoted 1 line

affected by the lack of a space before the semicolon. Moreover,

this

Quoted 6 lines

Quoted 4 lines

is never actually parsed by a shell (except as part of a variable value). The semicolon is used by useradd.bbclass to split the variables, after which it lets the shell evaluate the part before the semicolon (which will ignore any trailing whitespace).

I've seen broken shells in the past where you would do something

like:

Quoted 5 lines

/bin/echo foo;/bin/echo bar

and get: "/bin echo foo;/bin/echo bar" since it treated the middle item as a single command. I'm not saying it wasn't a bug in the

shell

Quoted 2 lines

or system -- just that I've been burned by it in the past and because of this, I try not to rely on it.. (when adding a space solves the

issue.)

Ok, sounds weird, but I will take your word for it.

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

You don't have that in the python world with the split behavior.

Quoted 5 lines

Actually, now that I think about it, I do wonder why useradd-staticids.bbclass use this advanced variant to split the useradd/groupadd lines:

        for param in re.split('''[ \t]*;[

\t]*(?=(?:[^'"]|'[^']*'|"[^"]*")*$)''', params):

Quoted 5 lines

Quoted 5 lines

Quoted 3 lines

It is perfectly legal to allow a ';' in the middle of a parameter (that allows it), a parameter that is quoted.

Sure, and the code above handles some cases, but definitely not

all.

Quoted 4 lines

Quoted 4 lines

E.g., this would not be parsed as intended by useradd- staticids.bbclass:

USERADD_PARAMS_${PN} += "-c Comment\ with\ an\ \'\ in\ it

oddcomment; \

Quoted 9 lines

Quoted 3 lines

-c Other\ odd \'\ comment otheruser"

but it would be handled correctly by useradd.bbclass...

In this case, we need to emulate a reasonable set of argument processing. If there is something built into bitbake/oe then we can use it. The re.split though was a good approximation of the common configurations I was seeing at the time. (Specifically quotes parameters for spaces and ;)

Normally I would suggest shlex.split(). It is already used by both BitBake and OE-Core. Unfortunately, it cannot be used in this case as it cannot distinguish between an escaped/quoted semicolon and one that is not escaped/quoted. This is because it is designed to split one command line, not multiple lines.

So the split of multiple commands would probably still have to be done using a regular expression, extended to support escaping.

shlex.split() can be used for the second split though (where the command is fed to the parser).

We would also need the opposite to construct a command line with all options properly quoted. Unfortunately, shlex.quote() was not added until Python 3.3. However, there is an unofficial pipes.quote() that is already used by OE-Core, so I assume it is ok to use.

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Something like:

adduser -c "This user;that user;all users" -d /home/allusers

alluser

Quoted 7 lines

Quoted 7 lines

Quoted 4 lines

it's odd, but I've certainly seen people put ';' in the comment before.. and it is legal in other palces, like the home dir and such -- just not advised.

It may be legal, but it has never been supported by the adduser.bbclass. And thus it has never worked in practice to take

an

Quoted 10 lines

Quoted 2 lines

existing passwd file that contains a semicolon in the comment field and expect it to work as input to adduser-staticids.bbclass.

Then that is a bug we should fix. At one time this was working
(perhaps not in OE, but locally?) Since I did have a customer who
had both spaces and ';' in their comment field. (This is how I originally found the problem and needed to figure out a regex that worked in more cases.)

Quoted 1 line

Moreover, neither adduser.bbclass nor adduser-staticids.bbclass

will

Quoted 4 lines

Quoted 3 lines

handle a semicolon correctly in any other field. And let's not get started on other special characters like ", \, $ and > which could wreak havoc on both classes in a correctly crafted passwd file.

Quoted 11 lines

I realize there are may corner cases here. We need to try to support the reasonable ones and prevent the others. If that list should be avoided in certain cases -- then we should enhance the system catch things "not to do". But using ';' in the comment is fairly common in some environments.

Quoted 5 lines

Quoted 5 lines

Quoted 5 lines

when this would do the job just as well:

for param in params.split(';'):

given that that is what useradd.bbclass does. It looks as if

tries

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

to support something like --comment "something with a ; in it",

but

Quoted 9 lines

Quoted 9 lines

Quoted 8 lines

Quoted 1 line

using that would break in useradd.bbclass anyway...

Then the useradd class is broken in this case. The --comment processing needs to work, it's just rarely used in the normal case, but very much used in the "lets take a previously generated passwd file and reuse it" case of the adduser-static.

Quoted 1 line

//Peter

Quoted 7 lines

Quoted 5 lines

So, from reading the code in both adduser.bbclass and adduser-staticids.bbclass, having spaces or no spaces before the semicolon in the USERADD_PARAM_${PN} variable cannot make any difference to how the users are created in the end. Thus it should be safe to remove them.

The (initial) generated adduser/addgroup/etc command is dumped into

the

Quoted 2 lines

pre/post install script section of the packages. So when the package manager runs it needs to execute the shell script. This is where

I've

Quoted 3 lines

seen the problem of the ';' in the past...

Adduser and adduser-static should be synced to use the same

delimitation

Quoted 1 line

mechanism. And for things like a ; in the comment, both should

equally

Quoted 1 line

support it. We've got a mismatch and that is definitely a bug.

I agree that the correct thing to do is to make them support the same mechanism. However, this would require quite some more work, especially with the useradd.bbclass, and as it does not affect the changes in my patches, do you think the patches can be accepted as they are, and we can work on improving the parsing afterwards?

Quoted 3 lines

--Mark

Quoted 1 line

//Peter

//Peter

--
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core

This series of patches aims to improve useradd-staticids.bbclass.

We are currently using useradd-staticids.bbclass to make sure all users and groups have well defined IDs. So far we have had the definitions of the users both in the recipes and in the passwd file used by useradd-staticids.bbclass. Since we have a huge number of recipes that create users, having to duplicate the definitions all over the place has turned out to be a burden we should be able to avoid.

So the current plan for us is to have one passwd file per layer with the definitions of all users that layer needs. These definitions do not include the static IDs for the users. Instead the static IDs for the users are specified in a distro specific passwd-static file. There is also a distro specific group-static file for the group IDs. With that in place it should be enough to define a user as:

USERADD_PARAM_${PN} = "--system foobar"

in a recipe and let useradd-staticids.bbclass handle the specifics for how that user should be defined.

The above worked fine for all users that had a primary group with the same name as the user. However, it turned out that for users that wanted some other primary group, specifying it in the passwd file was not enough. We still had to add --gid <some group> in the recipe where <some group> had to match what was specified in the passwd file. This was less than optimal, and somewhat defeated the setup.

It also turned out that for users with a primary group that does not match the user name, useradd-staticids.bbclass would still add the creation of a group with the same name as the user (when it parsed the passwd-static file) and the add another creation of the correct group (when it parsed the passwd file).

So after spending quite a lot of time on trying to decode how rewrite_useradd() calculated the --gid option, I came up with this series of changes that should correct the problems described above and make the code easier to understand while (hopefully) maintaining compatibility with the old code.

I fully understand that this is very late for Jethro and do not really expect it to make it into 2.0.0, but if at all possible I hope it can make it since that would make my life much easier. :)

//Peter

The following changes since commit fc45deac89ef63ca1c44e763c38ced7dfd72cbe1:

build-appliance-image: Update to jethro head revision (2015-11-03 14:03:03 +0000)

are available in the git repository at:

git://git.yoctoproject.org/poky-contrib pkj/useradd_improvements
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=pkj/useradd_improvements

Peter Kjellerstedt (5):
useradd-staticids.bbclass: Treat mutually exclusive options as such useradd-staticids.bbclass: Make --no-user-group have effect useradd-staticids.bbclass: Simplify some logic useradd-staticids.bbclass: Simplify the logic for when to add groups useradd-staticids.bbclass: Read passwd/group files before parsing

 meta/classes/useradd-staticids.bbclass | 192 ++++++++++++++++++---------------
 1 file changed, 103 insertions(+), 89 deletions(-)