*ping* //Peter
> -----Original Message----- > From: openembedded-core-boun...@lists.openembedded.org > [mailto:openembedded-core-boun...@lists.openembedded.org] On Behalf Of > Peter Kjellerstedt > Sent: den 13 november 2015 13:52 > To: Mark Hatle > Cc: openembedded-core@lists.openembedded.org > Subject: Re: [OE-core] [PATCH 5/5] useradd-staticids.bbclass: Read > passwd/group files before parsing > > > -----Original Message----- > > From: openembedded-core-boun...@lists.openembedded.org > > [mailto:openembedded-core-boun...@lists.openembedded.org] On Behalf > Of > > Mark Hatle > > Sent: den 10 november 2015 17:07 > > To: Peter Kjellerstedt > > Cc: openembedded-core@lists.openembedded.org > > Subject: Re: [OE-core] [PATCH 5/5] useradd-staticids.bbclass: Read > > passwd/group files before parsing > > > > On 11/10/15 9:54 AM, Peter Kjellerstedt wrote: > > >> -----Original Message----- > > >> From: Mark Hatle [mailto:mark.ha...@windriver.com] > > >> Sent: den 6 november 2015 21:14 > > >> To: Peter Kjellerstedt > > >> Cc: openembedded-core@lists.openembedded.org > > >> Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read > > >> passwd/group files before parsing > > >> > > >> On 11/6/15 2:09 PM, Peter Kjellerstedt wrote: > > >>>> -----Original Message----- > > >>>> From: Mark Hatle [mailto:mark.ha...@windriver.com] > > >>>> Sent: den 4 november 2015 01:33 > > >>>> To: Peter Kjellerstedt; openembedded-core@lists.openembedded.org > > >>>> Subject: Re: [PATCH 5/5] useradd-staticids.bbclass: Read > > >>>> passwd/group files before parsing > > >>>> > > >>>> On 11/3/15 6:06 PM, Peter Kjellerstedt wrote: > > >>>>> 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. > > >>>> > > >>>> One minor thing > > >>>> > > >>>>> @@ -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. > > >>> > > >>> 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. > > > > > > 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.) > > Ok, sounds weird, but I will take your word for it. > > > >> You don't have that in the python world with the split behavior. > > >> > > >>> 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... > > > > 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. > > > >> 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. > > > > > > 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.) > > > > > 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. > > > > >>> 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. > > >> > > >>> //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. > > 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? > > > --Mark > > > > > //Peter > > //Peter > > -- > _______________________________________________ > Openembedded-core mailing list > Openembedded-core@lists.openembedded.org > http://lists.openembedded.org/mailman/listinfo/openembedded-core -- _______________________________________________ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core