[Buildroot] [PATCH] fs: allow filesystems to set the name of their output file

Arnout Vandecappelle arnout at mind.be
Sat Nov 3 10:37:02 UTC 2018



On 24/10/18 16:36, Yann E. MORIN wrote:
> On 2018-10-23 22:01 -0300, Carlos Santos spake thusly:
>> 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.
>>
>> Signed-off-by: Carlos Santos <casantos at datacom.com.br>
>> ---
>> NOTE: https://patchwork.ozlabs.org/patch/927116/ must be upated to
>> document this.
>> ---
>>  fs/common.mk | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/common.mk b/fs/common.mk
>> index 453da6010a..22ca56a1ff 100644
>> --- a/fs/common.mk
>> +++ b/fs/common.mk
>> @@ -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)
> 
> 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)

 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

>     +
>      $(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.
> 
>> +$$(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
>>  
>> -- 
>> 2.17.1
>>
> 


More information about the buildroot mailing list