fs: allow passing the image file name to inner-rootfs
RFC 1/1 fs: allow passing the image file name to inner-rootfs
Suppose that you want to create a filesystem image containing only the /var/lib subtree. That would something like this on a br2-external:
fs/rootfs-var-lib/Config.in:
menuconfig BR2_TARGET_ROOTFS_VAR_LIB
bool "/var/lib"
if BR2_TARGET_ROOTFS_VAR_LIB
source "$BR2_EXTERNAL_MFS_PATH/fs/rootfs-var-lib/ext4/Config.in"
endiffs/rootfs-var-lib/rootfs-var-lib.mk:
ifeq ($(BR2_TARGET_ROOTFS_VAR_LIB),y)
rootfs-var-lib = $(call inner-rootfs,var-lib-$(pkgname),VAR_LIB_$(call UPPERCASE,$(pkgname)))
include $(sort $(wildcard $(BR2_EXTERNAL_MFS_PATH)/fs/rootfs-var-lib/*/*.mk))
endiffs/rootfs-var-lib/ext4/Config.in:
config BR2_TARGET_ROOTFS_VAR_LIB_EXT4
bool "ext4 /var/lib filesystem"
select BR2_PACKAGE_HOST_E2FSPROGS
help
Build an ext4 /var/lib filesystem
if BR2_TARGET_ROOTFS_VAR_LIB_EXT4
[...]
endif # BR2_TARGET_ROOTFS_VAR_LIB_EXT4
fs/rootfs-var-lib/ext4/ext4.mk:
# Build the ext4 /var/lib filesystem image
[...]
ROOTFS_VAR_LIB_EXT4_OPTS = \
-d $(TARGET_DIR)/var/lib \
^^^^^^^^---- /var/lib, only
[...]
define ROOTFS_VAR_LIB_EXT4_CMD
rm -f $@
$(HOST_DIR)/sbin/mkfs.ext4 $(ROOTFS_VAR_LIB_EXT4_OPTS) $@ "$(ROOTFS_VAR_LIB_EXT4_SIZE)"
endef
$(eval $(rootfs-var-lib))Selecting BR2_TARGET_ROOTFS_VAR_LIB_EXT4 would produce the filesystem image $(BINARIES_DIR)/rootfs.var-lib-ext4 coresponging to the contents of $(TARGET_DIR)/var/lib. This is correct but the file name is a bit awkward. It would be better to call it "rootfs-var-lib.ext4", instead.
This can be achieved by passing a third argument to inner-rootfs with the desired file name:
rootfs-var-lib = $(call inner-rootfs,var-lib-$(pkgname),VAR_LIB_$(call UPPERCASE,$(pkgname)),rootfs-var-lib.$(pkgname))
--- fs/common.mk | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
fs/common.mk +6 -6
@@ -139,10 +139,10 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c endif -$$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) -$$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot -$$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) - @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") +$$(BINARIES_DIR)/$(3): ROOTFS=$(2) +$$(BINARIES_DIR)/$(3): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot +$$(BINARIES_DIR)/$(3): $$(ROOTFS_$(2)_DEPENDENCIES) + @$$(call MESSAGE,"Generating root filesystem image $(3)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)
@@ -163,7 +163,7 @@ endif rootfs-$(1)-show-depends: @echo $$(ROOTFS_$(2)_DEPENDENCIES) -rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) +rootfs-$(1): $$(BINARIES_DIR)/$(3) .PHONY: rootfs-$(1) rootfs-$(1)-show-depends
@@ -181,6 +181,6 @@ endif endef # $(pkgname) also works well to return the filesystem name -rootfs = $(call inner-rootfs,$(pkgname),$(call UPPERCASE,$(pkgname))) +rootfs = $(call inner-rootfs,$(pkgname),$(call UPPERCASE,$(pkgname)),rootfs.$(pkgname)) include $(sort $(wildcard fs/*/*.mk))
Signature · 1 line
2.17.1
Carlos, All,
On 2018-06-21 23:31 -0300, Carlos Santos spake thusly:
Quoted 2 lines
Suppose that you want to create a filesystem image containing only the
/var/libsubtree. That would something like this on a br2-external:
[--SNIP--]
Quoted 3 lines
This can be achieved by passing a third argument to inner-rootfs with the desired file name:
rootfs-var-lib = $(call inner-rootfs,var-lib-$(pkgname),VAR_LIB_$(call UPPERCASE,$(pkgname)),rootfs-var-lib.$(pkgname))
We've discussed this patch during the developers days, and we came to the conclusion that this should be better done by allowing filesystems to provide the name of their output file as a variable, something like:
ROOTFS_FOOBAR_IMAGE_NAME = my-own-filesystem.data
Then the infrastructure would do:
ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1)
$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): ROOTFS=$(2)
$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot
[...and so on...]Using a variable is more inline with how we handle this kind of customisations in Buildrooot, and allows one to continue calling the 'rootfs' macro instad of the 'rootfs-inner' one (which, as its name implies, is an inner macro, aka purely for internal use, not public).
Besides, the commit log was seen as too decorelated from the actual change. It is not needed to explain the 'sub-tree-only' case, and just state that some filesystems may want to tweak their output name, rather than the current scheme.
Regards,
Yann E. MORIN.
Quoted 48 lines
Signed-off-by Carlos Santos <[email protected]>--- fs/common.mk | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)fs/common.mk +6
-6@@ -139,10 +139,10 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c endif -$$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) -$$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot -$$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) - @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") +$$(BINARIES_DIR)/$(3): ROOTFS=$(2) +$$(BINARIES_DIR)/$(3): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot +$$(BINARIES_DIR)/$(3): $$(ROOTFS_$(2)_DEPENDENCIES) + @$$(call MESSAGE,"Generating root filesystem image $(3)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)@@ -163,7 +163,7 @@ endif rootfs-$(1)-show-depends: @echo $$(ROOTFS_$(2)_DEPENDENCIES) -rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) +rootfs-$(1): $$(BINARIES_DIR)/$(3) .PHONY: rootfs-$(1) rootfs-$(1)-show-depends@@ -181,6 +181,6 @@ endif endef # $(pkgname) also works well to return the filesystem name -rootfs = $(call inner-rootfs,$(pkgname),$(call UPPERCASE,$(pkgname))) +rootfs = $(call inner-rootfs,$(pkgname),$(call UPPERCASE,$(pkgname)),rootfs.$(pkgname)) include $(sort $(wildcard fs/*/*.mk))Signature · 6 lines
2.17.1 _______________________________________________ buildroot mailing list buildroot at busybox.net http://lists.busybox.net/mailman/listinfo/buildroot
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. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 fs: allow filesystems to set the name of their output file
Some filesystems may want to tweak their output names, rather than using the fixed "rootfs.foo" scheme. Add a ROOTFS_FOO_IMAGE_NAME variable for this purpose and document it.
---
NOTE: https://patchwork.ozlabs.org/patch/927116/ must be upated to document this.
--- fs/common.mk | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
fs/common.mk +6 -5
@@ -106,6 +106,7 @@ rootfs-common-show-depends: # all variable references except the arguments must be $$-quoted. define inner-rootfs +ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1) ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target
@@ -140,10 +141,10 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c endif -$$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) -$$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot -$$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) - @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): ROOTFS=$(2) +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES) + @$$(call MESSAGE,"Generating filesystem image $$(ROOTFS_$(2)_IMAGE_NAME)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)
@@ -164,7 +165,7 @@ endif rootfs-$(1)-show-depends: @echo $$(ROOTFS_$(2)_DEPENDENCIES) -rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) +rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME) .PHONY: rootfs-$(1) rootfs-$(1)-show-depends
Signature · 1 line
2.17.1
PATCH 1/1 fs: allow filesystems to set the name of their output file
On 2018-10-23 22:01 -0300, Carlos Santos spake thusly:
Quoted 25 lines
Some filesystems may want to tweak their output names, rather than using the fixed "rootfs.foo" scheme. Add a
ROOTFS_FOO_IMAGE_NAMEvariable for this purpose and document it.Signed-off-by Carlos Santos <[email protected]>---
NOTE: https://patchwork.ozlabs.org/patch/927116/ must be upated to document this.--- fs/common.mk | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)fs/common.mk +1
-0@@ -106,6 +106,7 @@ rootfs-common-show-depends: # all variable references except the arguments must be $$-quoted. define inner-rootfs +ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1) ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target
fs/common.mk ROOTFS_$(2)_COMPRESS_EXT = .xz · line 144
Quoted 8 lines
fs/common.mk +1
-4@@ -140,10 +141,10 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c endif -$$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) -$$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot -$$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) - @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): ROOTFS=$(2)
This unfortunately does not work when the filesystem gets 'imaginative' when setting that variable.
For example, I tweaked the ext2 fs in Buildroot to remove the current hook, and replacing it with a conditionally set name:
diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk
index 6bb4b1c7f8..923ccdca2f 100644
--- a/fs/ext2/ext2.mk
+++ b/fs/ext2/ext2.mk
@@ -40,7 +40,13 @@ ifneq ($(BR2_TARGET_ROOTFS_EXT2_GEN),2)
define ROOTFS_EXT2_SYMLINK
ln -sf rootfs.ext2$(ROOTFS_EXT2_COMPRESS_EXT) $(BINARIES_DIR)/rootfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN)$(ROOTFS_EXT2_COMPRESS_EXT)
endef
-ROOTFS_EXT2_POST_GEN_HOOKS += ROOTFS_EXT2_SYMLINK
+#ROOTFS_EXT2_POST_GEN_HOOKS += ROOTFS_EXT2_SYMLINK
endif
+ROOTFS_EXT2_IMAGE_NAME = \
+ $(if $(BR2_TARGET_ROOTFS_EXT2_2r0),rootfs.ext2) \
+ $(if $(BR2_TARGET_ROOTFS_EXT2_2r1),rootfs.ext2) \
+ $(if $(BR2_TARGET_ROOTFS_EXT2_3),rootfs.ext3) \
+ $(if $(BR2_TARGET_ROOTFS_EXT2_4),rootfs.ext4)
+
$(eval $(rootfs))And then, the variable gets a leading sapce, unfortunately.
So, you need to qstrip the variable before using it, probably going with an intermediate variable (in the fs infrastructure):
ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1)
ROOTFS_$(2)_FINAL_IMAGE_NAME = $$(call qstrip,$$(ROOTFS_$()_IMAGE_NAME))and then use ROOTFS_$(2)_FINAL_IMAGE_NAME to generate the rules...
Regards,
Yann E. MORIN.
Quoted 18 lines
fs/common.mk +4
-1+$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES) + @$$(call MESSAGE,"Generating filesystem image $$(ROOTFS_$(2)_IMAGE_NAME)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)@@ -164,7 +165,7 @@ endif rootfs-$(1)-show-depends: @echo $$(ROOTFS_$(2)_DEPENDENCIES) -rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) +rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME) .PHONY: rootfs-$(1) rootfs-$(1)-show-dependsSignature · 1 line
2.17.1
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. | '------------------------------^-------^------------------^--------------------'
PATCH v2 1/1 fs: allow filesystems to set the name of their output file
Some filesystems may want to tweak their output names, rather than using the fixed "rootfs.foo" scheme. Add a ROOTFS_FOO_IMAGE_NAME variable for this purpose.
---
Supersedes: https://patchwork.ozlabs.org/patch/988446/
Changes v1->v2:
- Prevent leading space when the filesystem gets 'imaginative', as pointed by Yann Morin.
NOTE: https://patchwork.ozlabs.org/patch/927116/ must be upated to document this.
--- fs/common.mk | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
fs/common.mk +7 -5
@@ -106,6 +106,8 @@ rootfs-common-show-depends: # all variable references except the arguments must be $$-quoted. define inner-rootfs +ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1) +ROOTFS_$(2)_FINAL_IMAGE_NAME = $$(call qstrip,$$(ROOTFS_$()_IMAGE_NAME)) ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target
@@ -140,10 +142,10 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c endif -$$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) -$$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot -$$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) - @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): ROOTFS=$(2) +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES) + @$$(call MESSAGE,"Generating filesystem image $$(ROOTFS_$(2)_FINAL_IMAGE_NAME)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)
@@ -164,7 +166,7 @@ endif rootfs-$(1)-show-depends: @echo $$(ROOTFS_$(2)_DEPENDENCIES) -rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) +rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME) .PHONY: rootfs-$(1) rootfs-$(1)-show-depends
Signature · 1 line
2.17.1
PATCH v2 1/1 fs: allow filesystems to set the name of their output file
Carlos, All,
On 2018-10-24 21:20 -0300, Carlos Santos spake thusly:
Quoted 23 lines
Some filesystems may want to tweak their output names, rather than using the fixed "rootfs.foo" scheme. Add a
ROOTFS_FOO_IMAGE_NAMEvariable for this purpose.Signed-off-by Carlos Santos <[email protected]>---
Supersedes: https://patchwork.ozlabs.org/patch/988446/Changes
v1->v2:
- Prevent leading space when the filesystem gets 'imaginative', as pointed by Yann Morin.NOTE: https://patchwork.ozlabs.org/patch/927116/ must be upated to document this.
--- fs/common.mk | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)fs/common.mk +0
-0
fs/common.mk rootfs-common-show-depends: · line 110
Quoted 5 lines
fs/common.mk +2
-0@@ -106,6 +106,8 @@ rootfs-common-show-depends: # all variable references except the arguments must be $$-quoted. define inner-rootfs +ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1) +ROOTFS_$(2)_FINAL_IMAGE_NAME = $$(call qstrip,$$(ROOTFS_$()_IMAGE_NAME))
^^^
This patch has not been tested at all, it can't even possibly work...
Regards,
Yann E. MORIN.
Quoted 30 lines
fs/common.mk +5
-5ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target@@ -140,10 +142,10 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c endif -$$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) -$$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot -$$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) - @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): ROOTFS=$(2) +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES) + @$$(call MESSAGE,"Generating filesystem image $$(ROOTFS_$(2)_FINAL_IMAGE_NAME)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)@@ -164,7 +166,7 @@ endif rootfs-$(1)-show-depends: @echo $$(ROOTFS_$(2)_DEPENDENCIES) -rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) +rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME) .PHONY: rootfs-$(1) rootfs-$(1)-show-dependsSignature · 1 line
2.17.1
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. | '------------------------------^-------^------------------^--------------------'
PATCH v2 1/1 fs: allow filesystems to set the name of their output file
Quoted 35 lines
From: "Yann Morin" <[email protected]>
To: "DATACOM" <[email protected]>
Cc: "buildroot" <[email protected]>
Sent: Quinta-feira, 1 de novembro de 2018 18:01:03
Subject: Re: [PATCH v2] fs: allow filesystems to set the name of their output fileCarlos, All,
On 2018-10-24 21:20 -0300, Carlos Santos spake thusly:
Quoted 26 lines
Some filesystems may want to tweak their output names, rather than using the fixed "rootfs.foo" scheme. Add a
ROOTFS_FOO_IMAGE_NAMEvariable for this purpose.Signed-off-by Carlos Santos <[email protected]>---
Supersedes: https://patchwork.ozlabs.org/patch/988446/Changes
v1->v2:
- Prevent leading space when the filesystem gets 'imaginative', as pointed by Yann Morin.NOTE: https://patchwork.ozlabs.org/patch/927116/ must be upated to document this.
--- fs/common.mk | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)fs/common.mk +0
-0@@ -106,6 +106,8 @@ rootfs-common-show-depends: # all variable references except the arguments must be $$-quoted. define inner-rootfs
Quoted 5 lines
fs/common.mk rootfs-common-show-depends: · line 110
Quoted 2 lines
fs/common.mk +2
-0+ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1) +ROOTFS_$(2)_FINAL_IMAGE_NAME = $$(call qstrip,$$(ROOTFS_$()_IMAGE_NAME))^^^This patch has not been tested at all, it can't even possibly work...
Pfff, I will send a follow-up fixing that. Sorry.
Scratching my head attempting to find how it happened.
[Mental note: stay away from recreational substances while at work].
Signature · 3 lines
Carlos Santos (Casantos) - DATACOM, P&D ?Marched towards the enemy, spear upright, armed with the certainty that only the ignorant can have.? ? Epitaph of a volunteer
PATCH v2 1/1 fs: allow filesystems to set the name of their output file
Quoted 13 lines
From: "DATACOM" <[email protected]>
To: "Yann Morin" <[email protected]>
Cc: "buildroot" <[email protected]>
Sent: Sexta-feira, 2 de novembro de 2018 23:20:06
Subject: Re: [Buildroot] [PATCH v2] fs: allow filesystems to set the name of their output fileQuoted 6 lines
From: "Yann Morin" <[email protected]>
To: "DATACOM" <[email protected]>
Cc: "buildroot" <[email protected]>
Sent: Quinta-feira, 1 de novembro de 2018 18:01:03
Subject: Re: [PATCH v2] fs: allow filesystems to set the name of their output file
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
+ROOTFS_$(2)_FINAL_IMAGE_NAME = $$(call qstrip,$$(ROOTFS_$()_IMAGE_NAME))
^^^This patch has not been tested at all, it can't even possibly work...
Pfff, I will send a follow-up fixing that. Sorry.
Scratching my head attempting to find how it happened.
FYI: I figured out the reason why the buggy patch was sent but I won't tell ya because it is too much embarrassing. :-(
Signature · 3 lines
Carlos Santos (Casantos) - DATACOM, P&D ?Marched towards the enemy, spear upright, armed with the certainty that only the ignorant can have.? ? Epitaph of a volunteer
PATCH v2 1/1 fs: allow filesystems to set the name of their output file
In reply to Carlos SantosSuperseded-by: https://patchwork.ozlabs.org/patch/992684/
PATCH 1/1 fs: allow filesystems to set the name of their output file
In reply to Yann E. MORINQuoted 50 lines · Yann E. MORIN
On 2018-10-23 22:01 -0300, Carlos Santos spake thusly:
Quoted 25 lines
Some filesystems may want to tweak their output names, rather than using the fixed "rootfs.foo" scheme. Add a
ROOTFS_FOO_IMAGE_NAMEvariable for this purpose and document it.Signed-off-by Carlos Santos <[email protected]>---
NOTE: https://patchwork.ozlabs.org/patch/927116/ must be upated to document this.--- fs/common.mk | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)fs/common.mk +1
-0@@ -106,6 +106,7 @@ rootfs-common-show-depends: # all variable references except the arguments must be $$-quoted. define inner-rootfs +ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1) ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/targetfs/common.mk ROOTFS_$(2)_COMPRESS_EXT = .xz · line 144
Quoted 8 lines
fs/common.mk +1
-4@@ -140,10 +141,10 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c endif -$$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) -$$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot -$$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) - @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): ROOTFS=$(2)This unfortunately does not work when the filesystem gets 'imaginative' when setting that variable.
For example, I tweaked the ext2 fs in Buildroot to remove the current hook, and replacing it with a conditionally set name:
diff --git a/fs/ext2/ext2.mk b/fs/ext2/ext2.mk index 6bb4b1c7f8..923ccdca2f 100644 --- a/fs/ext2/ext2.mk +++ b/fs/ext2/ext2.mk @@ -40,7 +40,13 @@ ifneq ($(BR2_TARGET_ROOTFS_EXT2_GEN),2) define ROOTFS_EXT2_SYMLINK ln -sf rootfs.ext2$(ROOTFS_EXT2_COMPRESS_EXT) $(BINARIES_DIR)/rootfs.ext$(BR2_TARGET_ROOTFS_EXT2_GEN)$(ROOTFS_EXT2_COMPRESS_EXT) endef -ROOTFS_EXT2_POST_GEN_HOOKS += ROOTFS_EXT2_SYMLINK
Quoted 8 lines
+#ROOTFS_EXT2_POST_GEN_HOOKS += ROOTFS_EXT2_SYMLINK
endif
+ROOTFS_EXT2_IMAGE_NAME = \
+ $(if $(BR2_TARGET_ROOTFS_EXT2_2r0),rootfs.ext2) \
+ $(if $(BR2_TARGET_ROOTFS_EXT2_2r1),rootfs.ext2) \
+ $(if $(BR2_TARGET_ROOTFS_EXT2_3),rootfs.ext3) \
+ $(if $(BR2_TARGET_ROOTFS_EXT2_4),rootfs.ext4) NACK that. You're defining ROOTFS_EXT2_IMAGE_NAME to contain spaces, so I think
that's a bug on your side, not on the side where we use the variable. The proper definition of this variable would be:
ifeq ($(BR2_TARGET_ROOTFS_EXT2_2r0),y)ROOTFS_EXT2_IMAGE_NAME = rootfs.ext2
else ifeq (....)
...
Because this is a bit verbose, we often handle the "switch" in Config.in rather
than *.mk.
You should know this stuff :-)
Bottom line: NACK against the stripping, Carlos' original patch was fine.
Regards,
Arnout
Quoted 35 lines
+ $(eval $(rootfs))And then, the variable gets a leading sapce, unfortunately.
So, you need to qstrip the variable before using it, probably going with an intermediate variable (in the fs infrastructure):
ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1) ROOTFS_$(2)_FINAL_IMAGE_NAME = $$(call qstrip,$$(ROOTFS_$()_IMAGE_NAME))and then use ROOTFS_$(2)_FINAL_IMAGE_NAME to generate the rules...
Regards,
Yann E. MORIN.Quoted 18 lines
fs/common.mk +4
-1+$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES) + @$$(call MESSAGE,"Generating filesystem image $$(ROOTFS_$(2)_IMAGE_NAME)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)@@ -164,7 +165,7 @@ endif rootfs-$(1)-show-depends: @echo $$(ROOTFS_$(2)_DEPENDENCIES) -rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) +rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME) .PHONY: rootfs-$(1) rootfs-$(1)-show-dependsSignature · 1 line
2.17.1
PATCH 1/1 fs: allow filesystems to set the name of their output file
Arnout, All,
On 2018-11-03 11:37 +0100, Arnout Vandecappelle spake thusly:
Quoted 1 line · Yann E. MORIN
[--SNIP--]
Quoted 8 lines
Quoted 5 lines
+ROOTFS_EXT2_IMAGE_NAME = \ + $(if $(BR2_TARGET_ROOTFS_EXT2_2r0),rootfs.ext2) \ + $(if $(BR2_TARGET_ROOTFS_EXT2_2r1),rootfs.ext2) \ + $(if $(BR2_TARGET_ROOTFS_EXT2_3),rootfs.ext3) \ + $(if $(BR2_TARGET_ROOTFS_EXT2_4),rootfs.ext4)NACK that. You're defining
ROOTFS_EXT2_IMAGE_NAMEto contain spaces, so I think
that's a bug on your side, not on the side where we use the variable.
So, why do we call qstrip on other user-provided options, like FOO_SITE, FOO_SOURCE, FOO_LICENSE_FILES for example, or others that we simply strip, like FOO_VERSION? Those are even less prone to contain unexpected spaces to begin with, yet we decided to sanitise them based on some feedback from users who would write things like (see 2f074857816):
FOO_VERSION = something # some comment
Note: in fact, here, when I suggested we call qstrip, I was wrong; I should have said plain strip instead. Mea culpa.
Quoted 6 lines
The proper
definition of this variable would be:ifeq ($(BR2_TARGET_ROOTFS_EXT2_2r0),y)
ROOTFS_EXT2_IMAGE_NAME= rootfs.ext2
else ifeq (....)
We already use the $(if...) construct in many places, so I don't see what is wrong with it, and I find it in fact more readable than the alternative... See the huge one in util-linux for an extreme example (which would be barely readbale should we switch to the ifeq-else-endif format), or aircrack-ng for a more reasonable case.
We even already use it in fs/btrfs too, which was added recently-ish.
Quoted 2 lines
Because this is a bit verbose, we often handle the "switch" in Config.in rather
than *.mk.
Sorry, but the above is pretty trivial, so yes it becomes easy to do in kconfig. But under more complex situations, where the extension can be computed, it might be easier to do in the .mk file.
Quoted 1 line
You should know this stuff :-)
Well, yeah, I know it, and I even tested it on version 1 of Carlos patch.
Quoted 1 line
Bottom line: NACK against the stripping, Carlos' original patch was fine.
Meh, I was dragged in just because that touches the fs infra, but I don't even care about this feature; I was even pretty oppposed to it to begin with. This is even only a corner case that we won't even be subjected to in Buildroot itself, so I don't care about the outcome...
My position with that would have been to suggest that, should a user want to name their generated image differently, they should do as we currently do in the ext filesystem: add a post-gen hook that creates a (sym|hard)link to the name of their heart's content.
Regards,
Yann E. MORIN.
Quoted 38 lines
Regards,
ArnoutQuoted 35 lines
+ $(eval $(rootfs))And then, the variable gets a leading sapce, unfortunately.
So, you need to qstrip the variable before using it, probably going with an intermediate variable (in the fs infrastructure):
ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1) ROOTFS_$(2)_FINAL_IMAGE_NAME = $$(call qstrip,$$(ROOTFS_$()_IMAGE_NAME))and then use ROOTFS_$(2)_FINAL_IMAGE_NAME to generate the rules...
Regards,
Yann E. MORIN.Quoted 18 lines
+$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
fs/common.mk +2
-1+ @$$(call MESSAGE,"Generating filesystem image $$(ROOTFS_$(2)_IMAGE_NAME)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)@@ -164,7 +165,7 @@ endif rootfs-$(1)-show-depends: @echo $$(ROOTFS_$(2)_DEPENDENCIES) -rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) +rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_IMAGE_NAME) .PHONY: rootfs-$(1) rootfs-$(1)-show-dependsSignature · 1 line
2.17.1
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. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 fs: allow filesystems to set the name of their output file
Some filesystems may want to tweak their output names, rather than using the fixed "rootfs.foo" scheme. Add a ROOTFS_FOO_IMAGE_NAME variable for this purpose.
---
Supersedes: https://patchwork.ozlabs.org/patch/988843/
Supersedes: https://patchwork.ozlabs.org/patch/988446/
---
Changes v1->v2:
- Prevent leading space when the filesystem gets 'imaginative', as pointed by Yann Morin.
Changes v2->v3:
- Fix typo and use strip instead of qstrip, as spotted by Yann Morin.
NOTE: https://patchwork.ozlabs.org/patch/927116/ must be upated to document this.
--- fs/common.mk | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
fs/common.mk +7 -5
@@ -106,6 +106,8 @@ rootfs-common-show-depends: # all variable references except the arguments must be $$-quoted. define inner-rootfs +ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1) +ROOTFS_$(2)_FINAL_IMAGE_NAME = $$(call strip,$$(ROOTFS_$(2)_IMAGE_NAME)) ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target
@@ -140,10 +142,10 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c endif -$$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) -$$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot -$$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) - @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): ROOTFS=$(2) +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES) + @$$(call MESSAGE,"Generating filesystem image $$(ROOTFS_$(2)_FINAL_IMAGE_NAME)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)
@@ -164,7 +166,7 @@ endif rootfs-$(1)-show-depends: @echo $$(ROOTFS_$(2)_DEPENDENCIES) -rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) +rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME) .PHONY: rootfs-$(1) rootfs-$(1)-show-depends
Signature · 1 line
2.17.1
PATCH 1/1 fs: allow filesystems to set the name of their output file
Carlos, All,
On 2018-11-03 19:09 -0300, Carlos Santos spake thusly:
Quoted 5 lines
Some filesystems may want to tweak their output names, rather than using the fixed "rootfs.foo" scheme. Add a
ROOTFS_FOO_IMAGE_NAMEvariable for this purpose.Signed-off-by Carlos Santos <[email protected]>
I was a bit reluctant at replying to this third iteration, but as Thomas asked for my opinion on IRC, here is my review...
I'll say it blundly: this patch does not work. It can't possibly have been tested as-is.
Quoted 21 lines
---
Supersedes: https://patchwork.ozlabs.org/patch/988843/
Supersedes: https://patchwork.ozlabs.org/patch/988446/
---
Changesv1->v2:
- Prevent leading space when the filesystem gets 'imaginative', as pointed by Yann Morin.
Changesv2->v3:
- Fix typo and use strip instead of qstrip, as spotted by Yann Morin.NOTE: https://patchwork.ozlabs.org/patch/927116/ must be upated to document this.
--- fs/common.mk | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)fs/common.mk +0
-0
fs/common.mk rootfs-common-show-depends: · line 110
Quoted 5 lines
fs/common.mk +2
-0@@ -106,6 +106,8 @@ rootfs-common-show-depends: # all variable references except the arguments must be $$-quoted. define inner-rootfs +ROOTFS_$(2)_IMAGE_NAME ?= rootfs.$(1) +ROOTFS_$(2)_FINAL_IMAGE_NAME = $$(call strip,$$(ROOTFS_$(2)_IMAGE_NAME))
'strip' is not a macro, it is a built-in, so $(call)-ing it expands to an empty string, so...
Quoted 4 lines
fs/common.mk +0
-0ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target
fs/common.mk ROOTFS_$(2)_COMPRESS_EXT = .xz · line 147
Quoted 10 lines
fs/common.mk +3
-4@@ -140,10 +142,10 @@ ROOTFS_$(2)_COMPRESS_EXT = .xz ROOTFS_$(2)_COMPRESS_CMD = xz -9 -C crc32 -c endif -$$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) -$$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot -$$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) - @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): ROOTFS=$(2) +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot +$$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME): $$(ROOTFS_$(2)_DEPENDENCIES)
... these rules thus all have just $$(BINARIES_DIR) as their target, so that doe not work very nicely...
fs/common.mk endif · line 169
Quoted 10 lines
fs/common.mk +2
-1+ @$$(call MESSAGE,"Generating filesystem image $$(ROOTFS_$(2)_FINAL_IMAGE_NAME)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT)@@ -164,7 +166,7 @@ endif rootfs-$(1)-show-depends: @echo $$(ROOTFS_$(2)_DEPENDENCIES) -rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) +rootfs-$(1): $$(BINARIES_DIR)/$$(ROOTFS_$(2)_FINAL_IMAGE_NAME)
... except maybe because of that?
Anyway, that is still an incorrect patch...
Regards,
Yann E. MORIN.
Quoted 6 lines
fs/common.mk +0
-0.PHONY: rootfs-$(1) rootfs-$(1)-show-dependsSignature · 1 line
2.17.1
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. | '------------------------------^-------^------------------^--------------------'
PATCH 1/1 fs: allow filesystems to set the name of their output file
In reply to Carlos SantosSuperseded by https://patchwork.ozlabs.org/patch/988843/
Superseded by https://patchwork.ozlabs.org/patch/988446/