[Buildroot] [PATCH v4 0/3] Misc fs/common.mk improvements

[Buildroot] [PATCH v4 0/3] Misc fs/common.mk improvements

[Thomas Petazzoni]

Hello,

This small patch series is a new iteration of the few remaining
patches from Yann's series "[PATCH 0/6 v2] fs: fix and better handle
capabilities".

In Yann series, patches 1 to 3 have already been applied.

Patch 4 was not liked by Arnout, with some good explanation.

In this patch series, I propose:

 - An additional patch to rename FULL_DEVICE_TABLE to
   ROOTFS_FULL_DEVICES_TABLE to be consistent with
   ROOTFS_FULL_USERS_TABLE.

 - Yann's original PATCH 5/6, but reworked to apply without what was
   his PATCH 4/6.

 - A patch that reimplements PATCH 6/6 from Yann, but in a different
   way, due to the fact that his PATCH 4/6 was not taken, and
   therefore there is no permisson/device table split.

Changes since v3:

 - Reintegrate the idea of Yann's PATCH 6/6 (which was correct after
   all!), but in a way that works without splitting permission/device
   tables, since PATCH 4/6 from Yann was not accepted.

Thanks,

Thomas


Thomas Petazzoni (2):
  fs/common.mk: rename FULL_DEVICE_TABLE to ROOTFS_FULL_DEVICES_TABLE
  fs/common.mk: make sure that static devices from packages are created

Yann E. MORIN (1):
  fs/common.mk: allow user provided permissions to override packages
    permissions

 fs/common.mk | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

-- 
2.19.2

[Buildroot] [PATCH v4 1/3] fs/common.mk: rename FULL_DEVICE_TABLE to ROOTFS_FULL_DEVICES_TABLE

[Thomas Petazzoni]

In commit 6b50f988ad0a6044530b30344d52139da2994237 ("fs/common.mk:
rename internal variable"), USERS_TABLE was renamed to
ROOTFS_FULL_USERS_TABLE.

This commit follows the same direction by renaming the
FULL_DEVICE_TABLE variable to ROOTFS_FULL_DEVICE_TABLE.

In addition, for consistency, the file itself is renamed
full_device_table.txt.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 fs/common.mk | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/common.mk b/fs/common.mk
index 6c67fbfbbd..6ad4cd0b01 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -28,12 +28,12 @@
 # macro will automatically generate a compressed filesystem image.
 
 FS_DIR = $(BUILD_DIR)/buildroot-fs
-FULL_DEVICE_TABLE = $(FS_DIR)/device_table.txt
 ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \
 	$(BR2_ROOTFS_STATIC_DEVICE_TABLE))
 
 ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))
 
+ROOTFS_FULL_DEVICES_TABLE = $(FS_DIR)/full_devices_table.txt
 ROOTFS_FULL_USERS_TABLE = $(FS_DIR)/full_users_table.txt
 
 ifeq ($(BR2_REPRODUCIBLE),y)
@@ -58,12 +58,12 @@ ifneq ($(ROOTFS_USERS_TABLES),)
 	cat $(ROOTFS_USERS_TABLES) >> $(ROOTFS_FULL_USERS_TABLE)
 endif
 ifneq ($(ROOTFS_DEVICE_TABLES),)
-	cat $(ROOTFS_DEVICE_TABLES) > $(FULL_DEVICE_TABLE)
+	cat $(ROOTFS_DEVICE_TABLES) > $(ROOTFS_FULL_DEVICES_TABLE)
 ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
-	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE)
+	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
 endif
 endif
-	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE)
+	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
 
 rootfs-common-show-depends:
 	@echo $(ROOTFS_COMMON_DEPENDENCIES)
@@ -125,7 +125,7 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
 
 	echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
 	PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(ROOTFS_FULL_USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT)
-	echo "$$(HOST_DIR)/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
+	echo "$$(HOST_DIR)/bin/makedevs -d $$(ROOTFS_FULL_DEVICES_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
 	$$(foreach s,$$(call qstrip,$$(BR2_ROOTFS_POST_FAKEROOT_SCRIPT)),\
 		echo "echo '$$(TERM_BOLD)>>>   Executing fakeroot script $$(s)$$(TERM_RESET)'" >> $$(FAKEROOT_SCRIPT); \
 		echo $$(EXTRA_ENV) $$(s) $$(TARGET_DIR) $$(BR2_ROOTFS_POST_SCRIPT_ARGS) >> $$(FAKEROOT_SCRIPT)$$(sep))
-- 
2.19.2

[Buildroot] [PATCH v4 1/3] fs/common.mk: rename FULL_DEVICE_TABLE to ROOTFS_FULL_DEVICES_TABLE

[Yann E. MORIN]

Thomas, All,

On 2018-12-03 22:05 +0100, Thomas Petazzoni spake thusly:
> In commit 6b50f988ad0a6044530b30344d52139da2994237 ("fs/common.mk:
> rename internal variable"), USERS_TABLE was renamed to
> ROOTFS_FULL_USERS_TABLE.
> 
> This commit follows the same direction by renaming the
> FULL_DEVICE_TABLE variable to ROOTFS_FULL_DEVICE_TABLE.
> 
> In addition, for consistency, the file itself is renamed
> full_device_table.txt.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> ---
>  fs/common.mk | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/common.mk b/fs/common.mk
> index 6c67fbfbbd..6ad4cd0b01 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -28,12 +28,12 @@
>  # macro will automatically generate a compressed filesystem image.
>  
>  FS_DIR = $(BUILD_DIR)/buildroot-fs
> -FULL_DEVICE_TABLE = $(FS_DIR)/device_table.txt
>  ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \
>  	$(BR2_ROOTFS_STATIC_DEVICE_TABLE))
>  
>  ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))
>  
> +ROOTFS_FULL_DEVICES_TABLE = $(FS_DIR)/full_devices_table.txt
>  ROOTFS_FULL_USERS_TABLE = $(FS_DIR)/full_users_table.txt
>  
>  ifeq ($(BR2_REPRODUCIBLE),y)
> @@ -58,12 +58,12 @@ ifneq ($(ROOTFS_USERS_TABLES),)
>  	cat $(ROOTFS_USERS_TABLES) >> $(ROOTFS_FULL_USERS_TABLE)
>  endif
>  ifneq ($(ROOTFS_DEVICE_TABLES),)
> -	cat $(ROOTFS_DEVICE_TABLES) > $(FULL_DEVICE_TABLE)
> +	cat $(ROOTFS_DEVICE_TABLES) > $(ROOTFS_FULL_DEVICES_TABLE)
>  ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
> -	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE)
> +	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
>  endif
>  endif
> -	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE)
> +	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
>  
>  rootfs-common-show-depends:
>  	@echo $(ROOTFS_COMMON_DEPENDENCIES)
> @@ -125,7 +125,7 @@ $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
>  
>  	echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
>  	PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(ROOTFS_FULL_USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT)
> -	echo "$$(HOST_DIR)/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
> +	echo "$$(HOST_DIR)/bin/makedevs -d $$(ROOTFS_FULL_DEVICES_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
>  	$$(foreach s,$$(call qstrip,$$(BR2_ROOTFS_POST_FAKEROOT_SCRIPT)),\
>  		echo "echo '$$(TERM_BOLD)>>>   Executing fakeroot script $$(s)$$(TERM_RESET)'" >> $$(FAKEROOT_SCRIPT); \
>  		echo $$(EXTRA_ENV) $$(s) $$(TARGET_DIR) $$(BR2_ROOTFS_POST_SCRIPT_ARGS) >> $$(FAKEROOT_SCRIPT)$$(sep))
> -- 
> 2.19.2
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v4 2/3] fs/common.mk: allow user provided permissions to override packages permissions

[Thomas Petazzoni]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Reported-by: Matthew Weber <matthew.weber@rockwellcollins.com>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Matthew Weber <matthew.weber@rockwellcollins.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 fs/common.mk | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/common.mk b/fs/common.mk
index 6ad4cd0b01..077ce8903e 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -57,13 +57,14 @@ rootfs-common: $(ROOTFS_COMMON_DEPENDENCIES) target-finalize
 ifneq ($(ROOTFS_USERS_TABLES),)
 	cat $(ROOTFS_USERS_TABLES) >> $(ROOTFS_FULL_USERS_TABLE)
 endif
+
+	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) > $(ROOTFS_FULL_DEVICES_TABLE)
 ifneq ($(ROOTFS_DEVICE_TABLES),)
-	cat $(ROOTFS_DEVICE_TABLES) > $(ROOTFS_FULL_DEVICES_TABLE)
+	cat $(ROOTFS_DEVICE_TABLES) >> $(ROOTFS_FULL_DEVICES_TABLE)
 ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
 	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
 endif
 endif
-	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
 
 rootfs-common-show-depends:
 	@echo $(ROOTFS_COMMON_DEPENDENCIES)
-- 
2.19.2

[Buildroot] [PATCH v4 3/3] fs/common.mk: make sure that static devices from packages are created

[Thomas Petazzoni]

The static devices defined by packages are currently added to the full
device table when two conditions are met:

 (1) ROOTFS_DEVICE_TABLES is non-empty
 (2) BR2_ROOTFS_DEVICE_CREATION_STATIC=y

(2) is obviously correct. However, depending on (1) is not correct: if
the user doesn't provide any custom permission table and custom device
table, then ROOTFS_DEVICE_TABLES will be empty.

So instead, move the addition of the package-defined static devices
outside of condition (1), and have it only under condition (2).

Reported-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 fs/common.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/common.mk b/fs/common.mk
index 077ce8903e..a560417c6c 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -61,10 +61,10 @@ endif
 	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) > $(ROOTFS_FULL_DEVICES_TABLE)
 ifneq ($(ROOTFS_DEVICE_TABLES),)
 	cat $(ROOTFS_DEVICE_TABLES) >> $(ROOTFS_FULL_DEVICES_TABLE)
+endif
 ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
 	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
 endif
-endif
 
 rootfs-common-show-depends:
 	@echo $(ROOTFS_COMMON_DEPENDENCIES)
-- 
2.19.2

[Buildroot] [PATCH v4 3/3] fs/common.mk: make sure that static devices from packages are created

[Arnout Vandecappelle]

On 03/12/2018 22:05, Thomas Petazzoni wrote:
> The static devices defined by packages are currently added to the full
> device table when two conditions are met:
> 
>  (1) ROOTFS_DEVICE_TABLES is non-empty
>  (2) BR2_ROOTFS_DEVICE_CREATION_STATIC=y
> 
> (2) is obviously correct. However, depending on (1) is not correct: if
> the user doesn't provide any custom permission table and custom device
> table, then ROOTFS_DEVICE_TABLES will be empty.
> 
> So instead, move the addition of the package-defined static devices
> outside of condition (1), and have it only under condition (2).

 It's been like this since 2012, so probably this doesn't matter to anyone (most
likely because for static device creation, you *must* have a static devices
table so the ROOTFS_DEVICE_TABLES condition will be true).

 Still, it's cleaner this way, that's why I applied.

 However...

> 
> Reported-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
>  fs/common.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/common.mk b/fs/common.mk
> index 077ce8903e..a560417c6c 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -61,10 +61,10 @@ endif
>  	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) > $(ROOTFS_FULL_DEVICES_TABLE)
>  ifneq ($(ROOTFS_DEVICE_TABLES),)
>  	cat $(ROOTFS_DEVICE_TABLES) >> $(ROOTFS_FULL_DEVICES_TABLE)
> +endif
>  ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
>  	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
>  endif

 This is still conflicting with the purpose of patch 2/3: the package device
tables are still overriding the user-specified global device table.

 Regards,
 Arnout


> -endif
>  
>  rootfs-common-show-depends:
>  	@echo $(ROOTFS_COMMON_DEPENDENCIES)
> 

[Buildroot] [PATCH v4 3/3] fs/common.mk: make sure that static devices from packages are created

[Thomas Petazzoni]

Hello,

On Tue, 4 Dec 2018 22:05:19 +0100, Arnout Vandecappelle wrote:

> > diff --git a/fs/common.mk b/fs/common.mk
> > index 077ce8903e..a560417c6c 100644
> > --- a/fs/common.mk
> > +++ b/fs/common.mk
> > @@ -61,10 +61,10 @@ endif
> >  	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) > $(ROOTFS_FULL_DEVICES_TABLE)
> >  ifneq ($(ROOTFS_DEVICE_TABLES),)
> >  	cat $(ROOTFS_DEVICE_TABLES) >> $(ROOTFS_FULL_DEVICES_TABLE)
> > +endif
> >  ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
> >  	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
> >  endif  
> 
>  This is still conflicting with the purpose of patch 2/3: the package device
> tables are still overriding the user-specified global device table.

Fair point. Should we do:

	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) > $(ROOTFS_FULL_DEVICES_TABLE)
ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
endif
ifneq ($(ROOTFS_DEVICE_TABLES),)
	cat $(ROOTFS_DEVICE_TABLES) >> $(ROOTFS_FULL_DEVICES_TABLE)
endif

instead ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v4 3/3] fs/common.mk: make sure that static devices from packages are created

[Arnout Vandecappelle]

On 05/12/2018 08:53, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 4 Dec 2018 22:05:19 +0100, Arnout Vandecappelle wrote:
> 
>>> diff --git a/fs/common.mk b/fs/common.mk
>>> index 077ce8903e..a560417c6c 100644
>>> --- a/fs/common.mk
>>> +++ b/fs/common.mk
>>> @@ -61,10 +61,10 @@ endif
>>>  	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) > $(ROOTFS_FULL_DEVICES_TABLE)
>>>  ifneq ($(ROOTFS_DEVICE_TABLES),)
>>>  	cat $(ROOTFS_DEVICE_TABLES) >> $(ROOTFS_FULL_DEVICES_TABLE)
>>> +endif
>>>  ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
>>>  	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
>>>  endif  
>>
>>  This is still conflicting with the purpose of patch 2/3: the package device
>> tables are still overriding the user-specified global device table.
> 
> Fair point. Should we do:
> 
> 	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) > $(ROOTFS_FULL_DEVICES_TABLE)
> ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
> 	$(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FULL_DEVICES_TABLE)
> endif
> ifneq ($(ROOTFS_DEVICE_TABLES),)
> 	cat $(ROOTFS_DEVICE_TABLES) >> $(ROOTFS_FULL_DEVICES_TABLE)
> endif
> 
> instead ?

 Yep, that's what I meant. I guess.

 Regards,
 Arnout

[Buildroot] [PATCH v4 0/3] Misc fs/common.mk improvements

[Arnout Vandecappelle]

On 03/12/2018 22:05, Thomas Petazzoni wrote:
> Hello,
> 
> This small patch series is a new iteration of the few remaining
> patches from Yann's series "[PATCH 0/6 v2] fs: fix and better handle
> capabilities".
> 
> In Yann series, patches 1 to 3 have already been applied.
> 
> Patch 4 was not liked by Arnout, with some good explanation.
> 
> In this patch series, I propose:
> 
>  - An additional patch to rename FULL_DEVICE_TABLE to
>    ROOTFS_FULL_DEVICES_TABLE to be consistent with
>    ROOTFS_FULL_USERS_TABLE.
> 
>  - Yann's original PATCH 5/6, but reworked to apply without what was
>    his PATCH 4/6.
> 
>  - A patch that reimplements PATCH 6/6 from Yann, but in a different
>    way, due to the fact that his PATCH 4/6 was not taken, and
>    therefore there is no permisson/device table split.

 Applied series to master, thanks.


 Regards,
 Arnout

> 
> Changes since v3:
> 
>  - Reintegrate the idea of Yann's PATCH 6/6 (which was correct after
>    all!), but in a way that works without splitting permission/device
>    tables, since PATCH 4/6 from Yann was not accepted.
> 
> Thanks,
> 
> Thomas
> 
> 
> Thomas Petazzoni (2):
>   fs/common.mk: rename FULL_DEVICE_TABLE to ROOTFS_FULL_DEVICES_TABLE
>   fs/common.mk: make sure that static devices from packages are created
> 
> Yann E. MORIN (1):
>   fs/common.mk: allow user provided permissions to override packages
>     permissions
> 
>  fs/common.mk | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>