diff mbox

[1/1] system: add ability to pass additional users tables.

Message ID 1395937176-7585-2-git-send-email-eric.le.bihan.dev@free.fr
State Superseded
Headers show

Commit Message

Eric Le Bihan March 27, 2014, 4:19 p.m. UTC
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(-)

Comments

Yann E. MORIN March 27, 2014, 6:19 p.m. UTC | #1
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.
Eric Le Bihan March 28, 2014, 11:10 a.m. UTC | #2
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
Yann E. MORIN March 28, 2014, 4:50 p.m. UTC | #3
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.
Yann E. MORIN April 20, 2014, 5:21 p.m. UTC | #4
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 mbox

Patch

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 ""