| Message ID | 1445007019-23720-1-git-send-email-peter@korsgaard.com |
|---|---|
| State | Rejected |
| Headers | show |
Peter, Yann, On Fri, 16 Oct 2015 16:50:19 +0200, Peter Korsgaard wrote: > The format of the users table files is non trivial, so it is sometimes handy > to add comments explaining the syntax (or simply the reason for the user) > inline in the files. > > Support comment lines prefixed with '#' similar to shell / makedevs files and strip > those lines before passing to the mkusers script. > > Signed-off-by: Peter Korsgaard <peter@korsgaard.com> Shouldn't it the responsibility of the mkusers script to ignore such comments instead ? Thanks, Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > Peter, Yann, > On Fri, 16 Oct 2015 16:50:19 +0200, Peter Korsgaard wrote: >> The format of the users table files is non trivial, so it is sometimes handy >> to add comments explaining the syntax (or simply the reason for the user) >> inline in the files. >> >> Support comment lines prefixed with '#' similar to shell / makedevs files and strip >> those lines before passing to the mkusers script. >> >> Signed-off-by: Peter Korsgaard <peter@korsgaard.com> > Shouldn't it the responsibility of the mkusers script to ignore such > comments instead ? It could be, but that is more work as it iterates over the (combined) file multiple times, and comments only really makes sense for custom users files (and nog <PKG>_USERS) so just stripping those lines when handling the custom files imho makes sense.
Peter, Thomas, All, On 2015-10-16 17:10 +0200, Peter Korsgaard spake thusly: > >>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > > > Peter, Yann, > > On Fri, 16 Oct 2015 16:50:19 +0200, Peter Korsgaard wrote: > >> The format of the users table files is non trivial, so it is sometimes handy > >> to add comments explaining the syntax (or simply the reason for the user) > >> inline in the files. > >> > >> Support comment lines prefixed with '#' similar to shell / makedevs files and strip > >> those lines before passing to the mkusers script. > >> > >> Signed-off-by: Peter Korsgaard <peter@korsgaard.com> > > > Shouldn't it the responsibility of the mkusers script to ignore such > > comments instead ? > > It could be, but that is more work as it iterates over the (combined) > file multiple times, Arguably, we could read the file only once and store the lines in variables. Then, we could do the filtering in the script. However, that script is not supposed to be called outside of Buildroot, so we don't really care where we do the filtering. > and comments only really makes sense for custom > users files (and nog <PKG>_USERS) so just stripping those lines when > handling the custom files imho makes sense. Still, the script could probably use a bit of love in this area... Regards, Yann E. MORIN.
Peter, All, On 2015-10-16 16:50 +0200, Peter Korsgaard spake thusly: > The format of the users table files is non trivial, so it is sometimes handy > to add comments explaining the syntax (or simply the reason for the user) > inline in the files. > > Support comment lines prefixed with '#' similar to shell / makedevs files and strip > those lines before passing to the mkusers script. > > Signed-off-by: Peter Korsgaard <peter@korsgaard.com> > --- > fs/common.mk | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/common.mk b/fs/common.mk > index 528e194..7110bc7 100644 > --- a/fs/common.mk > +++ b/fs/common.mk > @@ -87,7 +87,7 @@ endif > echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) > endif > ifneq ($$(ROOTFS_USERS_TABLES),) > - cat $$(ROOTFS_USERS_TABLES) >> $$(USERS_TABLE) > + sed '/^#/d' $$(ROOTFS_USERS_TABLES) >> $$(USERS_TABLE) Thomas and I both think it should be done in the script itself. I'll do it. ;-) Regards, Yann E. MORIN. > endif > printf '$$(subst $$(sep),\n,$$(PACKAGES_USERS))' >> $$(USERS_TABLE) > PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT) > -- > 2.1.4 >
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes: > Peter, All, > On 2015-10-16 16:50 +0200, Peter Korsgaard spake thusly: >> The format of the users table files is non trivial, so it is sometimes handy >> to add comments explaining the syntax (or simply the reason for the user) >> inline in the files. >> >> Support comment lines prefixed with '#' similar to shell / makedevs files and strip >> those lines before passing to the mkusers script. >> >> Signed-off-by: Peter Korsgaard <peter@korsgaard.com> >> --- >> fs/common.mk | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/common.mk b/fs/common.mk >> index 528e194..7110bc7 100644 >> --- a/fs/common.mk >> +++ b/fs/common.mk >> @@ -87,7 +87,7 @@ endif >> echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) >> endif >> ifneq ($$(ROOTFS_USERS_TABLES),) >> - cat $$(ROOTFS_USERS_TABLES) >> $$(USERS_TABLE) >> + sed '/^#/d' $$(ROOTFS_USERS_TABLES) >> $$(USERS_TABLE) > Thomas and I both think it should be done in the script itself. > I'll do it. ;-) Ok, fine by me. Again, I'm not particular in favour of one approach over the other - Doing it like this was just simpler as it is a trivial oneliner.
Peter, On Fri, 16 Oct 2015 16:50:19 +0200, Peter Korsgaard wrote: > The format of the users table files is non trivial, so it is sometimes handy > to add comments explaining the syntax (or simply the reason for the user) > inline in the files. > > Support comment lines prefixed with '#' similar to shell / makedevs files and strip > those lines before passing to the mkusers script. > > Signed-off-by: Peter Korsgaard <peter@korsgaard.com> Yann has added support in mkusers to ignore comments, so I applied Yann's patch and marked yours as Rejected. Thanks! Thomas
diff --git a/fs/common.mk b/fs/common.mk index 528e194..7110bc7 100644 --- a/fs/common.mk +++ b/fs/common.mk @@ -87,7 +87,7 @@ endif echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) endif ifneq ($$(ROOTFS_USERS_TABLES),) - cat $$(ROOTFS_USERS_TABLES) >> $$(USERS_TABLE) + sed '/^#/d' $$(ROOTFS_USERS_TABLES) >> $$(USERS_TABLE) endif printf '$$(subst $$(sep),\n,$$(PACKAGES_USERS))' >> $$(USERS_TABLE) PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT)
The format of the users table files is non trivial, so it is sometimes handy to add comments explaining the syntax (or simply the reason for the user) inline in the files. Support comment lines prefixed with '#' similar to shell / makedevs files and strip those lines before passing to the mkusers script. Signed-off-by: Peter Korsgaard <peter@korsgaard.com> --- fs/common.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)