Message ID | 1395937176-7585-2-git-send-email-eric.le.bihan.dev@free.fr |
---|---|
State | Superseded |
Headers | show |
Eric, All, On 2014-03-27 17:19 +0100, Eric Le Bihan spake thusly: > A new entry has been added to the "System Configuration" menu to allow > the user to set the location of additional user tables (besides the ones > defined in packages). > > A user table is a text file, formatted using the mkusers syntax, which > describes the users on the target system, with their UID/GID, home > directory, password, etc. > > The target root file system will be populated according the content of > these files. > > Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr> > --- > fs/common.mk | 6 +++++- > system/Config.in | 9 +++++++++ > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/fs/common.mk b/fs/common.mk > index d95c26b..6f37bd0 100644 > --- a/fs/common.mk > +++ b/fs/common.mk > @@ -33,6 +33,7 @@ FULL_DEVICE_TABLE = $(BUILD_DIR)/_device_table.txt > ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \ > $(BR2_ROOTFS_STATIC_DEVICE_TABLE)) > USERS_TABLE = $(BUILD_DIR)/_users_table.txt > +ROOTFS_USERS_TABLE = $(call qstrip,$(BR2_ROOTFS_USERS_TABLE)) > > define ROOTFS_TARGET_INTERNAL > > @@ -78,7 +79,10 @@ endif > printf '$$(subst $$(sep),\n,$$(PACKAGES_PERMISSIONS_TABLE))' >> $$(FULL_DEVICE_TABLE) > echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) > endif > - printf '$(subst $(sep),\n,$(PACKAGES_USERS))' > $(USERS_TABLE) > +ifneq ($$(ROOTFS_USERS_TABLE),) > + cat $$(ROOTFS_USERS_TABLE) > $(USERS_TABLE) > +endif > + printf '$(subst $(sep),\n,$(PACKAGES_USERS))' >> $(USERS_TABLE) > $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT) > echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT) > chmod a+x $$(FAKEROOT_SCRIPT) > diff --git a/system/Config.in b/system/Config.in > index e8f1ed6..b7052f5 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -280,6 +280,15 @@ config BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW > > endif # BR2_ROOTFS_SKELETON_DEFAULT > > +config BR2_ROOTFS_USERS_TABLE > + string "Path to the users tables" > + help > + Specify a space-separated list of users table locations, > + that will be passed to the mkusers utility to create > + users on the system, with home directory, password, etc. > + > + See manual for details on the usage and syntax of these files. > + There is a discrepancy bewteen the description commit log and this help text, and the actual behaviour. The commit log and help text both explain this can be a space-separated list of files, but in the code you just treat it as if it were a unique file: ifneq ($$(ROOTFS_USERS_TABLE),) cat $$(ROOTFS_USERS_TABLE) > $(USERS_TABLE) endif I think you should do something like: $(foreach f,$$(ROOTFS_USERS_TABLE),cat $$(f) >>$(USERS_TABLE)$(sep)) Regards, Yann E. MORIN.
Hi! On Thu, Mar 27, 2014 at 07:19:15PM +0100, Yann E. MORIN wrote: > Eric, All, > [...] > > There is a discrepancy bewteen the description commit log and this help > text, and the actual behaviour. The commit log and help text both > explain this can be a space-separated list of files, but in the code you > just treat it as if it were a unique file: > > ifneq ($$(ROOTFS_USERS_TABLE),) > cat $$(ROOTFS_USERS_TABLE) > $(USERS_TABLE) > endif > > I think you should do something like: > > $(foreach f,$$(ROOTFS_USERS_TABLE),cat $$(f) >>$(USERS_TABLE)$(sep)) In fact I use the same trick as for ROOTFS_DEVICE_TABLES: whether ROOTFS_USERS_TABLE is 'foo' or 'foo bar quux', the invocation of `cat` is valid, as `cat` takes a list of filenames as arguments. So no need to loop on a list. Or is the loop preferred because of portability issues? But it is true that I should use the plural form ROOTFS_USERS_TABLES. Thanks for your review. Best regards, ELB
Eric, All, On 2014-03-28 12:10 +0100, Eric Le Bihan spake thusly: > On Thu, Mar 27, 2014 at 07:19:15PM +0100, Yann E. MORIN wrote: > > Eric, All, > > > [...] > > > > There is a discrepancy bewteen the description commit log and this help > > text, and the actual behaviour. The commit log and help text both > > explain this can be a space-separated list of files, but in the code you > > just treat it as if it were a unique file: > > > > ifneq ($$(ROOTFS_USERS_TABLE),) > > cat $$(ROOTFS_USERS_TABLE) > $(USERS_TABLE) > > endif > > > > I think you should do something like: > > > > $(foreach f,$$(ROOTFS_USERS_TABLE),cat $$(f) >>$(USERS_TABLE)$(sep)) > In fact I use the same trick as for ROOTFS_DEVICE_TABLES: whether > ROOTFS_USERS_TABLE is 'foo' or 'foo bar quux', the invocation of `cat` is > valid, as `cat` takes a list of filenames as arguments. So no need to loop > on a list. Oh, right. OK, then. > But it is true that I should use the plural form ROOTFS_USERS_TABLES. Yes, please. Regards, Yann E. MORIN.
On 2014-03-27 17:19 +0100, Eric Le Bihan spake thusly: > A new entry has been added to the "System Configuration" menu to allow > the user to set the location of additional user tables (besides the ones > defined in packages). > > A user table is a text file, formatted using the mkusers syntax, which > describes the users on the target system, with their UID/GID, home > directory, password, etc. > > The target root file system will be populated according the content of > these files. > > Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Although a minor nit below: > --- > fs/common.mk | 6 +++++- > system/Config.in | 9 +++++++++ > 2 files changed, 14 insertions(+), 1 deletion(-) > > diff --git a/fs/common.mk b/fs/common.mk > index d95c26b..6f37bd0 100644 > --- a/fs/common.mk > +++ b/fs/common.mk > @@ -33,6 +33,7 @@ FULL_DEVICE_TABLE = $(BUILD_DIR)/_device_table.txt > ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \ > $(BR2_ROOTFS_STATIC_DEVICE_TABLE)) > USERS_TABLE = $(BUILD_DIR)/_users_table.txt > +ROOTFS_USERS_TABLE = $(call qstrip,$(BR2_ROOTFS_USERS_TABLE)) This is a list of space-separated files, so I'd prefer TABLE be a plural TABLES. > define ROOTFS_TARGET_INTERNAL > > @@ -78,7 +79,10 @@ endif > printf '$$(subst $$(sep),\n,$$(PACKAGES_PERMISSIONS_TABLE))' >> $$(FULL_DEVICE_TABLE) > echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) > endif > - printf '$(subst $(sep),\n,$(PACKAGES_USERS))' > $(USERS_TABLE) > +ifneq ($$(ROOTFS_USERS_TABLE),) > + cat $$(ROOTFS_USERS_TABLE) > $(USERS_TABLE) Rename here too. > +endif > + printf '$(subst $(sep),\n,$(PACKAGES_USERS))' >> $(USERS_TABLE) > $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT) > echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT) > chmod a+x $$(FAKEROOT_SCRIPT) > diff --git a/system/Config.in b/system/Config.in > index e8f1ed6..b7052f5 100644 > --- a/system/Config.in > +++ b/system/Config.in > @@ -280,6 +280,15 @@ config BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW > > endif # BR2_ROOTFS_SKELETON_DEFAULT > > +config BR2_ROOTFS_USERS_TABLE Rename here. Regards, Yann E. MORIN.
diff --git a/fs/common.mk b/fs/common.mk index d95c26b..6f37bd0 100644 --- a/fs/common.mk +++ b/fs/common.mk @@ -33,6 +33,7 @@ FULL_DEVICE_TABLE = $(BUILD_DIR)/_device_table.txt ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \ $(BR2_ROOTFS_STATIC_DEVICE_TABLE)) USERS_TABLE = $(BUILD_DIR)/_users_table.txt +ROOTFS_USERS_TABLE = $(call qstrip,$(BR2_ROOTFS_USERS_TABLE)) define ROOTFS_TARGET_INTERNAL @@ -78,7 +79,10 @@ endif printf '$$(subst $$(sep),\n,$$(PACKAGES_PERMISSIONS_TABLE))' >> $$(FULL_DEVICE_TABLE) echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) endif - printf '$(subst $(sep),\n,$(PACKAGES_USERS))' > $(USERS_TABLE) +ifneq ($$(ROOTFS_USERS_TABLE),) + cat $$(ROOTFS_USERS_TABLE) > $(USERS_TABLE) +endif + printf '$(subst $(sep),\n,$(PACKAGES_USERS))' >> $(USERS_TABLE) $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT) echo "$$(ROOTFS_$(2)_CMD)" >> $$(FAKEROOT_SCRIPT) chmod a+x $$(FAKEROOT_SCRIPT) diff --git a/system/Config.in b/system/Config.in index e8f1ed6..b7052f5 100644 --- a/system/Config.in +++ b/system/Config.in @@ -280,6 +280,15 @@ config BR2_TARGET_GENERIC_REMOUNT_ROOTFS_RW endif # BR2_ROOTFS_SKELETON_DEFAULT +config BR2_ROOTFS_USERS_TABLE + string "Path to the users tables" + help + Specify a space-separated list of users table locations, + that will be passed to the mkusers utility to create + users on the system, with home directory, password, etc. + + See manual for details on the usage and syntax of these files. + config BR2_ROOTFS_OVERLAY string "Root filesystem overlay directories" default ""
A new entry has been added to the "System Configuration" menu to allow the user to set the location of additional user tables (besides the ones defined in packages). A user table is a text file, formatted using the mkusers syntax, which describes the users on the target system, with their UID/GID, home directory, password, etc. The target root file system will be populated according the content of these files. Signed-off-by: Eric Le Bihan <eric.le.bihan.dev@free.fr> --- fs/common.mk | 6 +++++- system/Config.in | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-)