Misc fs/common.mk improvements

PATCH New Thomas Petazzoni 3 participants 9 messages Dec 3, 2018 – Dec 5, 2018 VFS

lore

Share & tools

lore

Thomas Petazzoni

author

2018-12-03 21:05 UTC #0 lore raw

v4 cover Misc fs/common.mk improvements

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(-)

Signature · 1 line

2.19.2

Thomas Petazzoni

author

2018-12-03 21:05 UTC #1 lore raw

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

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 <[email protected]>

---
 fs/common.mk | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

fs/common.mk +5 -5

index 6c67fbfbbd..6ad4cd0b01 100644
@@ -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))

Signature · 1 line

2.19.2

Yann E. MORIN

2018-12-03 21:18 UTC #2 lore raw

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

Thomas, All,

On 2018-12-03 22:05 +0100, Thomas Petazzoni spake thusly:

Quoted 11 lines

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 <[email protected]>

Acked-by "Yann E. MORIN" <[email protected]>

Regards,
Yann E. MORIN.

Quoted 50 lines

---
 fs/common.mk | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

fs/common.mk +5 -5

index 6c67fbfbbd..6ad4cd0b01 100644
@@ -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))

Signature · 1 line

2.19.2

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

Thomas Petazzoni

author

2018-12-03 21:05 UTC #3 lore raw

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

In reply to Thomas Petazzoni

From: "Yann E. MORIN" <[email protected]>

Reported-by Matthew Weber <[email protected]>

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Matthew Weber <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Signed-off-by Thomas Petazzoni <[email protected]>

---
 fs/common.mk | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

fs/common.mk +3 -2

index 6ad4cd0b01..077ce8903e 100644
@@ -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)

Signature · 1 line

2.19.2

Thomas Petazzoni

author

2018-12-03 21:05 UTC #4 lore raw

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

In reply to 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" <[email protected]>

Signed-off-by Thomas Petazzoni <[email protected]>

---
 fs/common.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/common.mk +1 -1

index 077ce8903e..a560417c6c 100644
@@ -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)

Signature · 1 line

2.19.2

Arnout Vandecappelle

2018-12-04 21:05 UTC #5 lore raw

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

Quoted 6 lines · 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

Quoted 6 lines

(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...

Quoted 12 lines

Reported-by "Yann E. MORIN" <[email protected]>
Signed-off-by Thomas Petazzoni <[email protected]>
---
 fs/common.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/common.mk +0 -0
index 077ce8903e..a560417c6c 100644

fs/common.mk endif · line 67

Quoted 7 lines

fs/common.mk +1 -0

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

Quoted 5 lines

fs/common.mk +0 -1

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

Thomas Petazzoni

author

2018-12-05 07:53 UTC #6 lore raw

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

Hello,

Quoted 7 lines · Arnout Vandecappelle

Quoted 7 lines

fs/common.mk +0 -0

index 077ce8903e..a560417c6c 100644
@@ -61,10 +61,10 @@ endif
 	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) > $(ROOTFS_FULL_DEVICES_TABLE)
 ifneq ($(ROOTFS_DEVICE_TABLES),)

Quoted 8 lines

fs/common.mk endif · line 67
Quoted 5 lines
fs/common.mk +1 -0
 	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

Signature · 3 lines

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

Arnout Vandecappelle

2018-12-05 09:18 UTC #7 lore raw

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

Quoted 28 lines · Thomas Petazzoni

Hello,
Quoted 7 lines · Arnout Vandecappelle
Quoted 7 lines
fs/common.mk +0 -0
index 077ce8903e..a560417c6c 100644
@@ -61,10 +61,10 @@ endif
 	$(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) > $(ROOTFS_FULL_DEVICES_TABLE)
 ifneq ($(ROOTFS_DEVICE_TABLES),)
Quoted 8 lines
fs/common.mk endif · line 67
Quoted 5 lines
fs/common.mk +1 -0
 	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)

Quoted 3 lines

endif
instead ?

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

Regards,
Arnout

Arnout Vandecappelle

Applied

2018-12-04 20:59 UTC #8 lore raw

v4 cover Misc fs/common.mk improvements

In reply to Thomas Petazzoni

Quoted 16 lines · 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.

Quoted 6 lines

- 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

Quoted 23 lines

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(-)